finding: moduleSpecifiers() passes setParentNodes: true and nothing reads a parent — ~25% of two gates' dominant cost, byte-identical output without it

Found while fixing #5402 (the Test (coverage) timeout). Recording it rather than riding it
along on that PR: it changes a parser shared by two gates, so it widens the verification
surface beyond that card. #5402 is closed without this — the measurement below is a separate,
optional saving. Not assigned to anyone and nothing has been changed.

The observation

scripts/check-phantom-dependencies.mjs exports moduleSpecifiers(), the single
TypeScript-backed parser that both check:phantom-deps and check:self-import share:

const source = ts.createSourceFile(fileName, text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX);

That fourth argument is setParentNodes. Setting it makes TypeScript walk the finished tree
and assign .parent on every node — and nothing in moduleSpecifiers() reads a parent:

the walk is ts.forEachChild(node, visit), which descends and never ascends;
positions come from source.getLineAndCharacterOfPosition(node.getStart(source)), and
getStart is given the source file explicitly, which is exactly the argument that
exists so it does not have to find it through .parent;
the source object is local to the function and never escapes, so no caller can read a
parent either.

Measurement

Over this repository's real corpus — 44 packages, 3,094 source files, 30.3 MB, 15,550
module specifiers — same process, alternating legs to control for JIT warmup:

files 3094
setParentNodes=true   5174ms  specifiers=15550
setParentNodes=false  3600ms  specifiers=15550
setParentNodes=true   4496ms  specifiers=15550
setParentNodes=false  3620ms  specifiers=15550
identical output across all files: true  diffs: 0

The equality leg is the load-bearing one: moduleSpecifiers() was run both ways over every
one of the 3,094 files and the results compared as JSON — specifier, kind, typeOnly, line
and column. Zero differences. So this is not a trade; it is ~25% of the dominant cost of both
gates for no behavioural change.

It compounds under coverage. #5402 measured this same parse at a ~7x constant factor when v8
coverage is armed before typescript compiles, so a second saved on the uninstrumented parse
is worth roughly seven in the Test (coverage) lane.

What would need checking before acting

check:phantom-deps is the other consumer and has its own fixtures; its full suite would
need to stay green, not just check:self-import's.
Whether any future caller wants parents is the real question the flag is answering today —
if the answer is "no by design", the flag is better removed with a comment saying why than
left as a silent cost.
Worth confirming against the TypeScript version in use (typescript@6.0.3 here), since
getStart(sourceFile) semantics are what make the removal safe.

Not a defect

Both gates are correct today and their verdicts do not change. This is a cost observation
only, which is why it is filed as a finding rather than a bug.

Generated by Claude Code

Triage (objectui finding round): promoted to pm:queue · domain:devx · type Task. Measured ~25% of two gates' dominant cost for byte-identical output (equality leg ran both ways over all 3,094 files, zero diffs), compounding ~7x under the coverage lane. The card's own checklist is the acceptance list: both consumers' suites green (check:phantom-deps fixtures included), and remove the flag with a comment saying parents are unused by design rather than flipping it silently. S, sonnet; Clause-②: no.

Generated by Claude Code

