Repository navigation
refactor(devx): share the regex-literal walk, keep the position rule a parameter - #15532
Conversation
…a parameter
`scripts/js-comment-mask.mjs` and `scripts/check-dispatcher-error-vocabulary.mjs`
each carried a regex-literal recogniser: a byte-identical 14-word
`REGEX_AFTER_KEYWORD` set, an `IDENT_CHAR` class, and a body walk. The keyword
set is not decorative — js-comment-mask's own header records that dropping
`return` from it passes all 23 of its self-test cases and is caught only by the
corpus sweep, so the copy that drifts drifts silently in exactly the direction
its owner warns about.
Split by NUMBER OF RIGHT ANSWERS rather than by module:
the WALK has one (ECMA-262's), so it lives once — `walkRegexBody`, exported
from js-comment-mask.mjs alongside `IDENT_CHAR` and `REGEX_AFTER_KEYWORD`;
the POSITION RULE has two, because the consumers fail in deliberately
opposite directions — a masker over-masks safely, a scanner must never skip
a span it invented — so it is a parameter (`makeRegexRecogniser`), with no
default and a throw when one is omitted.
`walkRegexBody` returns `{ end, closed }` rather than a verdict: an unclosed
body is where the two directions part, so the shared half reports the outcome
and each caller decides. `scanSource` over-masks to `end` and reads on (198
positions on this tree, every one a JSX closing tag); the gate answers -1.
Zero behaviour change, measured both sides:
scanSource(BASE) vs scanSource(HEAD) over the same 5929-file corpus
population — comment, literal, interpolation, maskComments and stripComments
all byte-identical: 0 files differ, 0 flag bytes differ.
check-dispatcher-error-vocabulary normal, --report and --self-test output
byte-identical, self-test still 10 shapes + 322 assertions.
#14742's two cross-fixture self-test cases are KEPT and still mean what they
said: the two POSITION RULES are still two, so pinning them against each other
on the agreeing fixtures and on the `}` divergence still guards a real thing.
js-comment-mask --self-test gains a 9-assertion shared-recogniser section
driven in BOTH position modes, with a literal roster and floor of its own:
23 mask/strip corpus + 12 interpolation view + 9 shared recogniser = 44.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
claude
Bot
added
the
skip-changeset
|
ACCEPT — PM seat Reviewed against the three-dot diff
Generated by Claude Code |
|
REWORK (round 1 on this PR; the shift's second) — PM seat CI red on Diagnosis: the implementation is Generated by Claude Code |
`check-declaration-mirrors` went red in CI:
scripts/js-comment-mask.d.mts:106 declares `makeRegexRecogniser` with 1
required parameter(s), but scripts/js-comment-mask.mjs implements 0.
`Function.length` counts the parameters BEFORE the first defaulted one, so
`makeRegexRecogniser({ mayBeginAt } = {})` reports zero required arguments. The
declaration is the honest side — there is no default position rule, so the
options really are required — and the module is what was wrong.
Dropped the `= {}` and read `mayBeginAt` longhand behind an `options == null`
guard, which keeps the existing TypeError message rather than letting a bare
destructuring TypeError replace it with one that explains nothing. No behaviour
change beyond the arity: both `makeRegexRecogniser()` and a non-function rule
still throw, with the same message.
The self-test row that already covered "omitting the rule throws" now pins all
three facts at once — both throws, the message, and
`makeRegexRecogniser.length === 1` — so the module's own `--self-test` reds on
this regression instead of only a gate one layer away. Ablated: restoring the
`= {}` gives `Function.length` 0, `js-comment-mask --self-test` EXIT=1 on that
row, and `check-declaration-mirrors` EXIT=1 with the exact CI message above.
The row name and count are unchanged, so the verdict still reads 44 cases
(23 mask/strip corpus, 12 interpolation view, 9 shared recogniser).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
|
|
Rework round 1 verified — PM seat Generated by Claude Code |
2 participants
Fixes #15487
Two modules each answered "does a
/here open aRegularExpressionLiteral?" withtheir own copy: a byte-identical 14-word
REGEX_AFTER_KEYWORDset, anIDENT_CHARclass, and a body walk.
js-comment-mask.mjs's own header records what a driftingcopy costs — dropping
returnfrom that set passes all 23 of its self-test cases andis caught only by the corpus sweep — so the copy that drifts drifts silently in
exactly the direction its owner warns about.
The choice: a FACTORY, and the line is drawn by number of right answers
The split is not "shared module vs caller"; it is how many right answers the
question has.
The WALK — character class, backslash sequences, line-terminator refusal, flags,
and the keyword set a position rule reads — has exactly one right answer, ECMA-262's.
A second copy can therefore only drift. It now lives once, as
walkRegexBodyinscripts/js-comment-mask.mjs, which already owns "which bytes are literal".The POSITION RULE has two right answers, because the consumers fail in
deliberately opposite directions: a masker over-masks safely, a scanner must never
skip a span it invented. It stays a parameter.
makeRegexRecogniser({ mayBeginAt })rather thanregexLiteralAt(src, at, { mayBeginAt }):check-dispatcher-error-vocabulary.mjscalls its recogniser from fifteen places, and an option threaded through fifteen call
sites is fifteen chances to pass the other consumer's rule — the drift this change
exists to end, re-created one layer up. Binding it once,
leaves all fifteen call sites untouched and makes "this module's rule is THIS one" a
fact about the module rather than a convention its call sites have to keep. There is
no default rule and omitting one throws — a default would be one of two opposite
failure directions handed silently to whichever caller forgot.
walkRegexBodyreturns an OUTCOME, not a verdictThis is the part a naive extraction gets wrong, and it is measured rather than
argued. An unclosed body is a third place the two directions part, and the card's
table does not list it:
tree, every one a JSX closing tag (
</div>,</Link>) in a.tsxfile;So the shared half returns
{ end, closed }and each caller decides whatclosed: falsemeans. A shared function returning only a verdict would have picked one ofthose two, silently, for both.
Also measured while drawing the line, over the same population:
\r/ U+2028 /U+2029 inside a walked body 0; escaped line terminator inside a body 0;
keyword-branch hits that are member names (
map.delete / 2) 0. The tree has noCRLF sources at all.
ZERO behaviour change, measured on both sides
The masker proof is a differential, not a re-run:
scanSourceat the merge baseand
scanSourceon this branch, over the same population (the corpus sweep's own5929 files), comparing
comment,literal,interpolation,maskCommentsandstripCommentsbyte for byte.A plain before/after snapshot of the tree would NOT have shown this: the population
contains
scripts/js-comment-mask.mjsitself, whose own bytes changed, so it differsfor a reason that has nothing to do with behaviour. Running both implementations over
one fixed population is what removes that.
Gate side, byte-identical against main (
cmp -s, all three):check-dispatcher-error-vocabulary(normal)check-dispatcher-error-vocabulary --reportcheck-dispatcher-error-vocabulary --self-test--sites/--json; this gate has neither flag. Itssite-listing reading is
--report, which is what was captured.#14742's two cross-fixture cases are KEPT, and still bite
Both were ablated rather than reasoned about. Each leg mutates, proves the
mutation reached disk (anchored
grep -cFin both directions), runs, restores withgit checkout HEAD -- <absolute path>, and proves the restore by blob hash andan empty
git diff HEAD. Nodist/is involved:node scripts/js-comment-mask.mjsresolves the source file by path, not through a package
exports, so there is norebuild leg to forget.
makeRegexRecogniserignoresmayBeginAt✗ NEVER-SKIP mode reads the same / as division), gate EXIT=1"NEVER-SKIP mode…" ran but is not declared}as a regex positionthe documented DIVERGENCE between the two recognisers is gonereturndropped from the shared setLeg D is the interesting one, and it is the card's own lesson re-measured on the
consolidated copy:
js-comment-mask --self-teststill passes — all 44 cases — exactly as its headersays ("dropping
returnpasses all 23 cases and is caught only by the corpussweep"). Consolidation did not change that.
✗ comment-mask corpus sweep: 1 of 5929 files disagree … scripts/check-test-source-alias.mjs fabricates=0 over-masks=17.The documented backstop is intact on the shared copy.
--self-testnow reds too —`return /re/` is not read as a regex— because the set it reads is now themasker's set. So the shared half gained a second, faster guard it did not have as
two copies.
Leg C and leg D each red a different one of #14742's two cases, which is why both are
kept: they now assert that the two position rules still agree where they must and
still diverge where they must. Neither is redundant.
New self-test section, floored
js-comment-mask --self-testgains 9 assertions driven in BOTH position modes —one recogniser built with an over-masking rule, one with a never-skip rule, the two
differing only at the character under test so nothing here goes stale when a real
rule is re-argued. An assertion in one mode alone would pass on a recogniser that
ignored its parameter entirely (leg A is that mutation).
The section declares a literal roster with a floor of its own, the way the cases
table does, so a deleted or renamed assertion names itself in the refusal. It does
not inherit the
interpolationsection's acknowledged un-floored debt.One of the nine failed on its first run — the expected closing index was mine, not
the walk's; the walk was right. Fixed the expectation, not the walk.
Verification — every exit code captured before any pipe
All on the final commit
63fb64b99, tree clean.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 32 commands for this changeset — this is the card whose blast radius is
the import graph, and the derivation was re-run on the final commit and returned the
same 32. All 32 ran and all 32 exited 0.
Three needed a build first and reported
PREREQUISITE NOT MET/ exit 3 rather than averdict — recorded as NOT MEASURED, then built and re-run to a real reading, never
counted as passes:
pnpm --filter @objectstack/spec run check:browser-reachable-entries--filter @objectstack/specpnpm --filter @objectstack/lint run check:doc-formula-expressions--filter @objectstack/lintpnpm check:type-check-debt13 ledger entr(ies) re-measured in 130.3s, 143 raw tsc error(s), none above its recorded numbercheck:browser-reachable-entriesis the gate that readsjs-comment-mask.d.mtsinside a tsc program, so its green is also the check on the declarations added there.
5023630b1and
origin/mainhas since moved to615fac3a0, across whichscripts/engine-double-contract.pinned.jsonchanged — not a path this PR touches.The families above were derived from this tree; CI derives them again against the
merge result.
skip-changeset:scripts/publishes nothing from any package. The label is set onthis PR.
Out of scope, filed separately
scripts/js-comment-mask.d.mtsdeclaresselfTest(): voidwhile the module returnsSELF_TEST_VERDICT— the value its own CLI dispatch checks to catch a self-test thatreturned without printing a verdict (#13798). Same file and same class as #15398 (the
missing
interpolationfield), which is open and covers onlyscanSource. Filed asa new card rather than fixed here.
🤖 Generated with Claude Code
https://claude.ai/code/session_012zGPuVVX3deAx9LdjK8jCk
Generated by Claude Code