Bring media/json/tokenizer to 100% line/branch/function coverage: 97.06% → 100% branch - #1580
Conversation
97.06% branch coverage (1 uncovered branch): parseMinusState's own case '-' arm (a second '-' token arriving while already in the '-' state) was unreachable — the underlying JS tokenizer always merges adjacent '-' characters into a single '--' token, so this state never sees a second '-'. Same reasoning as djs/tokenizer's PR #1553, which removed the analogous case for the same reason. Removes the dead case; a run of '-' past the first now falls through to the default arm, which already produces the same error result. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmWCv5YGRXToq26xPoSXjX
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmWCv5YGRXToq26xPoSXjX
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | bb8e934 | Commit Preview URL Branch Preview URL |
Aug 15 2026, 07:23 AM |
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approving. Baseline for every comparison below is current origin/main = aa18f265028eb3f6ba749e778b79b8dd8641bbf2 (Remove custom serialization infrastructure (Serializable trait) (#1579)).
This deletes a branch rather than covering one, so the burden is proving the arm dead — and dead on main, not only at the PR head. It is.
The reachability condition. parseMinusState's case '-' is entered only when the scan state is already '-' and the current JS token is '-'. State '-' is produced only by parseDefaultState on a '-' token, and (before this PR) by the removed arm itself. So the arm is reachable iff js/tokenizer can emit two consecutive - tokens.
Instrumentation on main, with a live negative control. Replacing the arm body with throw new Error(...) on main:
- full suite:
2797 pass, 0 fail— the throw never fires; - driving the public
tokenize()over 2,923,664 inputs (every ASCII string of length 1–3, plus every length-4 and length-5 string over- + 1 SP NL > < = * / . a " \ TAB): 0 throws.
Negative control, same harness, throw moved to parseMinusState's default: arm instead — the arm that now absorbs these inputs:
- full suite:
2789 pass, 8 fail; - fuzz: 223,613 throws (first at input
"-").
So the harness demonstrably reaches that switch; it is the case '-' arm specifically that is unreachable. Independently, scanning the same corpus for the token-level condition found 182,992 non-initial - tokens and zero adjacent -/- pairs — maximal munch turns -- into one '--' token (--- → '--', '-'), and - - gets a ws/nl token in between which routes through the default arm and resets the state to 'def'.
Two-sided equivalence. The removed arm and the default arm are not interchangeable (the removed one emitted one error token and stayed in state '-'; default emits an error plus mapToken(token) and returns to 'def'), so identical behaviour has to be shown, not assumed. Differentially running main's tokenize against the PR's over 4,720,084 inputs (the corpus above plus digits/e/E in the length-4/5 alphabet), comparing full token arrays and throw behaviour: 0 differences. Negative control on the differential (perturbing the PR side for inputs containing -): 660,901 differences, so the comparator is live.
Coverage claim measured, both sides. node --test --experimental-test-coverage --test-coverage-include='fjs/media/json/tokenizer/module.f.mjs' fjs/emergent_testing/all.test.mjs:
main:100.00 | 97.06 | 100.00(line | branch | func)- PR head:
100.00 | 100.00 | 100.00
The title's 97.06% → 100% is exactly right.
Battery. npx tsc --noEmit exit 0. npm run prepack from a freshly cleaned tree exit 0. node ./fjs/module.mjs t: 2797 pass, 0 fail — identical to main's 2797, as a dead-code removal should be. Broken-link sets diffed against main: identical. Changelog: changelog/unreleased/1580.md, one per-entry file, no headings, no links or PR numbers inside — correct shape, and its factual claim is the one verified above. No nanvm-lib/ or generator surface touched, so no Rust gates or round-trip needed.
One nit, not a blocker: the new comment says a run of - past the first "always falls through to the default arm below". For -- the state machine never enters the '-' state at all — the merged '--' token is handled by parseDefaultState's default. The emitted result is the same invalid token either way, so nothing behavioural rides on it.
Summary
fjs/media/json/tokenizer/module.f.mjswas at 97.06% branch coverage (1 uncovered branch). This brings it to 100% line/branch/function coverage:parseMinusState's owncase '-'arm — handling a second'-'token arriving while the scanner is already in the'-'state — was unreachable through the publictokenize(): the underlying JS tokenizer (js/tokenizer) always merges adjacent-characters into a single'--'token, so this state never actually sees a second standalone'-'token. This is the same reasoningdjs/tokenizer's PR Improve djs/tokenizer branch coverage: 96.83% → 98.40% #1553 used to remove the analogous case there.-past the first now falls through to thedefaultarm, which already produces the same error result the removed case did.Test plan
npx tsc --noEmitnode --test --experimental-test-coverage --test-coverage-include='fjs/media/json/tokenizer/module.f.mjs' fjs/emergent_testing/all.test.mjs→ 100.00% line/branch/funcnode ./fjs/module.mjs t→ 2797 pass, 0 fail🤖 Generated with Claude Code
Generated by Claude Code