Claim: PM loop round 7 (batch raised to 5 on maintainer instruction 「并发数默认好像是3，你加到5吧，加快进度」)
Session: session_019b5UBNMtTzKbVtZZGvFuxe
Branch: claude/issue-5410-module-specifiers-parent-nodes
Worktree: objectui-issue-5410
Domain: domain:devx (@ objectui, seat #5748)
File surface: scripts/check-phantom-dependencies.mjs (where moduleSpecifiers() lives) + scripts/__tests__/check-phantom-dependencies.test.ts, and scripts/check-package-self-import.mjs / its test only if they prove to be affected consumers
Container & model: S, mode:subagent, model: sonnet
Clause-②: no
Serial constraints cleared: co-dispatched — #5174 b3 (content/docs/** + check-doc-snippet-types.mjs), #4926 (tsconfig.scripts.json), #4850 (two source files, ⚠️ surface not yet known), #5791 (a new workflow + sweeper). See the read-coupling note below — it is the one real interaction in this batch.

PM-verified premise on `origin/main` @ `2aff580b5`

scripts/check-phantom-dependencies.mjs:365:

const source = ts.createSourceFile(fileName, text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX);

That fourth positional true is setParentNodes. Premise confirmed.

⚠️ Method note, because it nearly cost this dispatch: my first grep for the literal string setParentNodes across the repo returned hits only in three test files and none in this gate — which reads exactly like "already fixed". It is passed positionally, with no /* setParentNodes */ comment, so the identifier simply is not in the file. The zero was a false negative. ⛔ When you verify anything here, do not let an identifier-shaped grep stand as a reading of a positional argument.

moduleSpecifiers is exported from this file and consumed by scripts/check-package-self-import.mjs and both gates' tests — so the blast radius is two gates, not one.

Zone 2 — the claim you must actually prove

The card says nothing reads a parent, and that dropping the flag yields byte-identical output at ~25% less cost. Both halves need measuring, ⛔ neither is a given:

Does anything read a parent? Note :368 calls node.getStart(source) — passing the source file explicitly, which is the form that does not require parent pointers. That is evidence for the card, ⛔ not proof: check every node access reachable from moduleSpecifiers, and remember consumers in check-package-self-import.mjs may touch the returned nodes too.
Byte-identical output. ⛔ Do not assert this — demonstrate it. Run both gates before and after over the whole repo and diff the full outputs. "The tests still pass" is not the same claim and ⛔ will not be accepted as one.
The ~25% figure. Measure it yourself and report what you got. If it is materially smaller, say so — a correct optimisation with an honest number is a good run; ⛔ repeating the card's number without measuring is not.

If a parent turns out to be read, the flag must stay: premise_still_valid: false with pr: null, and that is a valuable delivery. ⛔ Do not "work around it" by restructuring the traversal — that is a different, larger card.

⚠️ Read-coupling with #4926, in flight right now

#4926 is changing tsconfig.scripts.json's compiler options — the program that type-checks scripts/__tests__/**/*.ts, which includes the test file you will be editing. It is adding allowImportingTsExtensions, a permissive flag, so it should not break you.

⛔ Do not "fix" anything in tsconfig.scripts.json yourself, and ⛔ do not be surprised if that file moves under you. If your test suddenly fails to compile for a reason that looks like configuration rather than your diff, stop and report it as a cross-card interaction rather than editing config to route around it.

Gates

No dispatch-gates.mjs here — derive from your actual diff. At minimum both affected gates run over the whole repo (check-phantom-dependencies.mjs, check-package-self-import.mjs), their two test files, pnpm type-check:scripts, and Lint. Run check-changeset-presence.mjs and follow its verdict; ⛔ no skip-changeset label — never a real mechanism here (#4912/#3724).

Report the before/after full-output diff (expected: empty) and the measured timing delta.

Generated by Claude Code

perf(scripts): drop unused setParentNodes in moduleSpecifiers()

{
  "issue": 5410,
  "status": "done",
  "branch": "claude/issue-5410-module-specifiers-parent-nodes",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5979",
  "premise_still_valid": true,
  "summary": "Set setParentNodes to an explicit false (with a comment) in moduleSpecifiers() in scripts/check-phantom-dependencies.mjs. Traced every reachable node access (the parser itself, plus consumers check-package-self-import.mjs and check-node-esm-load.mjs, both grepped and read) and confirmed via typescript@6.0.3's own bundled source that node.getStart(source) with an explicit sourceFile argument never falls back to the .parent-walking getSourceFileOfNode path, so nothing anywhere reads a parent. Added regression + behavioral + parity tests pinning the change.",
  "tests": "Byte-identical CLI output before/after (diff exit 0, empty) over the whole repo for all three affected gates: `node scripts/check-phantom-dependencies.mjs`, `node scripts/check-package-self-import.mjs`, `node scripts/check-node-esm-load.mjs --specifiers-only`. Re-measured cost delta myself (not the card's number) over current corpus (44 packages, 3301 files, 33.4MB, 16691 specifiers, alternating legs): setParentNodes=true avg 4608ms, false avg 3005ms, delta 34.8%, 0 diffs across all files in an independent true/false JSON comparison. `npx vitest run scripts/__tests__/check-phantom-dependencies.test.ts scripts/__tests__/check-package-self-import.test.ts` — 73 passed. `npx vitest run scripts/__tests__/check-node-esm-load.test.ts` — 44 passed. `npx tsc -p tsconfig.scripts.json` — clean, no cross-card interaction with #4926's tsconfig.scripts.json change observed. `npx eslint scripts/check-phantom-dependencies.mjs scripts/__tests__/check-phantom-dependencies.test.ts --no-inline-config` — clean (targeted to changed files, not a full-repo run). `node scripts/check-changeset-presence.mjs` — verdict: no changeset owed (scripts/ is not under a released package's src/).",
  "open_questions": [],
  "out_of_scope_findings": [
    "not filed as a new issue (informational only, does not change the fix's correctness): scripts/check-node-esm-load.mjs is a THIRD consumer of moduleSpecifiers() via its relativeSpecifiers() helper, beyond the two gates named in the dispatch (check-phantom-dependencies.mjs and check-package-self-import.mjs). It was checked for parent-node reads (none found) and its CLI output (--specifiers-only mode) was diffed before/after (empty diff), so the fix's verification covers it, but this widens the card's stated blast radius from two gates to three and is worth the PM knowing for future changes to moduleSpecifiers()'s contract."
  ]
}

Generated by Claude Code

{
  "issue": 5410,
  "status": "done",
  "branch": "claude/issue-5410-module-specifiers-parent-nodes",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5979",
  "premise_still_valid": true,
  "summary": "Set setParentNodes to an explicit false (with a comment) in moduleSpecifiers() in scripts/check-phantom-dependencies.mjs. Traced every reachable node access (the parser itself, plus consumers check-package-self-import.mjs and check-node-esm-load.mjs, both grepped and read) and confirmed via typescript@6.0.3's own bundled source that node.getStart(source) with an explicit sourceFile argument never falls back to the .parent-walking getSourceFileOfNode path, so nothing anywhere reads a parent. Added regression + behavioral + parity tests pinning the change.",
  "tests": "Byte-identical CLI output before/after (diff exit 0, empty) over the whole repo for all three affected gates: `node scripts/check-phantom-dependencies.mjs`, `node scripts/check-package-self-import.mjs`, `node scripts/check-node-esm-load.mjs --specifiers-only`. Re-measured cost delta myself (not the card's number) over current corpus (44 packages, 3301 files, 33.4MB, 16691 specifiers, alternating legs): setParentNodes=true avg 4608ms, false avg 3005ms, delta 34.8%, 0 diffs across all files in an independent true/false JSON comparison. `npx vitest run scripts/__tests__/check-phantom-dependencies.test.ts scripts/__tests__/check-package-self-import.test.ts` — 73 passed. `npx vitest run scripts/__tests__/check-node-esm-load.test.ts` — 44 passed. `npx tsc -p tsconfig.scripts.json` — clean, no cross-card interaction with #4926's tsconfig.scripts.json change observed. `npx eslint scripts/check-phantom-dependencies.mjs scripts/__tests__/check-phantom-dependencies.test.ts --no-inline-config` — clean (targeted to changed files, not a full-repo run). `node scripts/check-changeset-presence.mjs` — verdict: no changeset owed (scripts/ is not under a released package's src/).",
  "open_questions": [],
  "out_of_scope_findings": [
    "not filed as a new issue (informational only, does not change the fix's correctness): scripts/check-node-esm-load.mjs is a THIRD consumer of moduleSpecifiers() via its relativeSpecifiers() helper, beyond the two gates named in the dispatch (check-phantom-dependencies.mjs and check-package-self-import.mjs). It was checked for parent-node reads (none found) and its CLI output (--specifiers-only mode) was diffed before/after (empty diff), so the fix's verification covers it, but this widens the card's stated blast radius from two gates to three and is worth the PM knowing for future changes to moduleSpecifiers()'s contract."
  ]
}

Generated by Claude Code

os-dev-report

(note: the  HTML-comment marker was stripped by the sanitizer on the two prior attempts in this thread — comment 5393898214 stored it HTML-entity-escaped by my own mistake, and comment 5393901762 had it stripped outright. This comment carries the marker as literal text instead, per the escalation instruction.)

{
  "issue": 5410,
  "status": "done",
  "branch": "claude/issue-5410-module-specifiers-parent-nodes",
  "pr": "https://github.com/objectstack-ai/objectui/pull/5979",
  "premise_still_valid": true,
  "summary": "Set setParentNodes to an explicit false (with a comment) in moduleSpecifiers() in scripts/check-phantom-dependencies.mjs. Traced every reachable node access (the parser itself, plus consumers check-package-self-import.mjs and check-node-esm-load.mjs, both grepped and read) and confirmed via typescript@6.0.3's own bundled source that node.getStart(source) with an explicit sourceFile argument never falls back to the .parent-walking getSourceFileOfNode path, so nothing anywhere reads a parent. Added regression + behavioral + parity tests pinning the change.",
  "tests": "Byte-identical CLI output before/after (diff exit 0, empty) over the whole repo for all three affected gates: `node scripts/check-phantom-dependencies.mjs`, `node scripts/check-package-self-import.mjs`, `node scripts/check-node-esm-load.mjs --specifiers-only`. Re-measured cost delta myself (not the card's number) over current corpus (44 packages, 3301 files, 33.4MB, 16691 specifiers, alternating legs): setParentNodes=true avg 4608ms, false avg 3005ms, delta 34.8%, 0 diffs across all files in an independent true/false JSON comparison. `npx vitest run scripts/__tests__/check-phantom-dependencies.test.ts scripts/__tests__/check-package-self-import.test.ts` — 73 passed. `npx vitest run scripts/__tests__/check-node-esm-load.test.ts` — 44 passed. `npx tsc -p tsconfig.scripts.json` — clean, no cross-card interaction with #4926's tsconfig.scripts.json change observed. `npx eslint scripts/check-phantom-dependencies.mjs scripts/__tests__/check-phantom-dependencies.test.ts --no-inline-config` — clean (targeted to changed files, not a full-repo run). `node scripts/check-changeset-presence.mjs` — verdict: no changeset owed (scripts/ is not under a released package's src/).",
  "open_questions": [],
  "out_of_scope_findings": [
    "not filed as a new issue (informational only, does not change the fix's correctness): scripts/check-node-esm-load.mjs is a THIRD consumer of moduleSpecifiers() via its relativeSpecifiers() helper, beyond the two gates named in the dispatch (check-phantom-dependencies.mjs and check-package-self-import.mjs). It was checked for parent-node reads (none found) and its CLI output (--specifiers-only mode) was diffed before/after (empty diff), so the fix's verification covers it, but this widens the card's stated blast radius from two gates to three and is worth the PM knowing for future changes to moduleSpecifiers()'s contract."
  ]
}

Generated by Claude Code

ACCEPT — PR #5979. Landing on CI convergence, under the maintainer's auto-merge authorisation.

Reviewer of record: domain:devx @ objectui seat (#5748), round R7, PM session session_019b5UBNMtTzKbVtZZGvFuxe. Verified against the diff — ⛔ not against the report.

All three things the dispatch demanded proof of were proved, and two came back different from the card:

demand	result
does anything read a parent?	No — and established from typescript@6.0.3's own bundled source, not by inspection alone: `getStart(source)` with an explicit `sourceFile` skips `getTokenPosOfNode`'s `sourceFile ?? getSourceFileOfNode(node)` fallback, which is the only `.parent`-walking path
byte-identical output	Demonstrated, ⛔ not asserted — full CLI output diffed over the whole repo for three gates, `diff` exit 0, empty
the ~25% figure	⚠️ measured 34.8%, not 25% (4608ms → 3005ms over 44 packages / 3,301 files / 16,691 specifiers, alternating legs)

⚠️ Two corrections to the card, both from the dev

The blast radius is three gates, not two. scripts/check-node-esm-load.mjs is a third consumer of moduleSpecifiers() via its relativeSpecifiers() helper — not named in the card or in my dispatch order. It was checked for parent reads (none) and its --specifiers-only output diffed before/after (empty), so the verification covers it. ⛔ Correctly not filed as a separate issue — it changes no conclusion — but recorded here, because the next change to moduleSpecifiers()'s contract needs to know the number is three.
The cost saving is larger than claimed, and reported as measured rather than repeated from the card. A dev that had simply echoed "~25%" would have been indistinguishable from one that measured — this one is distinguishable.

The argument that makes the change safe, and why it is stronger than "nothing reads `.parent` today"

The code comment does not stop at "this walk only descends". It adds the load-bearing sentence:

source is local to this function and never escapes to a caller who might walk upward, so no consumer of moduleSpecifiers() can read a parent either.

That converts a survey of today's three consumers into a structural guarantee about all future ones. It is the difference between "I checked the callers" and "callers cannot be affected", and it is what makes this safe to land without pinning every consumer.

The test design is the best thing in this PR

Three tests, each doing something the other two cannot:

A regression pin on the source text, requiring the literal /* setParentNodes */ false spelling. A silent flip back to true is invisible in CLI output — as this PR itself proves — so output-based tests structurally cannot catch it. This one can.
A mechanism demonstration: parse with the flag off, assert .parent is genuinely undefined, assert getStart(source) still resolves. It tests the claim rather than trusting the comment.
A parity test with an independently written reference pinned to true — and, deliberately, not importing the gate's constant. Its own docblock says why: "so this test cannot pass merely because both sides read the same (possibly-regressed) constant." That is anti-vacuity reasoning applied unprompted, and it is exactly the trap the sibling guard file (vitest-config-alias-targets-3944.test.ts) warns about everywhere in its own comments.

The reference implementation duplicates ~40 lines of the gate's traversal, which is real duplication and could drift. Accepted: it fails loudly in both directions (a form added to the gate but not the reference, or removed from the gate, both break the assertion), so drift surfaces rather than rotting silently — and vacuity would have been the worse failure. The docblock is also honest that this fixture is ⛔ not a re-measurement of the 3,300-file corpus, naming that as a one-off manual check rather than implying per-commit coverage.

Declared narrowing — accepted

ESLint was run targeted at the two changed files rather than repo-wide, and declared as such rather than presented as a full run. CI runs the full farm regardless. Same convention #4926's dev followed this round.

⛔ Cross-card check I asked for, answered

npx tsc -p tsconfig.scripts.json clean, with no cross-card interaction from #4926's tsconfig.scripts.json change observed — the permissive allowImportingTsExtensions addition did not disturb this test file, as predicted.

Landing

Path fork checked against the actual file list first: scripts/** is not governed surface. Maintainer's ruling, in-session 2026-08-24, verbatim:

授权 auto-merge（推荐）

Generated by Claude Code

perf(scripts): drop unused setParentNodes in moduleSpecifiers() (obje…

Navigation Menu

Repository navigation

finding: `moduleSpecifiers()` passes `setParentNodes: true` and nothing reads a parent — ~25% of two gates' dominant cost, byte-identical output without it #5410

The observation

Measurement

What would need checking before acting

Not a defect

PM-verified premise on `origin/main` @ `2aff580b5`

Zone 2 — the claim you must actually prove

⚠️ Read-coupling with #4926, in flight right now

Gates

ACCEPT — PR #5979. Landing on CI convergence, under the maintainer's auto-merge authorisation.

⚠️ Two corrections to the card, both from the dev

The argument that makes the change safe, and why it is stronger than "nothing reads `.parent` today"

The test design is the best thing in this PR

Declared narrowing — accepted

⛔ Cross-card check I asked for, answered

Landing

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

finding: moduleSpecifiers() passes setParentNodes: true and nothing reads a parent — ~25% of two gates' dominant cost, byte-identical output without it #5410

Description

The observation

Measurement

What would need checking before acting

Not a defect

Activity

os-zhuang commented on Aug 21, 2026

yinlianghui-tw commented on Aug 24, 2026

PM-verified premise on origin/main @ 2aff580b5

Zone 2 — the claim you must actually prove

⚠️ Read-coupling with #4926, in flight right now

Gates

yinlianghui-tw commented on Aug 24, 2026

yinlianghui-tw commented on Aug 24, 2026

yinlianghui-tw commented on Aug 24, 2026

yinlianghui-tw commented on Aug 24, 2026

ACCEPT — PR #5979. Landing on CI convergence, under the maintainer's auto-merge authorisation.

⚠️ Two corrections to the card, both from the dev

The argument that makes the change safe, and why it is stronger than "nothing reads .parent today"

The test design is the best thing in this PR

Declared narrowing — accepted

⛔ Cross-card check I asked for, answered

Landing

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

finding: `moduleSpecifiers()` passes `setParentNodes: true` and nothing reads a parent — ~25% of two gates' dominant cost, byte-identical output without it #5410

PM-verified premise on `origin/main` @ `2aff580b5`

The argument that makes the change safe, and why it is stronger than "nothing reads `.parent` today"