Skip to content

fix(spec): re-spell step17.rationale as a joined fragment array, collapsing registry.ts AST depth 977 to 76 - #10446

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10122-registry-rationale-depth
Aug 20, 2026
Merged

fix(spec): re-spell step17.rationale as a joined fragment array, collapsing registry.ts AST depth 977 to 76#10446
os-zhuang merged 1 commit into
mainfrom
claude/issue-10122-registry-rationale-depth

Conversation

@qq9340100

@qq9340100qq9340100 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Fixes#10122

step17.rationale in packages/spec/src/migrations/registry.ts was a single
left-nested + chain of 971 string literals. @typescript-eslint/parser builds
its ESTree AST by recursion, so what it costs is depth, not size — that one
expression gave the whole file a max AST depth of 977, roughly 13x the repo's
runner-up, and put it over V8's default main-thread stack.

Re-spelled as an array of the same literals joined with .join('').

Before / after, measured on this branch

before (origin/main @ 6439f8b42)after (192d3c58e)
registry.ts max AST depth97776
step17.rationale initializer subtree depth9714
step17.rationale initializer kindBinaryExpressionCallExpression
longest + chain in the file971 operands, L397–136770 operands, L4968–5037 (step18.rationale, untouched)
min --stack-size for eslint to parse the file1099 KB104 KB
parses at V8's default stack (984 KB, no flag)?no — cliffsyes

Depth convention: SourceFile = depth 1, walked with ts.forEachChild. The card
measured 976 / 970 operands at 1800ffac2; the file has since grown by one
fragment, and the numbers above are re-measured on current main.

--stack-size method: binary search over
node --stack-size=N node_modules/eslint/bin/eslint.js PATH --no-inline-config,
1 KB resolution, cliff detected by the literal Maximum call stack size exceeded
string. Node v22.22.2. The 1099 KB "before" figure sits close to the 1085 KB
recorded in eslint.config.mjs — the difference is the one fragment appended
since, at the ~1.10 KB/fragment cost that comment measured.

After the change the file needs 9.5x less stack than V8 gives it by default,
where before it was ~12% over budget and could only parse on borrowed headroom.

The acceptance criterion: the string value is unchanged

The 971 fragments are carried over byte-for-byte — same quoting (941 single,
30 double), same escapes, one fragment per line. Only the + separators became
,, plus the [ / ].join('') wrapper.

Proven three ways, all mechanical:

  1. Source-level, pre-commit. Both spellings evaluated and compared with strict
    === plus sha256 — 79328 chars, 2099de977992696945f00d772042b27c3be79135d2a4d44109433973f979de1f, identical.
  2. Independent re-check on the committed artifacts. The initializer was
    re-extracted from origin/main's file and from this branch's file via the
    TypeScript AST and re-evaluated: same length, same sha256, strict === : true.
  3. At runtime, out of the built package.import('packages/spec/dist/index.mjs')
    then MIGRATIONS_BY_MAJOR[17].rationale79328 chars, sha256
    2099de97…de1f. The value a consumer actually receives is the same string.

A gap check also ran before the rewrite: every one of the 970 gaps between
consecutive operands was asserted to be whitespace plus a single +, so no comment
or non-literal operand could be silently dropped. All 971 operands are plain
StringLiteral (0 other kinds).

Nothing else moved

The rewrite replaced exactly the byte range [19762, 108025) — the initializer —
and the surrounding bytes were asserted unchanged on both sides. git diff is a
single hunk, @@ -397,971 +397,973 @@.

  • The os-generated marker regions are untouched. step17's markers are at L1432 and
    L4926; the chain ended at L1367, entirely outside them.
  • Other steps' rationales, conversionIds, RETIRED_KEYS_BY_MAJOR and
    RETIRED_DEFS_BY_MAJOR are byte-identical.
  • The custom merge driver's marker structure is preserved — pnpm check:merge-driver
    is green.

