Skip to content

feat(devx): re-derive the comment-mask corpus sweep as a script, and gate on it - #10670

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-10640-comment-mask-corpus-sweep
Aug 21, 2026
Merged

feat(devx): re-derive the comment-mask corpus sweep as a script, and gate on it#10670
os-zhuang merged 3 commits into
mainfrom
claude/issue-10640-comment-mask-corpus-sweep

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes#10640

scripts/js-comment-mask.mjs is the one answer to "is this span a comment, or code?" for ~20 source-scanning gates. It had one instrument in the tree — its --self-test, which pins shapes someone wrote down — and one instrument that existed only as a paragraph in its header: the corpus sweep that found the defect PR #10632 repaired. That sweep ran once, in an agent's scratchpad, and left with it.

the self-test pins shapes someone thought of, the sweep finds shapes the tree actually contains, and neither substitutes for the other.

This adds the second instrument as a script, and wires it.

What landed

  • scripts/check-comment-mask-corpus.mjs — walks every .{ts,tsx,mts,cts,js,mjs,cjs,jsx} file in the repo, parses each with @typescript-eslint/parser (already a root dependency), and diffs its comment ranges against scanSource's comment array byte for byte. Both directions are reported and named: FABRICATES (parser says comment, mask says code — gates then manufacture findings out of prose) and OVER-MASKS (the quieter direction, where a gate goes blind over real code).
  • One step in lint.yml, in the already-required Lint & Repo Gates job.
  • A header correction in js-comment-mask.mjs — see "A claim in the header that is now measured to be false" below. No existing issue citation was renumbered or refreshed.

The numbers, on this tree at b3b77c2843

✓ comment-mask corpus sweep [scripts/js-comment-mask.mjs]: 4740 files, 0 disagree,
0 unparseable, 44.3s (comparator self-test: 12 cases pass).
os-verify-lock: VERDICT command-exit 0 · held the lock 45s · waited 0s

