fix(state,vscode-wcs): recursive-path quality pass after five review cycles - #261
Merged
Merged
Conversation
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 indocs/state-recursive-path-impl-plan.md§7-4 to §7-10.Runtime (
@wcstack/state)nodes.1.total,nodes.*.children.0.total), a**suffix, or a path inside the getter's value now raisewcs/recursion-readonlybefore the walk. Before, the value landed on the node object and was pinned as the getter's cached result, corrupting the aggregates above it.<list>.lengthand index spellings of child nodes and child lists.**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 raisewcs/recursion-declaration-invalid; a malformed**suffix raiseswcs/recursion-anchor.**takes its depth from the innermost frame only. A plain getter reached from a recursive getter reportswcs/recursion-contextinstead of a rawListIndex not foundor a silently wrong value.**:$trackDependency,$postUpdate,$listKeyskeys. A non-arrayindexeson$getAllthrows a diagnostic instead of aTypeError(this also applies to plain$getAll; listed under Changed).bind-component) that declares a**getter getswcs/mount-dollar-declaration.PathInfo. Unrelated writes on a state with$recursionwent from up to 1.5x to 1.02-1.09x of a plain state; deep reads from +123 ns to +42 ns.defineStatetypesthis["nodes.**.x"]andthis["nodes.**"]asany, matching the editor preamble.expand.ts, generation teardown togeneration.ts,materialize.tsfolded intogetByAddress; the recursion test harness is shared through__tests__/helpers/recursionTestUtils.ts.Editor and
wcs-validate(vscode-wcs,@wcstack/lint)lengthand structural getter keys; concrete collisions;++/--; assignments,$postUpdate,$trackDependency,$listKeys; non-array$getAllindexes;$recursioninside volumes andbind-componentblocks.classstates, top-level spreads, computed keys, non-literal repeat values.${}-free template literals are read as path arguments. This also reacheswcs/index-arityandgetter-untracked-read, so existing pages can get new diagnostics (noted in the root CHANGELOG).packages/lint/dist/cli.cjsis regenerated; the smoke test gained a spread-declaration case.Known defects (unchanged, pinned in
integration.recursionKnownDefects.test.ts)Tests
packages/statetsc --noEmitcleanpackages/vscode-wcstsc --noEmitcleanpackages/lintpackages/state/distis not rebuilt (tracked; the release workflow rebuilds it).Follow-ups (not in this PR)
🤖 Generated with Claude Code