A template literal was considered and rejected: the content contains backticks
(`execute`, `target` …) and ${-hostile text, so it would have required
escaping inside every fragment — which is exactly the byte-level churn this change
is trying not to introduce. The .join('') spelling keeps each literal verbatim.

Gates

Union re-derived from the actual diff with node scripts/pm/dispatch-gates.mjs
(no paths — it reads the change set from the merge base itself) and run at
192d3c58e, the final commit.

The two that this card is about, both green:

=== pnpm lint :: EXIT=0
=== check:slot-lookup :: EXIT=0

pnpm lint is the whole-repo run — the one that was returning
registry.ts 0:0 error Parsing error: Maximum call stack size exceeded.

Also green at 192d3c58e: check:merge-driver, check:changeset-gate-self-tests,
check:cross-package-test-inputs, check:spec-parsed-alias,
check:type-source-resolution, check-adr-0087-registration,
check-cross-package-test-inputs, check-affected-docs, and the spec-liveness
family (check:empty-state, check:liveness, check:strictness-ledger,
check:variant-docs), plus pnpm --filter @objectstack/spec typecheck and
pnpm --filter @objectstack/spec test.

check:doc-formula-expressions red on first pass with
ERR_MODULE_NOT_FOUND … @objectstack/formula/dist/index.mjs — a missing build
artifact in a fresh worktree, not this diff. After
pnpm --filter '@objectstack/formula...' build it is green:

✓ check:doc-formula-expressions: 22 record-scoped formula example(s) across 407 files / 1416 TS blocks judged clean

One declared narrowing:node scripts/check-dev-prereqs.mjs was not brought
green locally. Its own verdict names the cause and it is not this diff —
✗ The workspace is not built — 1 unmet precondition, not a list of problems. / 66 of 67 workspace packages declare an entry point under dist/ that is not on disk. Clearing it needs a full pnpm build of the workspace, which would hold
the shared verify lock for a long time on a container running several agents; CI
builds the workspace and runs this gate anyway.

No changeset — deliberate, and here is what dist actually contains

registry.ts does ship in the published @objectstack/spec, and its emitted
bytes do change
: dist/index.js / dist/index.mjs are not minified, so they
carry the source spelling verbatim — rationale: [ … ].join('') where they
previously carried the + chain.

What does not change is anything a consumer can observe: the exported string is
the same 79328 characters with the same sha256 (verified out of the built artifact,
above), no export was added or removed, and no type changed. Per AGENTS.md — "Add a
changeset for feature work … Pure bug fixes do not require a changeset"
— this is a
repo-tooling fix with no functional delta, so it carries the skip-changeset label
instead of declaring a release of its own.

For the closers of the duplicate reports

#10071 and #10121 report the same signature and were already triaged as
duplicates of the card this PR addresses; they remain open or closed at their
own triagers' discretion — nothing here changes their state. The evidence
recorded on the card at 18:45Z today — PR #10396
dequeued from the merge queue with reason: CI_FAILURE while its own head was
green (24 checks, 0 failing, mergeable_state: clean), the queue build failing on
packages/spec/src/migrations/registry.ts:0:0 — Parsing error: Maximum call stack size exceeded for a file not in that PR's diff — is the collateral-eviction mode
this removes. #10030 raised the parser's headroom to --stack-size=4000; this
lowers the floor instead, from 1099 KB to 104 KB.

Two things deliberately left alone

  1. eslint.config.mjs still says --stack-size=4000, and its comment block
    (L31–91) still describes the pre-fix state.
    Whether that headroom should now be
    lowered or kept as margin is a real decision with a measurement attached, not a
    mechanical follow-on, and this PR's file surface is one file by claim. Filed
    separately as [finding] Once the registry.ts depth fix lands, eslint.config.mjs's --stack-size=4000 and its 60-line rationale describe a state that no longer exists #10448 (unassigned), which lays out both dispositions.
  2. step18.rationale is a 70-operand chain (file depth 76). That is the same
    authoring habit at harmless scale — the repo's runner-up across all 4659 linted
    files was already 71 — and the card explicitly scopes the smaller same-shape
    chains out.

Generated by Claude Code

…apsing AST depth 977 to 76
`packages/spec/src/migrations/registry.ts` held `step17.rationale` as a single
left-nested 971-operand `+` chain, giving the file a max AST depth of 977 —
about 13x the repo's runner-up (71). @typescript-eslint/parser recurses over
the AST, so what it costs is depth, not size: the file needed a minimum
--stack-size above V8's 984 KB default to parse at all, and cliffed with
`0:0 error Parsing error: Maximum call stack size exceeded` depending on which
other files shared the eslint invocation.
Re-spelled as an array of the same string literals joined with .join(''). The
971 fragments are carried over byte-for-byte (same quoting, same escapes);
the resulting string value is asserted identical — 79328 chars, sha256
2099de977992696945f00d772042b27c3be79135d2a4d44109433973f979de1f — before
and after. step17.rationale's own subtree depth goes 971 -> 4.
Nothing outside the initializer's byte range moved: the generated
<os-generated> regions, other steps' rationales, conversionIds and the two
retirement tables are untouched.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gcKVsiywU9CcS96S5t9qD
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 3 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx(via /api/v1/apps (route))
  • content/docs/automation/hook-bodies.mdx(via crypto.hash (literal))
  • content/docs/getting-started/quick-reference.mdx(via /api/v1/apps (route))
  • content/docs/protocol/kernel/http-protocol.mdx(via /api/v1/apps (route))
  • content/docs/protocol/objectui/widget-contract.mdx(via my_picker (literal))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 116 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 91f303cb4bfa47bc3526633f5275d0aafc5aad24packageMentionDocs.

Which tree this was computed on

This run read content/docs from 95d8adee5003c8b3faaf7588aeacc0a9f474214e — the merge of head 192d3c58e71f5fcfdb90a22cecb2dafba4893d3e into base 91f303cb4bfa47bc3526633f5275d0aafc5aad24, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 95d8adee5003c8b3faaf7588aeacc0a9f474214e && git checkout 95d8adee5003c8b3faaf7588aeacc0a9f474214e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 91f303cb4bfa47bc3526633f5275d0aafc5aad24 192d3c58e71f5fcfdb90a22cecb2dafba4893d3e && git checkout -B drift-repro 91f303cb4bfa47bc3526633f5275d0aafc5aad24 && git merge --no-ff 192d3c58e71f5fcfdb90a22cecb2dafba4893d3e
node scripts/docs-audit/affected-docs.mjs --json 91f303cb4bfa47bc3526633f5275d0aafc5aad24

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 91f303cb4bfa47bc3526633f5275d0aafc5aad24 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@qq9340100qq9340100 added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 20, 2026 — with Claude
@qq9340100
qq9340100 marked this pull request as ready for review August 20, 2026 23:57
@os-zhuang
os-zhuang merged commit f094214 into mainAug 20, 2026
37 of 38 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10122-registry-rationale-depth branch August 20, 2026 23:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xlskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@qq9340100@os-zhuang