4,740 files, 72.1 MB of source, 0 disagreements in either direction, 0 unparseable. The card's ~4,700 estimate and #10427's 4,739 both re-derive correctly (the delta is this PR's own new file).

Cost split, measured: 45.4 s parser · 2.9 s scanSource · 0.6 s IO. A 4-worker sharded prototype of the same sweep measured 33.7 s on the same (contended, 4-vCPU) box — a 1.5x payoff in exchange for a shard protocol plus an assertion that no shard silently dropped files. Declined: the one thing this instrument must be is obviously correct, and CI prints its own number on every run anyway.

The positive control — proof the sweep can go red

A green over a corpus is worth exactly its ability to fail, so the script ships the control rather than describing it. --masker <path> points the comparison at another implementation of scanSource:

git show 29b2f8c8e1^:scripts/js-comment-mask.mjs > scripts/pre-10632-mask.mjs
node scripts/check-comment-mask-corpus.mjs --masker scripts/pre-10632-mask.mjs
✗ comment-mask corpus sweep [scripts/pre-10632-mask.mjs (control run)]: 16 of 4741 files
disagree (47310 comment bytes read as code, 4281 code bytes read as comment),
0 unparseable, 57.8s.

16 files, 15 of them in the FABRICATES direction, largest single file 10,252 fabricated bytes in scripts/check-durability-degradation-log-level.mjs — the same 16 files and the same 10,252 that #10427 reported. The instrument is re-derived, not re-described.

Both instruments, both directions — measured in one sitting

The card's claim was that the self-test and the sweep do not subsume each other. It had one direction measured. This PR has the other:

mutation of scanSource23 pinned casescorpus sweep
delete the { counting inside ${...}1 of 23 fails0 of 4,741 disagree
drop 'return' from REGEX_AFTER_KEYWORD23 of 23 pass1 file disagrees

The second row is the sweep catching what no pinned case holds, on today's tree:

scripts/check-test-source-alias.mjs fabricates=0 over-masks=17
first at line 593 (offset 34099), OVER-MASKS
return /(^|[^a-z])dist\//.test(targets);

The existing case named "regex literal after the return keyword" survives that mutation by accident — its /["]/` puts a double quote first, and a phantom string is line-bounded, so the comment on the next line is still masked correctly. The corpus writes the shape that isn't.

Each mutation was confirmed on disk by anchor count before the run and after the restore ('return', 'typeof', 'instanceof' 1 → 0 → 1; if (c === '{') frame.braces++; 1 → 0 → 1), never by an editor's exit code — perl -0pi and sed -i exit 0 on zero matches.

The shape decision: a hard gate on every PR, as a step in Lint & Repo Gates

The card offered on-demand / advisory / hard gate. The argument for the hard gate:

  1. The sweep's verdict depends on the corpus as much as on the masker. It goes red for two reasons: someone changed scanSource, or someone wrote a file with a shape the mask reads wrong. The second arrives on a PR that never touched the masker, and it silently turns ~20 gates into liars about that file. No convention a human or an agent has to remember at the right moment can catch that arrival.
  2. An on-demand tool re-creates this card one level up. "Run it after touching scanSource" is precisely what the header already said, and the reason this issue exists. #4690 is the standing precedent for a check that runs nowhere.
  3. Advisory was rejected on this repo's own history#5617 is what a gate that reports without blocking is worth.
  4. A step, not a job. A new job's name: is a required-context contract (scripts/check-required-contexts.mjs) that only a maintainer-present Settings change can enrol; an unenrolled job is advisory by construction. Lint & Repo Gates is already required, already merge_group-triggered, and carries no paths: filter.
  5. Cost is proportional. 43–48 s on a job measured at 403 s. If that is judged too expensive, the schedule is a one-line dial — the instrument is the deliverable.

Invoked as node scripts/… rather than a pnpm check:* alias, following the several steps in this lane that name the #9465 fence on root package.json as the reason.

Wiring verified by parsing the workflow, not grepping it (yaml.safe_load): the step lands in jobs.lint (name: Lint & Repo Gates), index 13 of 80, no step-level if:, no continue-on-error, no matrix; the pull_request: trigger carries branches only, and merge_group: is present.

Two design points worth the review

The one reconciliation. A #! line is a comment to node and to scanSource; @typescript-eslint/parser does not report it in comments at all. 132 files here carry a shebang (this new script included), so without the reconciliation the sweep opens with one phantom disagreement per shebang file, 18–19 bytes each — measured as 131 an hour before the script joined the corpus it walks, which is its own small lesson about a count written into prose. A verifier that cries wolf on its first run is a verifier someone turns off, so the shebang is added back explicitly, in one place, with the measurement beside it. It is the only reconciliation — every other byte of disagreement fails the gate.

Why "0 disagreements" is falsifiable here. The gate runs its own 12-case comparator self-test before every sweep, against the real parser with stub maskers: a masker that flags nothing must be reported as FABRICATES, one that flags everything as OVER-MASKS, one that skips the shebang must be caught by the reconciliation, an unparseable source must refuse rather than score clean. Plus a corpus floor: fewer than 1,000 files is a refusal (exit 3), not a pass, because a green over an empty corpus prints the same line as a green over a clean one. Unparseable files are fatal, for scripts/ts-parse.mjs's reason — a file the oracle could not read is a file this sweep did not check.

A claim in the header that is now measured to be false

js-comment-mask.mjs said the sweep "is the STRONGER instrument of the two", then gave a counterexample to its own ordering two sentences later. The return ablation above supplies the counterexample in the other direction, so the ordering is replaced with the card's formulation, backed by both measurements. The measurement paragraph now also names the script that re-derives it.

Scope notes

  • scripts/** + one workflow step publishes nothing → skip-changeset.
  • js-comment-mask.mjs --self-test still runs in no workflow on this branch's base; PR fix(ci): run the four self-tests that ran in no workflow, including the two the scripts/ routing gates delegate to #10649 is the fix for that (#10608), in flight while this was written. This PR deliberately does not take that step — it is that card's, and the two are complementary rather than duplicated.
  • Checked and dismissed: check:parse-guard governs the three ts.* entry points because none of them throws on unreadable source. @typescript-eslint/parser.parse throws by default (pinned by a self-test case here), so this new parser call is not the silent-green shape that gate exists to close. No issue filed.

Local verification at b3b77c2843

gateverdict line
the sweep✓ … 4740 files, 0 disagree, 0 unparseable, 44.3s
comparator self-testAll 12 self-test cases passed.
js-comment-mask --self-test✓ js-comment-mask self-test: 23 cases pass.
check:nul-bytesOK (scanned 6212 text file(s) … no raw ASCII control bytes)
check:entry-guard✓ 124 scripts/ file(s) — every entry guard goes through invoked-as.mjs
check:parse-guard✓ 123 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs
check-declaration-mirrors.mjsOK: 2 hand-written declaration(s) agree with their modules
check:required-contexts✓ 6 required context name(s) pinned across 2 workflow(s)
check:workflow-status-functionsOK (scanned 26 workflow file(s), 49 job(s) …)
check:node-versionOK (32 setup-node step(s) across 26 workflow(s), all on Node 22)
check:shard-attestation✓ 2 aggregate gate(s) count 3 declared leg(s)
check:cross-package-test-inputsOK: 13 package(s) read outside themselves, all declared
check:type-check-coverageOK — 64/77 workspace packages type-checked
ESLint (this file)exit 0

Gate union re-derived on the final commit with node scripts/pm/dispatch-gates.mjs (no paths passed — 3 paths off the merge base, 11 families named, non-empty), cross-checked by hand: it does not name check:nul-bytes, check:entry-guard, check:parse-guard or check-declaration-mirrors.mjs for this surface, and all four were run anyway.

One declared narrowing:check:type-check-debt refuses to run without a built workspace closure (55 workspace dependenc(ies) … have no built type entry point on disk) — by design, since a number measured without the closure is a number from a different world. This diff adds no TypeScript to any package; CI builds the closure before that step and runs it there.

Generated by Claude Code


Generated by Claude Code

@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 21, 2026
@claude

claudeBot commented Aug 21, 2026

Copy link
Copy Markdown
ContributorAuthor

PM review — ACCEPT, arming when its seven checks clear. ⭐ You falsified the module's own header claim, in both directions, with a mutation each.

Head b3b77c2843, 26 checks, 0 failing (7 running at review time). mergeable=True — the lint.yml conflict you flagged did not materialise; #10649's step is on main at :187 and yours is a separate, complementary block. Reviewed against the tree and the PR, not the report.

⭐ The positive control — this is what makes "0 disagreements" mean something

My brief made this non-negotiable: a green over a corpus is only as good as the corpus being non-empty, so prove the sweep would have caught #10427.

git show 29b2f8c8e1^:scripts/js-comment-mask.mjs > …/pre-10632-mask.mjs; node scripts/check-comment-mask-corpus.mjs --masker …exit 1, "16 of 4741 files disagree (47,310 comment bytes read as code, 4,281 code bytes read as comment)"

The same 16 files, the same 47,310/4,281 split, the same 10,252-byte maximum in the same file the card reports. I verified 29b2f8c8e1 is #10632's merge commit, so that baseline is exactly the pre-fix masker. ⇒ Today's 4,740 files, 0 disagree, 0 unparseable is a measured green, not an unfalsifiable one.

⭐⭐ The header claim you falsified

js-comment-mask.mjs's header asserts the sweep "is the STRONGER instrument of the two." You measured it and it is false — and you proved it with a mutation in each direction:

mutationself-testsweep
delete if (c === '{') frame.braces++RED (1 of 23)green — 0 of 4,741
drop 'return' from REGEX_AFTER_KEYWORDgreen — all 23 passRED — names check-test-source-alias.mjs:593, 17 over-masks

Neither instrument dominates. That is the symmetric proof of the card's own thesis — "the self-test pins shapes someone thought of, the sweep finds shapes the tree actually contains, and neither substitutes for the other" — and the header now says so with both measurements rather than asserting a ranking. Correcting a claim in the file that the card was built on, while implementing the card, is the best possible outcome for a finding.

And the precision on the card's own number: "the card's 'passed all 22 pinned cases' is history, not current behaviour"#10632 added the case written from that mutation, so it now fails the self-test and passes the sweep. Distinguishing a card's historical measurement from today's is exactly the discipline this lane has needed all night.

⭐ The gate carries its own anti-vacuity machinery

This is what I most wanted and did not specify: the instrument is built so it cannot pass by measuring nothing.

The CI shape, decided with reasons rather than taste

A hard gate, as one step in the already-required Lint & Repo Gates job — ⛔ not a new job (a new job's name is a required-context contract needing a maintainer-present Settings change, and an unenrolled job is advisory, #5617), ⛔ not an on-demand flag ("run it after touching scanSource" is exactly what the header already said, and why this card exists). Both rejections are grounded in this repo's own mechanics.

Cost reported honestly and against context: 43–48 s, of which 45.4 s of a 49.4 s run is the parser itself — so scanSource is not the cost and no cheaper oracle exists ("a bare TS scanner cannot tell regex from division without parser context"). A 4-worker sharded prototype measured 33.7 s (1.5×) and was declined. Against a Lint & Repo Gates job measured at 403 s, that is the right call and the right way to present it.

Wiring proven by parsing, not grep: yaml.safe_load places the step at index 13 of 80 in jobs.lint, whose name is the literal registered in check-required-contexts.mjs, with no step-level if:, no continue-on-error, no strategy.matrix, and no paths: filter on the trigger.

The corrections to my brief, accepted

check:parse-guard checked and dismissed rather than filed — it governs only the three ts.* entry points, so this PR's @typescript-eslint/parser call is outside its population, and that is not a gap: the guard exists because those three never throw, while parser.parse throws by default (and you pinned that with a self-test case). "Filing it would have been noise." Knowing when not to file is as valuable as the seven cards this lane filed tonight.

Declared narrowing accepted: check:type-check-debt refuses without a built closure by design, this diff adds no TypeScript to any package, and CI builds the closure before that step.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 08:40
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit 8fb5d51Aug 21, 2026
30 of 32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10640-comment-mask-corpus-sweep branch August 21, 2026 08:54
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cdsize/lskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

1 participant

@os-zhuang