Skip to content

fix(state,vscode-wcs): recursive-path quality pass after five review cycles - #261

Merged
mogera551 merged 7 commits into
mainfrom
improve/recursive-path-quality
Sep 11, 2026
Merged

mogera551 merged 7 commits into
mainfrom
improve/recursive-path-quality

Conversation

@mogera551

Copy link
Copy Markdown
Contributor

Summary

Quality pass over the recursive-path feature ($recursion / **) that landed in #259 / #260 and is not released yet. Five independent reviewers ran one after another; within each cycle the fixes were re-verified until that reviewer reported nothing left. 88 findings were handled and one was rejected with a reason. Every finding, fix and deferral is recorded in docs/state-recursive-path-impl-plan.md §7-4 to §7-10.

Runtime (@wcstack/state)

  • Writes into a recursive getter are refused at every entry. A concrete expansion, an index spelling (nodes.1.total, nodes.*.children.0.total), a ** suffix, or a path inside the getter's value now raise wcs/recursion-readonly before the walk. Before, the value landed on the node object and was pinned as the getter's cached result, corrupting the aggregates above it.
  • Structural writes also cover <list>.length and index spellings of child nodes and child lists.
  • Declaration-time rejections carry codes. A ** getter that names the structure, a concrete getter colliding with an expansion (own or on the prototype), and an index segment in the anchor or repeat raise wcs/recursion-declaration-invalid; a malformed ** suffix raises wcs/recursion-anchor.
  • A bound ** takes its depth from the innermost frame only. A plain getter reached from a recursive getter reports wcs/recursion-context instead of a raw ListIndex not found or a silently wrong value.
  • More consumers refuse **: $trackDependency, $postUpdate, $listKeys keys. A non-array indexes on $getAll throws a diagnostic instead of a TypeError (this also applies to plain $getAll; listed under Changed).
  • Re-seeding is transactional. Every pure validation runs before teardown, so a failed re-seed leaves the element on the previous generation. A successful re-seed of the same object drops the generated accessors with their dependency edges and cache entries, including getters whose suffix has its own wildcard.
  • Root-only: a mounted component (bind-component) that declares a ** getter gets wcs/mount-dollar-declaration.
  • Performance: the read and write gates memoize per interned PathInfo. Unrelated writes on a state with $recursion went from up to 1.5x to 1.02-1.09x of a plain state; deep reads from +123 ns to +42 ns.
  • Types: defineState types this["nodes.**.x"] and this["nodes.**"] as any, matching the editor preamble.
  • Structure: pure path algebra moved to expand.ts, generation teardown to generation.ts, materialize.ts folded into getByAddress; the recursion test harness is shared through __tests__/helpers/recursionTestUtils.ts.

Editor and wcs-validate (vscode-wcs, @wcstack/lint)

  • Parity with every runtime form above: readonly on concrete, index and shifted spellings; structural length and structural getter keys; concrete collisions; ++ / --; assignments, $postUpdate, $trackDependency, $listKeys; non-array $getAll indexes; $recursion inside volumes and bind-component blocks.
  • No false errors when the declaration cannot be read statically: identifier references, class states, top-level spreads, computed keys, non-literal repeat values.
  • ${}-free template literals are read as path arguments. This also reaches wcs/index-arity and getter-untracked-read, so existing pages can get new diagnostics (noted in the root CHANGELOG).
  • packages/lint/dist/cli.cjs is regenerated; the smoke test gained a spread-declaration case.

Known defects (unchanged, pinned in integration.recursionKnownDefects.test.ts)

Tests

Package Result
packages/state 3238 tests; coverage 99.57 / 98.61 / 100 / 99.75 (thresholds 99.5 / 98.5 / 100 / 99.5); lint and tsc --noEmit clean
packages/vscode-wcs 854 tests (was 800); tsc --noEmit clean
packages/lint smoke 18 / 18

packages/state/dist is not rebuilt (tracked; the release workflow rebuilds it).

Follow-ups (not in this PR)

  • wcstack-skill: the new diagnostics and the refused forms.
  • Minor release and the vsix.

🤖 Generated with Claude Code

mogera551 and others added 7 commits September 11, 2026 23:14
…d 2)

Runtime (@wcstack/state):
- Writes to a recursive getter's expanded concrete path (and inside its
  value) are rejected with wcs/recursion-readonly instead of polluting the
  row object and pinning a stale cache entry.
- Generated accessors no longer overwrite an author-defined prototype
  getter (class-syntax states); the collision check walks the prototype
  chain.
- `<repeat>.length` counts as a structural write in recursive $setAll.
- Recursion depth is resolved from the innermost address frame only; a
  plain getter reached from a recursive getter raises recursion-context.
- Re-seeding with the same state object also drops the generated
  accessors' cache entries, not just their dependency edges.
- Paths inside a recursive getter's value no longer raise a false
  binding-path-missing.
- $listKeys rejects `**`; $getAll form checks are diagnosed in runtime
  order (anchor, then array-ness, then prefix).
- Dead surface removed from the recursion registry / accessor types.

Editor / lint (vscode-wcs):
- No "declares no $recursion anchor" error when the declaration cannot be
  read statically (identifier reference, class syntax).
- matchesRecursion folds every depth and prefix, so `**` getters whose
  suffix repeats the list are found.
- New diagnostics: recursion-readonly on concrete-path writes,
  recursion-unsupported on `**` assignments / $postUpdate /
  $trackDependency / $listKeys, structural `.length` writes, and
  $recursion inside a mount volume.

