parser: nesting decides the separator, never the terminator (#2127) - #2151
Conversation
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
Merge-gate diagnosis: both red checks reproduce on the base, with the same file and lineThe order was: establish the cause of the red The two red checks, named exactlyWorkflow
Three-way comparison collapses to two-way, and the base is redThe PR base is
Identical job, identical step, identical file and line number. Over the whole history of Two defects behind the synthesis failure, both pre-existing
Both are independent of this PR and belong in their own issue; I have not touched them here. Every changed field set has a reason-codeThe aggregate "0 regressions" is not admissible alone, so each of the 18 files whose field set moved (14
Zero Why An unbalanced Self-correction on the earlier aggregate: the four files previously labelled A corpus defect this surfaced, which is larger than the PR63 spec files contain 115 field lines of the form Merge-gate, stated as a checklist
Two items remain, so this is not a merge request. |
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
PR DashboardGenerated at: 2026-08-14 19:07:31 UTC
Summary
Seal Status
|
Correction: the "0 regressions" claim was weaker than it sounded, and I found it by trying to make the fixtures failThe EOF fixture is now a test — five tests over five fixtures in The hard timeout, and why it is the pointThree cases run under a 10 s in-process wall-clock ceiling. Truncated input leaves bracket depth positive, so a terminator gated on depth is never accepted and the field loop runs past the end of the token stream. Asserting only on the error message would pass on a hang: a hung process never produces a message to compare, the test would sit until the CI job's own timeout killed it, and the result would read as infrastructure flake instead of as #2127. The ceiling is owned by the test thread, both pipes are drained from helper threads so a full pipe buffer cannot masquerade as a hang, and the child is killed before the assertion fires so a wedged parser cannot outlive the test binary. Those three tests are worth less than they lookRun against the pre-fix binary, all three pass unchanged — identical verdict, identical message, identical timing (106 ms vs 106 ms). Two fixtures that do discriminateField sets from
The first is the improvement and the reason the change is worth making: the pre-fix collector promoted The second row is the correctionThree declared fields become one. That is a loss of fields on malformed input. The corpus differential declared regression as "the fixed binary extracts fewer fields than the base" — and this is exactly that, yet the run reported 0 regressions. Both statements are true because the per-file classifier labelled it So the honest reading of the differential is: "0 regressions" means no field loss was found that I judged unacceptable — not that no field was lost. The 17 files under What this does to the merge-gateThe differential criterion is not satisfied as originally written. Restating it as what was actually established:
Two open, one of them newly reopened by this comment. Still not a merge request, and now less ready than the previous comment implied. One more thing the fixtures surfaced
|
Status: not-ready. Bounded evidence only. Not for merge.The differential behind this PR covered 868 of 1089 files, 79.7 %. The Zero regressions on 79.7 % of the corpus is not the absence of regressions. The Two further reasons not to merge yet:
What would move this to ready, in order:
Leaving open. Not closing, not merging. |
Closes #2158) Two measurement tools were lost and every number they had produced became unreproducible with them. cost.py and diffbin.py were written, quoted in #2151, and never committed; the working copy was later re-cloned. Six recovery routes came back empty -- dangling objects held only a git stash WIP with triage.py, the reflog records the clone rather than the content, shell history is absent, CI artifacts hold only FPGA outputs, no PR or issue comment carries the source, and the session snapshot preserved prose about the scripts instead of the scripts. So these are reimplementations from a written contract. Recalling what the old ones roughly did would have reproduced the old one's defect. That defect was the specification for the new one. It reported "0 regressions" over 634 specs while files were losing declared struct fields, because a per-file judgement had relabelled the loss as an acceptable trade and the aggregate then printed the judgement as if it were a measurement. No differential result may be called "0 regressions" unless the metric actually checks the claimed class of loss. diffbin now assigns five ordered categories -- unchanged, field-loss, strict-improvement, malformed-input-tradeoff, unknown -- with field-loss tested before strict-improvement, so removing a phantom while dropping a declared field is a loss and not an improvement. Phantom and declared are told apart by a stated rule: a removed field is a phantom only if its base type text was empty. Only an ExprIdentifier whose parent is a StructDecl counts, so identifiers in function bodies stay out of the totals. Re-measured on the same 634 specs and the same two binaries: 616 unchanged, 13 field-loss, 1 strict-improvement, 4 malformed-input-tradeoff, 0 unknown. handoff.t27 goes from 35 parsed fields to 12. All 17 files that moved are inside the damaged set and no well-formed spec changed at all, which is what 0 unknown is carrying. cost reports per stratum with n, median, p95, min-max ms/KB and coefficient of variation, alpha only at n >= 8 with its r2 and KB range, and no cross-family alpha at all: that number is a metric of corpus composition rather than of the parser (#2133), and a printed number gets quoted while its caveat does not travel with it. damage classifies the corrupt annotations by shape rather than repairing them (#2154): 125 lines, 65 files, 15 shapes, one fixture each. The first draft reported 429, of which 230 were the legitimate bound `target : < 5000ns`, so the fix was deleting two bad signals rather than tuning a threshold. loop-tools-tracked.sh fails when a loop tool is missing, untracked, or unrouted, and was verified to fail in exactly the pre-loss state. The dispatcher no longer looks for a built compiler before running helpers that never use one.
Closes #2158) Two measurement tools were lost and every number they had produced became unreproducible with them. cost.py and diffbin.py were written, quoted in #2151, and never committed; the working copy was later re-cloned. Six recovery routes came back empty -- dangling objects held only a git stash WIP with triage.py, the reflog records the clone rather than the content, shell history is absent, CI artifacts hold only FPGA outputs, no PR or issue comment carries the source, and the session snapshot preserved prose about the scripts instead of the scripts. So these are reimplementations from a written contract. Recalling what the old ones roughly did would have reproduced the old one's defect. That defect was the specification for the new one. It reported "0 regressions" over 634 specs while files were losing declared struct fields, because a per-file judgement had relabelled the loss as an acceptable trade and the aggregate then printed the judgement as if it were a measurement. No differential result may be called "0 regressions" unless the metric actually checks the claimed class of loss. diffbin now assigns five ordered categories -- unchanged, field-loss, strict-improvement, malformed-input-tradeoff, unknown -- with field-loss tested before strict-improvement, so removing a phantom while dropping a declared field is a loss and not an improvement. Phantom and declared are told apart by a stated rule: a removed field is a phantom only if its base type text was empty. Only an ExprIdentifier whose parent is a StructDecl counts, so identifiers in function bodies stay out of the totals. Re-measured on the same 634 specs and the same two binaries: 616 unchanged, 13 field-loss, 1 strict-improvement, 4 malformed-input-tradeoff, 0 unknown. handoff.t27 goes from 35 parsed fields to 12. All 17 files that moved are inside the damaged set and no well-formed spec changed at all, which is what 0 unknown is carrying. cost reports per stratum with n, median, p95, min-max ms/KB and coefficient of variation, alpha only at n >= 8 with its r2 and KB range, and no cross-family alpha at all: that number is a metric of corpus composition rather than of the parser (#2133), and a printed number gets quoted while its caveat does not travel with it. damage classifies the corrupt annotations by shape rather than repairing them (#2154): 125 lines, 65 files, 15 shapes, one fixture each. The first draft reported 429, of which 230 were the legitimate bound `target : < 5000ns`, so the fix was deleting two bad signals rather than tuning a threshold. loop-tools-tracked.sh fails when a loop tool is missing, untracked, or unrouted, and was verified to fail in exactly the pre-loss state. The dispatcher no longer looks for a built compiler before running helpers that never use one.
Closes #2158) (#2159) Two measurement tools were lost and every number they had produced became unreproducible with them. cost.py and diffbin.py were written, quoted in #2151, and never committed; the working copy was later re-cloned. Six recovery routes came back empty -- dangling objects held only a git stash WIP with triage.py, the reflog records the clone rather than the content, shell history is absent, CI artifacts hold only FPGA outputs, no PR or issue comment carries the source, and the session snapshot preserved prose about the scripts instead of the scripts. So these are reimplementations from a written contract. Recalling what the old ones roughly did would have reproduced the old one's defect. That defect was the specification for the new one. It reported "0 regressions" over 634 specs while files were losing declared struct fields, because a per-file judgement had relabelled the loss as an acceptable trade and the aggregate then printed the judgement as if it were a measurement. No differential result may be called "0 regressions" unless the metric actually checks the claimed class of loss. diffbin now assigns five ordered categories -- unchanged, field-loss, strict-improvement, malformed-input-tradeoff, unknown -- with field-loss tested before strict-improvement, so removing a phantom while dropping a declared field is a loss and not an improvement. Phantom and declared are told apart by a stated rule: a removed field is a phantom only if its base type text was empty. Only an ExprIdentifier whose parent is a StructDecl counts, so identifiers in function bodies stay out of the totals. Re-measured on the same 634 specs and the same two binaries: 616 unchanged, 13 field-loss, 1 strict-improvement, 4 malformed-input-tradeoff, 0 unknown. handoff.t27 goes from 35 parsed fields to 12. All 17 files that moved are inside the damaged set and no well-formed spec changed at all, which is what 0 unknown is carrying. cost reports per stratum with n, median, p95, min-max ms/KB and coefficient of variation, alpha only at n >= 8 with its r2 and KB range, and no cross-family alpha at all: that number is a metric of corpus composition rather than of the parser (#2133), and a printed number gets quoted while its caveat does not travel with it. damage classifies the corrupt annotations by shape rather than repairing them (#2154): 125 lines, 65 files, 15 shapes, one fixture each. The first draft reported 429, of which 230 were the legitimate bound `target : < 5000ns`, so the fix was deleting two bad signals rather than tuning a threshold. loop-tools-tracked.sh fails when a loop tool is missing, untracked, or unrouted, and was verified to fail in exactly the pre-loss state. The dispatcher no longer looks for a built compiler before running helpers that never use one.
861155d to
5c65be2
Compare
parse_struct_body collected raw lexemes until a comma, so a comma inside a nested type ended the field. Map<Key, Value> became field `generic : Map<Key` plus a phantom field named `Value`; (A, B) behaved the same way. Six declared fields parsed as eight. Two earlier repairs were reverted. Both failed the same way: they let a depth counter decide a block terminator. Attempt (a) let depth suppress RBrace and the scanner ate the struct's closing brace. Attempt (b) let depth suppress Eof and the build hung, because the lexer yields Eof indefinitely while the depth counter never moves, so the state is stationary and the token stream infinite. The rule this commit follows: RBrace, Semicolon and Eof terminate unconditionally at any depth; only Comma consults depth. Termination therefore does not depend on the input being well-formed, which is what a recovery scanner exists for. Measured over all 634 specs outside specs/scratch, candidate against the binary it replaces: 616 unchanged, 4 improved, 14 changed field sets, 0 regressions of any kind. scripts/tri_loop/diffbin.py produced that table and exits non-zero on any regression. Also adds tri cost and tri triage. tri cost reports per-KB spread next to the scaling exponent, because an exponent fitted over a heterogeneous sample is a composition metric rather than a property of the parser -- the confound #2133 named for the ratchet, which #1781 hit for parse scaling. Closes #2127
5c65be2 to
d996ce3
Compare
PR DashboardGenerated at: 2026-08-21 07:35:10 UTC
Summary
Seal Status
|
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
1 participant
What was wrong
parse_struct_bodycollected raw lexemes until a comma, so a comma inside a nested type ended the field. On a fixture with six declared fields:masterplainu8genericMap<Key← truncatedValuetuple(A← truncatedBnestedVec<Vec<u8>>arr[4]u16lasti32Six fields parsed as eight. After this change:
generic : Map<Key,Value>,tuple : (A,B), phantoms gone, exactly six.Why the two earlier attempts were reverted, and what is different
Both reverted repairs failed the same way: they let a depth counter decide a block terminator.
RBrace→ the scanner ate the struct's closing brace (161→171 recovery events, 0→6 specs capturing nothing)Eof→ the build hungThe hang is not bad luck, it is forced. The lexer yields
Eofindefinitely and never changes the depth counter, so if there is any depth at whichEofis not accepted as a terminator, the scanner's state is stationary while the token stream is infinite. A recovery scanner's termination must not depend on its input being well-formed — that input is precisely why it exists.This change follows the rule that both attempts lacked:
Depth is tracked over
(/),[/], and</>(the latter only when<directly follows an identifier, so<as a comparison operator cannot open a level;>>closes two).Evidence
Both binaries built from this tree, run over the same corpus — all 634 specs outside
specs/scratch:No file where the candidate fails, hangs, or leaves more truncated types than the baseline.
The
Eofhazard has a fixture rather than an argument: a struct whose last field is an unclosed generic at end of file exits1withParse error: Expected RBrace, got Eof ('') at line 5:1, in under a second.The limit, stated plainly
On malformed input the new collector absorbs the following field into the type text (
a : Map<K,b:u8) wheremastertruncated the type instead. That is one kind of damage traded for another on input that is already broken. On well-formed input it is strictly better, and that is the only claim made here.Tooling added, and why it belongs in this PR
scripts/tri_loop/diffbin.py(tri diffbin) is the tool that produced the table above; it exits non-zero on any regression, so the check a reviewer would want is now one command. Given that two prior repairs to this exact function were merged and reverted, the check being cheap is part of the fix.tri costandtri triagecome from the same session.tri costreports the per-kilobyte spread next to the scaling exponent, because an exponent fitted over a heterogeneous sample is partly a composition metric rather than a property of the parser — the confound #2133 named for the ratchet. Measured inside one homogeneous family (wNNN_bench_module_NNx2p6_aos_var_call_write, n=39, 342KB…818KB):alpha = 0.940,R^2 = 0.986, per-KB spread10.65%— i.e. linear. See the comment left on #1781.Closes #2127