docs(scripts): point the fixture justifications at the AST walk that replaced the regex - #7921
Merged
baozhoutao merged 1 commit intoSep 6, 2026
Conversation
…replaced the regex (objectui#6996) Six test suites under `scripts/__tests__/` justified how they spell their fixtures by describing `scripts-type-check.test.ts` as a TEXT matcher over `scripts/` that "cannot tell a string literal from an import". objectui#4902 replaced that regex with `workspaceImportSpecifiers()`, an AST walk over `ImportDeclaration`/`ExportDeclaration`/`ImportEqualsDeclaration` nodes, and pinned the string-literal case directly — so every one of those eight statements described a matcher that no longer exists. A wide sweep (not just the two sites found by exact wording) is what settled the shape: with six copies of the description, correcting each one in place just leaves six things that can drift again. So the authoritative account stays in `workspaceImportSpecifiers()`'s own docstring, that docstring now says siblings should point at it rather than restate it, and the six sites became pointers. The interpolation constants (`I18N_PKG` in both i18n suites) are KEPT, with corrected justifications. Measurement, not assumption: no live text-level reader of `scripts/` matches an import edge from text. `check-phantom- dependencies`, `check-unreferenced-sources`, `check-package-self-import` and `check-shell-escape-residue` all exclude `scripts/`; `check-pre-install- import-graph` and the `check-vi-mock-*` pair read text but mask comments and literals via `js-comment-mask.mjs`; the live `from '...'` regexes in `e2e-type-check.test.ts` and `vitest-setup-type-check.test.ts` read the e2e and vitest-setup programs, not `scripts/`. A probe that inlined the specifier at all 41 fixture sites and dropped both constants left every one of those readers green, `scripts-type-check.test.ts` included. The constants therefore survive on their own merit — one spelling across the fixtures — not as a workaround, and the fixtures are template literals either way, so nothing is paying an interpolation tax. No gate behaviour changes: comments only. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013uAaxiwgYDybsTNV9xwa1M
baozhoutao
marked this pull request as ready for review
September 6, 2026 03:24
baozhoutao
deleted the
claude/issue-6996-stale-regex-justification-comments
branch
September 6, 2026 03:50
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.
Fixes #6996
Six test suites under
scripts/__tests__/explained how they spell their fixtures by describingscripts-type-check.test.tsas a text matcher overscripts/— one that "greps this directory for import statements" and "cannot tell a string literal from an import". objectui#4902 replaced that regex withworkspaceImportSpecifiers(), an AST walk overImportDeclaration/ExportDeclaration/ImportEqualsDeclarationnodes, and pinned the string-literal case directly. Every one of those statements described a matcher that no longer exists.The sweep decided the shape
The card found two sites by exact wording. The triage comment made the wide sweep a precondition for deciding anything else, and it changed the answer: eight statements across six files, not two.
check-i18n-dead-keys.test.tscheck-i18n-call-site-keys.test.ts(x2)check-i18n-en-drift.test.tscheck-doc-component-types.test.tscheck-package-self-import.test.ts(x2)check-spec-symbol-derivation.test.tsProbes run (positive control first, so a silent zero-hit sweep is distinguishable from a clean one): the card's exact wording (returns the two known sites),
greps this directory, every mention ofscripts-type-check, every citation of4902, every mention ofworkspaceImportSpecifiers, grep-plus-import prose repo-wide,cannot tellscoped toscripts/, and "needs no build" prose repo-wide. Nothing outsidescripts/__tests__/describes the gate — docs andAGENTS.mdare clean.With six copies, correcting each in place leaves six things that can drift again. So: the authoritative account stays where it already was —
workspaceImportSpecifiers()'s own docstring — that docstring now says siblings should point at it rather than restate it, and the six sites became pointers.The constants are KEPT, and that was measured
Decision (a) from the card was whether
I18N_PKGand its twin die with the comments or survive with a corrected reason. Two independent measurements, because the triage forbids removing a constant a live reader depends on:Enumeration — every text-level reader of
scripts/, derived frompackage.json'scheck:*scripts and.github/workflows/, not from a hand-list:check-phantom-dependencies,check-unreferenced-sources,check-package-self-import,check-shell-escape-residue— scan roots excludescripts/(confirms the card's one checked gate, and extends it to the set).check-pre-install-import-graph,check-vi-mock-specifiers,check-vi-mock-inherit— do readscripts/textually, but mask comments and literals throughjs-comment-mask.mjs, so a specifier in a string is not an edge to them either.check-entry-guard,ci-cd-pipeline-doc.test.ts— readscripts/for other tokens entirely.e2e-type-check.test.ts:306andvitest-setup-type-check.test.ts:138— the only surviving rawfrom '...'regexes in the tree. They read the e2e and vitest-setup programs, notscripts/. These are the closest thing to the reader the comments imagined, and they look elsewhere.Probe — inlined the specifier at all 41 fixture sites and deleted both constants, proved the mutation reached disk, then ran the readers. All green,
scripts-type-check.test.tsincluded (15/15). Restored,git diff HEADempty.So nothing forces the constants. They are kept on their own merit: one spelling across the fixtures, one edit on a package rename. The fixtures are template literals either way, so there is no "interpolation tax" being paid at the call sites — the only thing the old comment cost was a false belief.
No gate behaviour change
Comments only, in all seven files. No constant removed, no assertion touched, no scan root moved.
Gates (all at
fad24dc6f)vitest run --maxWorkers=2Test Files 7 passed (7),Tests 348 passed (348); lock wrapperVERDICT command-exit 0pnpm type-check:scriptspnpm lint:root33 problems (0 errors, 33 warnings), all pre-existing)pnpm check:control-bytescheck-control-bytes: OK (scanned 6400 tracked text file(s); skipped 85 binary)pnpm check:i18n-keyspnpm check:i18n-dead-keyspnpm check:i18n-driftpnpm check:self-importNo package names itself inside its own src/.pnpm check:spec-symbolsspec member citations: no comment cites a key its spec symbol does not declare.node scripts/check-changeset-presence.mjsNo source or published contract of a released package changed in this range, so no changeset is owed.check-governed-queue-guard --testNOT GOVERNED — 7 path(s) checked against 5 governed surface(s); none matched.Draft, no auto-merge.
Generated by Claude Code