Docs: README (en/ja), CHANGELOG [Unreleased], vscode-wcs CHANGELOG,
impl-plan §7-4.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Index spellings in API path arguments ($setAll / $resolve, including
  wildcard-and-index mixes such as `nodes.*.children.0.total`) are folded
  to `*` before the recursion-readonly check, on both the runtime and the
  editor side, with the same predicate as ResolvedAddress.
- The editor no longer reports "declares no $recursion anchor" when the
  default export brings the declaration in through a top-level spread.
- `**` assignments inside string / template literals are not diagnosed.
- Test fixes and wording; impl-plan §7-5 / §7-6.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Runtime (@wcstack/state):
- Index spellings in the suffix of a `**` path (`nodes.**.children.0`,
  `.children.0.children`, `.children.0.total`) are folded before the
  structural / readonly checks, so they no longer replace the structure
  or stop mid-write with a raw TypeError.
- A `**` getter whose suffix names the recursion structure itself
  (`nodes.**.children`, `.children.length`, index spellings of those)
  is rejected when the declaration is read instead of shadowing the
  real child list.
- $trackDependency rejects `**` like the other APIs do.
- Re-seeding with the same state object also drops cache entries of
  `**` getters whose suffix contains a wildcard.
- The recursion declaration is validated before the state object is
  swapped in, so an invalid re-seed leaves the element on the previous
  generation.
- Concrete paths for bound reads are memoized per (suffix, depth);
  unused walk options / parts types removed; hasRecursion and
  recursionRegistry are required on IStateElement.

Editor / lint (vscode-wcs):
- No error when the repeat sub-path is not a plain string literal
  unless it is definitely not a string; `${}`-free template literals
  are accepted.
- Structural `**` getter keys, non-array $getAll indexes (`null`, a
  literal) and index-spelled suffixes are diagnosed with the runtime's
  codes.

Known defect #257 now also covers the new declaration-time raise
(pinned in integration.recursionKnownDefects.test.ts). Docs: README
(en/ja), CHANGELOGs, impl-plan §7-7.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Runtime (@wcstack/state):
- A concrete getter with the same name as a `**` getter's expansion
  (`get "nodes.*.children.*.total"()`) is rejected when the declaration
  is read, not on the first read of that depth.
- A mounted component (bind-component) that declares a `**` getter gets
  the wcs/mount-dollar-declaration notice instead of being dropped
  silently.
- The read-only check's memo is keyed by the folded pattern, so index
  spellings no longer grow it; the recursion-anchor message lives in
  pathDiagnostics; the 127/128/129 depth boundary is pinned.

Editor / lint (vscode-wcs):
- Concrete-getter collisions, `++` / `--` on `**` or expansion paths,
  and `$recursion` / `**` getters inside a bind-component block are
  diagnosed; computed keys and spreads inside the `$recursion` value
  are treated as "cannot decide" instead of "empty".
- Mount / volume blocks still get the declaration-independent checks.
- smoke-test: a state that brings `$recursion` in through a spread
  must exit 0 (passes once dist/cli.cjs is rebuilt).

Docs: README (en/ja) "not in this version" list and union order,
demo README, CHANGELOGs consolidated (unreleased fixes folded into
Added; the plain $getAll indexes diagnostic kept under Changed),
impl-plan §7-8.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Runtime (@wcstack/state):
- A recursive $setAll into the value of an expansion shifted by the
  repeat (`nodes.**.children.*.total.x`) is refused before the walk,
  like the other read-only forms (coversSuffix).
- The write-side read-only gate memoizes per interned PathInfo, so a
  state with $recursion pays no measurable overhead on unrelated
  writes (was up to 1.5x).
- Declaration-time raises carry wcs/recursion-declaration-invalid;
  a `**` getter key that does not match the anchor reports
  wcs/recursion-anchor through the shared message.
- A `**` suffix with an empty segment or a bare `*` right after `**`
  is refused on both sides.
- Re-seeding without $recursion also deletes the previous
  generation's own generated accessors; every pure validation of a
  re-seed runs before any teardown, so a failed re-seed leaves the
  element entirely on the previous generation.
- registry.ts split: pure path algebra in expand.ts, generation
  teardown in generation.ts, materialize.ts folded into getByAddress;
  the walk reuses the registry's concrete-path memo.
- Test harness shared through __tests__/helpers/recursionTestUtils.ts.

Editor / lint (vscode-wcs): coversSuffix / foldSuffixIndexes parity
for the forms above.

Docs: README (en/ja) diagnostics table, design doc §10 / §6-3,
set-all design §6-2, demo README, CHANGELOGs, impl-plan §7-9.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Runtime (@wcstack/state):
- The read-side materialization gate memoizes per interned PathInfo,
  symmetric with the write side; the duplicate string-keyed negative
  memo is gone.
- defineState types `this["nodes.**.value"]` and `this["nodes.**"]`
  as any (same index signatures as the editor preamble).
- An index segment inside the anchor or repeat sub-path
  (`{ "nodes.*": "children.0.*" }`) is wcs/recursion-declaration-invalid.
- IRecursionSpec carries anchorList / repeatList; findDescriptor is
  shared with State; unused exports and debug output removed.

Editor / lint (vscode-wcs):
- `$recursion: [...]` is reported; `${}`-free template literals are
  read as path arguments (this also reaches wcs/index-arity and
  getter-untracked-read).

Docs: README (en/ja) features and "not in this version", define-state
and typescript docs (en/ja), design doc D12 / §7-2, demo note,
CHANGELOGs rewritten as the final contract, impl-plan §7-10.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebuilt from packages/vscode-wcs so the committed wcs-validate matches
the editor diagnostics changed in review cycles 1-5. The smoke test
(18 cases, including the new spread-declaration case) passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mogera551
mogera551 merged commit 6297200 into main Sep 11, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant