From 6cff8f5c0d372bb5a12dfa02db654cbbc141f9a4 Mon Sep 17 00:00:00 2001 From: gHashTag Date: Fri, 14 Aug 2026 19:25:18 +0000 Subject: [PATCH] tri: restore cost and diffbin, and stop aggregating field loss to zero (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. --- .github/workflows/loop-tools-gate.yml | 63 ++++ .../tests/fixtures/damage/damage_class_01.t27 | 9 + .../tests/fixtures/damage/damage_class_02.t27 | 9 + .../tests/fixtures/damage/damage_class_03.t27 | 9 + .../tests/fixtures/damage/damage_class_04.t27 | 9 + .../tests/fixtures/damage/damage_class_05.t27 | 9 + .../tests/fixtures/damage/damage_class_06.t27 | 9 + .../tests/fixtures/damage/damage_class_07.t27 | 9 + .../tests/fixtures/damage/damage_class_08.t27 | 9 + .../tests/fixtures/damage/damage_class_09.t27 | 9 + .../tests/fixtures/damage/damage_class_10.t27 | 9 + .../tests/fixtures/damage/damage_class_11.t27 | 9 + .../tests/fixtures/damage/damage_class_12.t27 | 9 + .../tests/fixtures/damage/damage_class_13.t27 | 9 + .../tests/fixtures/damage/damage_class_14.t27 | 9 + .../tests/fixtures/damage/damage_class_15.t27 | 9 + .../tests/fixtures/terminator/eof_generic.t27 | 8 + .../tests/fixtures/terminator/eof_hazard.t27 | 4 + .../fixtures/terminator/field_swallow.t27 | 7 + .../fixtures/terminator/nested_types.t27 | 14 + .../fixtures/terminator/semicolon_phantom.t27 | 6 + .../tests/fixtures/terminator/tuple_type.t27 | 12 + .../tests/fixtures/terminator/unbalanced.t27 | 8 + bootstrap/tests/struct_body_terminator.rs | 325 +++++++++++++++++ docs/NOW.md | 16 + scripts/ci/loop-tools-tracked.sh | 114 ++++++ scripts/tri | 28 +- scripts/tri_loop/cost.py | 274 ++++++++++++++ scripts/tri_loop/damage.py | 191 ++++++++++ scripts/tri_loop/diffbin.py | 333 ++++++++++++++++++ scripts/tri_loop/triage.py | 172 +++++++++ 31 files changed, 1709 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/loop-tools-gate.yml create mode 100644 bootstrap/tests/fixtures/damage/damage_class_01.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_02.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_03.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_04.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_05.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_06.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_07.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_08.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_09.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_10.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_11.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_12.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_13.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_14.t27 create mode 100644 bootstrap/tests/fixtures/damage/damage_class_15.t27 create mode 100644 bootstrap/tests/fixtures/terminator/eof_generic.t27 create mode 100644 bootstrap/tests/fixtures/terminator/eof_hazard.t27 create mode 100644 bootstrap/tests/fixtures/terminator/field_swallow.t27 create mode 100644 bootstrap/tests/fixtures/terminator/nested_types.t27 create mode 100644 bootstrap/tests/fixtures/terminator/semicolon_phantom.t27 create mode 100644 bootstrap/tests/fixtures/terminator/tuple_type.t27 create mode 100644 bootstrap/tests/fixtures/terminator/unbalanced.t27 create mode 100644 bootstrap/tests/struct_body_terminator.rs create mode 100755 scripts/ci/loop-tools-tracked.sh create mode 100644 scripts/tri_loop/cost.py create mode 100644 scripts/tri_loop/damage.py create mode 100644 scripts/tri_loop/diffbin.py create mode 100644 scripts/tri_loop/triage.py diff --git a/.github/workflows/loop-tools-gate.yml b/.github/workflows/loop-tools-gate.yml new file mode 100644 index 0000000000..e61f59861c --- /dev/null +++ b/.github/workflows/loop-tools-gate.yml @@ -0,0 +1,63 @@ +# A loop tool that exists only in one working directory is not in the project. +# +# scripts/tri_loop/cost.py and diffbin.py were written, used to produce numbers +# that were quoted in a pull request, and lost -- never committed, and the working +# copy was later re-cloned. Every figure they had produced became unreproducible +# at once. Six recovery routes came back empty (#2158). +# +# Nothing in CI could tell the difference between a tool that exists and a tool +# that exists nowhere but one untracked directory. This gate is that difference. +# It is deliberately cheap: no build, no compiler, no corpus run, so there is no +# reason for it to be skipped or made optional. +name: loop-tools-gate + +on: + pull_request: + paths: + - "scripts/tri" + - "scripts/tri_loop/**" + - "scripts/ci/loop-tools-tracked.sh" + - ".github/workflows/loop-tools-gate.yml" + push: + branches: [master] + paths: + - "scripts/tri" + - "scripts/tri_loop/**" + - "scripts/ci/loop-tools-tracked.sh" + +permissions: + contents: read + +jobs: + loop-tools-tracked: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + # The check reads git index state, so it needs real history rather than + # a detached blob export. + fetch-depth: 1 + + - name: Every loop tool is present, tracked, and routed + run: bash scripts/ci/loop-tools-tracked.sh + + - name: Each loop helper is syntactically loadable + # A tracked tool that cannot be imported is tracked and useless. This is + # a syntax check only and asserts nothing about behaviour. + run: | + for f in scripts/tri_loop/*.py; do + python3 -c "import py_compile,sys; py_compile.compile('$f', doraise=True)" \ + && echo "ok $f" \ + || { echo "BROKEN $f"; exit 1; } + done + + - name: The dispatcher runs its helpers without a built compiler + # tri triage and tri damage read the tracker and the spec text and have no + # use for t27c. The dispatcher used to look for the binary first and + # refuse to run them on a machine with no build, so this step exists to + # keep that ordering from coming back. There is no compiler in this job, + # which is the whole point. + run: | + ./scripts/tri loop-help + ./scripts/tri damage specs >/dev/null || true + echo "ok helpers dispatch with no t27c present" diff --git a/bootstrap/tests/fixtures/damage/damage_class_01.t27 b/bootstrap/tests/fixtures/damage/damage_class_01.t27 new file mode 100644 index 0000000000..f58109de50 --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_01.t27 @@ -0,0 +1,9 @@ +module damage_class_01 + +// shape: [[]X", +// 52 line(s) in the corpus share this shape +// first seen: specs/tri/pipeline/builder.t27:14 +// signals: doubled-bracket,odd-quote +pub struct Damaged { + items : [[]T", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_02.t27 b/bootstrap/tests/fixtures/damage/damage_class_02.t27 new file mode 100644 index 0000000000..67ab30ab19 --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_02.t27 @@ -0,0 +1,9 @@ +module damage_class_02 + +// shape: [[]X X", +// 37 line(s) in the corpus share this shape +// first seen: specs/tri/pipeline/spec_writer.t27:14 +// signals: doubled-bracket,odd-quote +pub struct Damaged { + field_type : [[]Const u8", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_03.t27 b/bootstrap/tests/fixtures/damage/damage_class_03.t27 new file mode 100644 index 0000000000..f8f3acf3fb --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_03.t27 @@ -0,0 +1,9 @@ +module damage_class_03 + +// shape: [[]X [, +// 10 line(s) in the corpus share this shape +// first seen: specs/tri/pipeline/spec_writer.t27:23 +// signals: doubled-bracket +pub struct Damaged { + steps : [[]Const [, +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_04.t27 b/bootstrap/tests/fixtures/damage/damage_class_04.t27 new file mode 100644 index 0000000000..5f6e2e55e7 --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_04.t27 @@ -0,0 +1,9 @@ +module damage_class_04 + +// shape: [[][, +// 7 line(s) in the corpus share this shape +// first seen: specs/tri/encoding/mime.t27:15 +// signals: doubled-bracket +pub struct Damaged { + to : [[][, +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_05.t27 b/bootstrap/tests/fixtures/damage/damage_class_05.t27 new file mode 100644 index 0000000000..61147193ba --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_05.t27 @@ -0,0 +1,9 @@ +module damage_class_05 + +// shape: [[9]X", +// 5 line(s) in the corpus share this shape +// first seen: specs/tri/crypto/hmac.t27:14 +// signals: odd-quote +pub struct Damaged { + opad : [[64]U8", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_06.t27 b/bootstrap/tests/fixtures/damage/damage_class_06.t27 new file mode 100644 index 0000000000..cccdb197b4 --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_06.t27 @@ -0,0 +1,9 @@ +module damage_class_06 + +// shape: [?[]X X", +// 3 line(s) in the corpus share this shape +// first seen: specs/tri/pipeline/spec_writer.t27:36 +// signals: doubled-bracket,odd-quote +pub struct Damaged { + error_msg : [?[]Const u8", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_07.t27 b/bootstrap/tests/fixtures/damage/damage_class_07.t27 new file mode 100644 index 0000000000..da3ec57353 --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_07.t27 @@ -0,0 +1,9 @@ +module damage_class_07 + +// shape: [X.X([]X X)", +// 2 line(s) in the corpus share this shape +// first seen: specs/tri/encoding/html.t27:15 +// signals: odd-quote +pub struct Damaged { + attributes : [std.StringHashMap([]Const u8)", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_08.t27 b/bootstrap/tests/fixtures/damage/damage_class_08.t27 new file mode 100644 index 0000000000..d81e4e9be7 --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_08.t27 @@ -0,0 +1,9 @@ +module damage_class_08 + +// shape: [[9]?X", +// 2 line(s) in the corpus share this shape +// first seen: specs/tri/trees/octree.t27:24 +// signals: odd-quote +pub struct Damaged { + children : [[8]?OctNode", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_09.t27 b/bootstrap/tests/fixtures/damage/damage_class_09.t27 new file mode 100644 index 0000000000..c070a80bec --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_09.t27 @@ -0,0 +1,9 @@ +module damage_class_09 + +// shape: [[]?X", +// 1 line(s) in the corpus share this shape +// first seen: specs/tri/graph/dijkstra.t27:15 +// signals: doubled-bracket,odd-quote +pub struct Damaged { + parent : [[]?Usize", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_10.t27 b/bootstrap/tests/fixtures/damage/damage_class_10.t27 new file mode 100644 index 0000000000..096cf0e18f --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_10.t27 @@ -0,0 +1,9 @@ +module damage_class_10 + +// shape: [X.X(X, []X)", +// 1 line(s) in the corpus share this shape +// first seen: specs/tri/graph/graph.t27:14 +// signals: odd-quote +pub struct Damaged { + nodes : [std.HashMap(T, []T)", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_11.t27 b/bootstrap/tests/fixtures/damage/damage_class_11.t27 new file mode 100644 index 0000000000..1cd48eca65 --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_11.t27 @@ -0,0 +1,9 @@ +module damage_class_11 + +// shape: [[]?*X", +// 1 line(s) in the corpus share this shape +// first seen: specs/tri/trees/b_tree.t27:15 +// signals: doubled-bracket,odd-quote +pub struct Damaged { + children : [[]?*BTreeNode", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_12.t27 b/bootstrap/tests/fixtures/damage/damage_class_12.t27 new file mode 100644 index 0000000000..689a32ea57 --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_12.t27 @@ -0,0 +1,9 @@ +module damage_class_12 + +// shape: [[][9, +// 1 line(s) in the corpus share this shape +// first seen: specs/tri/trees/quadtree.t27:23 +// signals: doubled-bracket +pub struct Damaged { + points : [[][2, +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_13.t27 b/bootstrap/tests/fixtures/damage/damage_class_13.t27 new file mode 100644 index 0000000000..5c6756ca4a --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_13.t27 @@ -0,0 +1,9 @@ +module damage_class_13 + +// shape: [[9]?*X", +// 1 line(s) in the corpus share this shape +// first seen: specs/tri/search/aho_corasick.t27:14 +// signals: odd-quote +pub struct Damaged { + children : [[256]?*ACTrieNode", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_14.t27 b/bootstrap/tests/fixtures/damage/damage_class_14.t27 new file mode 100644 index 0000000000..0b4bfaf393 --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_14.t27 @@ -0,0 +1,9 @@ +module damage_class_14 + +// shape: [[]X(X, X)", +// 1 line(s) in the corpus share this shape +// first seen: specs/tri/collections/btree.t27:21 +// signals: doubled-bracket,odd-quote +pub struct Damaged { + children : [[]BTreeNode(K, V)", +} diff --git a/bootstrap/tests/fixtures/damage/damage_class_15.t27 b/bootstrap/tests/fixtures/damage/damage_class_15.t27 new file mode 100644 index 0000000000..85a5082aec --- /dev/null +++ b/bootstrap/tests/fixtures/damage/damage_class_15.t27 @@ -0,0 +1,9 @@ +module damage_class_15 + +// shape: [[]?X(X)", +// 1 line(s) in the corpus share this shape +// first seen: specs/tri/collections/skip_list.t27:15 +// signals: doubled-bracket,odd-quote +pub struct Damaged { + forward : [[]?SkipNode(T)", +} diff --git a/bootstrap/tests/fixtures/terminator/eof_generic.t27 b/bootstrap/tests/fixtures/terminator/eof_generic.t27 new file mode 100644 index 0000000000..6bbe35ead1 --- /dev/null +++ b/bootstrap/tests/fixtures/terminator/eof_generic.t27 @@ -0,0 +1,8 @@ +module eof_generic + +// End of file inside an unclosed generic argument list. The lexer returns Eof +// for every subsequent call, so a recovery scanner that refuses to accept Eof as +// a terminator has a stationary state on an infinite input, and the loop is a +// consequence of the construction rather than a rare race. +pub struct Hangs { + a : Map, + tuple : (A, B), + nested : Vec>, + arr : [4]u16, + last : i32 +} + +pub fn use_holder(h : Holder) -> u8 { + return h.plain; +} diff --git a/bootstrap/tests/fixtures/terminator/semicolon_phantom.t27 b/bootstrap/tests/fixtures/terminator/semicolon_phantom.t27 new file mode 100644 index 0000000000..a08c37c7c4 --- /dev/null +++ b/bootstrap/tests/fixtures/terminator/semicolon_phantom.t27 @@ -0,0 +1,6 @@ +module semicolon_phantom + +pub struct Semi { + a : Map, u8), + last : u16 +} diff --git a/bootstrap/tests/fixtures/terminator/unbalanced.t27 b/bootstrap/tests/fixtures/terminator/unbalanced.t27 new file mode 100644 index 0000000000..1977f275de --- /dev/null +++ b/bootstrap/tests/fixtures/terminator/unbalanced.t27 @@ -0,0 +1,8 @@ +module unbalanced + +pub struct Broken { + a : Map u8 { return 0; } diff --git a/bootstrap/tests/struct_body_terminator.rs b/bootstrap/tests/struct_body_terminator.rs new file mode 100644 index 0000000000..0eb0e5fb4e --- /dev/null +++ b/bootstrap/tests/struct_body_terminator.rs @@ -0,0 +1,325 @@ +// ============================================================================ +// #2127 -- nesting decides the separator, never the terminator. +// +// The field collector consults bracket nesting depth when it meets a `Comma`, +// because a comma inside `Map` or `(A, B)` separates type arguments and +// not fields. It must NOT consult depth when it meets a terminator: `RBrace`, +// `Semicolon` and `Eof` end the field list at any depth. Truncated input leaves +// the depth counter positive, so a depth-gated terminator is never accepted and +// the loop runs to the end of the token stream -- or, with a buggy bound, does +// not stop at all. +// +// The test that matters here is therefore a LIVENESS test, not an output test: +// a spec whose last line opens a bracket and then ends must make the parser +// TERMINATE. Asserting on the error message alone would pass even if the parser +// hung, because a hung process never produces a message to compare -- the test +// would sit there until the CI job's own timeout killed it, and report as +// infrastructure flake rather than as this defect. +// +// So every case here runs under a HARD wall-clock timeout enforced in-process. +// Exceeding it fails the test with a message that names the hang, and the child +// is killed so a wedged parser cannot outlive the test binary. +// +// Deliberately NOT asserted: the exact text of the parse error. On malformed +// input there is no single correct reading of the field list, and pinning the +// message would freeze one arbitrary recovery as the specification. What is +// pinned is: the process ends, its exit status is non-zero, it names the token +// it stopped at, and it fails as a diagnostic rather than as a panic. +// +// The wording is deliberately not pinned either. The recorded baseline for this +// fixture read `Error: Parse error: Expected RBrace, got Eof` and the binary now +// prints `Error: Expected RBrace, got Eof` -- the prefix moved at some point +// between the two. An assertion on the phrase "parse" fails on that alone while +// the parser behaves correctly, which is a test measuring diagnostic prose +// instead of parser behaviour. It asserts on the token names instead, because +// those are what the invariant is about. +// ============================================================================ + +use std::io::Read; +use std::path::PathBuf; +use std::process::{Command, Stdio}; +use std::sync::mpsc; +use std::thread; +use std::time::{Duration, Instant}; + +/// Wall-clock ceiling for a single parse of a four-line fixture. Generous by +/// three orders of magnitude: these files are under 100 bytes, and a healthy +/// parse of them is a few milliseconds even on a loaded shared runner. The +/// number exists to separate "slow" from "not returning", and 10 s cannot be +/// reached by any amount of ordinary slowness on this input. +const HARD_TIMEOUT: Duration = Duration::from_secs(10); + +fn t27c() -> &'static str { + env!("CARGO_BIN_EXE_t27c") +} + +/// Pull the `(field name, type text)` pairs out of a `t27c parse` dump. +/// +/// The three liveness cases above use `check`, which prints a verdict and hides +/// the field list. The two cases below are about the field list itself, so they +/// read the parse dump. This is the same extraction the corpus differential used, +/// reimplemented here without a regex crate: find each `name:` line and take the +/// `extra_type:` two lines below it. +fn fields(name: &str) -> Vec<(String, String)> { + let path = fixture(name); + let out = Command::new(t27c()) + .arg("parse") + .arg(&path) + .output() + .expect("failed to spawn t27c parse"); + let text = format!( + "{}{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + let lines: Vec<&str> = text.lines().collect(); + let unquote = |s: &str, key: &str| -> Option { + let t = s.trim(); + let rest = t.strip_prefix(key)?.trim(); + let rest = rest.strip_prefix('"')?; + let end = rest.rfind('"')?; + Some(rest[..end].to_string()) + }; + let mut out_pairs = Vec::new(); + for i in 0..lines.len() { + if !lines[i].trim_start().starts_with("kind: ExprIdentifier") { + continue; + } + if i + 3 >= lines.len() { + continue; + } + let n = unquote(lines[i + 1], "name:"); + let ty = unquote(lines[i + 3], "extra_type:"); + if let (Some(n), Some(ty)) = (n, ty) { + out_pairs.push((n, ty)); + } + } + out_pairs +} + +fn fixture(name: &str) -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests") + .join("fixtures") + .join("terminator") + .join(name) +} + +struct Outcome { + status: i32, + output: String, + elapsed: Duration, +} + +/// Run `t27c check ` under a hard timeout. +/// +/// The child is spawned, then waited on from a helper thread so the main thread +/// keeps a clock the child cannot influence. On timeout the child is killed +/// before the assertion fires, so a wedged parser does not survive the test. +fn parse_within_timeout(name: &str) -> Outcome { + let path = fixture(name); + assert!(path.exists(), "fixture missing: {}", path.display()); + + let mut child = Command::new(t27c()) + .arg("check") + .arg(&path) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("failed to spawn t27c"); + + let started = Instant::now(); + let mut out = child.stdout.take().expect("stdout"); + let mut err = child.stderr.take().expect("stderr"); + + // Drain both pipes from threads. A parser that fills a pipe buffer and then + // blocks on write would otherwise look like a hang caused by this test. + let (otx, orx) = mpsc::channel(); + thread::spawn(move || { + let mut s = String::new(); + let _ = out.read_to_string(&mut s); + let _ = otx.send(s); + }); + let (etx, erx) = mpsc::channel(); + thread::spawn(move || { + let mut s = String::new(); + let _ = err.read_to_string(&mut s); + let _ = etx.send(s); + }); + + // Poll rather than block, so the timeout is owned by this thread. + let status = loop { + match child.try_wait().expect("try_wait") { + Some(st) => break st, + None => { + if started.elapsed() > HARD_TIMEOUT { + let _ = child.kill(); + let _ = child.wait(); + panic!( + "HANG: t27c check {} did not terminate within {:?}. \ + This is #2127: a terminator ({{RBrace, Semicolon, Eof}}) was \ + gated on bracket nesting depth, and truncated input leaves \ + the depth positive, so the field loop never accepted an end.", + name, HARD_TIMEOUT + ); + } + thread::sleep(Duration::from_millis(20)); + } + } + }; + + let elapsed = started.elapsed(); + let combined = format!( + "{}{}", + orx.recv_timeout(Duration::from_secs(5)).unwrap_or_default(), + erx.recv_timeout(Duration::from_secs(5)).unwrap_or_default() + ); + Outcome { + status: status.code().unwrap_or(-1), + output: combined, + elapsed, + } +} + +/// The case the fix is about: the file ends mid-type, at depth 1, with no +/// closing brace anywhere. `Eof` must end the field list at any depth. +#[test] +fn eof_at_positive_depth_terminates() { + let r = parse_within_timeout("eof_hazard.t27"); + assert!( + r.elapsed < HARD_TIMEOUT, + "took {:?}, ceiling {:?}", + r.elapsed, + HARD_TIMEOUT + ); + assert_ne!( + r.status, 0, + "truncated input must be rejected, not accepted; output was:\n{}", + r.output + ); + let lower = r.output.to_lowercase(); + assert!( + lower.contains("eof"), + "the diagnostic must name the end of input it stopped at, got:\n{}", + r.output + ); + assert!( + !lower.contains("panicked") && !lower.contains("unwrap"), + "must fail as a diagnostic, not as a panic:\n{}", + r.output + ); +} + +/// A type argument list left open, then a further field, then a closing brace. +/// `RBrace` must end the field list even though depth is still positive. +#[test] +fn rbrace_at_positive_depth_terminates() { + let r = parse_within_timeout("unbalanced.t27"); + assert!( + r.elapsed < HARD_TIMEOUT, + "took {:?}, ceiling {:?}", + r.elapsed, + HARD_TIMEOUT + ); + let lower = r.output.to_lowercase(); + assert!( + !lower.contains("panicked"), + "must not panic:\n{}", + r.output + ); +} + +/// The control, and the reason the two above are not vacuous: a well-formed +/// struct whose fields contain commas inside `Map`, `(A, B)`, `Vec>` +/// and `[4]u16` must still parse cleanly. A fix that accepted any terminator by +/// ignoring depth altogether would break this one, by treating a comma inside a +/// type argument list as a field separator. +#[test] +fn commas_inside_types_are_not_field_separators() { + let r = parse_within_timeout("nested_types.t27"); + assert!( + r.elapsed < HARD_TIMEOUT, + "took {:?}, ceiling {:?}", + r.elapsed, + HARD_TIMEOUT + ); + assert_eq!( + r.status, 0, + "well-formed nested types must parse; output was:\n{}", + r.output + ); +} + +// --------------------------------------------------------------------------- +// The two cases below discriminate the fixed parser from the unfixed one. The +// three liveness cases above do NOT: run against the pre-fix binary they pass +// unchanged, because `check` prints the same verdict either way. A test that +// cannot fail on the defect it names is a regression guard, not evidence, and +// these two exist so the branch has at least one of each. +// --------------------------------------------------------------------------- + +/// The improvement, stated as a field set. `Map` should be. Before the fix the collector produced a PHANTOM field +/// named `V` with an empty type -- an identifier from inside a type argument +/// list promoted to a field of the struct. After the fix `Semicolon` terminates +/// at any depth, `V` stays inside the type text, and the struct has exactly the +/// two fields it was written with. +/// +/// This is the case that fails on the pre-fix binary, and the reason the change +/// is worth making. +#[test] +fn semicolon_at_depth_does_not_invent_a_field() { + let f = fields("semicolon_phantom.t27"); + let names: Vec<&str> = f.iter().map(|(n, _)| n.as_str()).collect(); + assert!( + !names.contains(&"V"), + "phantom field V promoted out of a type argument list: {:?}", + f + ); + assert_eq!(names, vec!["a", "b"], "expected exactly the declared fields: {:?}", f); +} + +/// The cost, pinned so it cannot grow unnoticed. +/// +/// On a type argument list left open by a comma, the fixed collector absorbs the +/// following `name : type` pairs into the type text of the first field: three +/// declared fields become one, whose type reads `Map= 8, printed with its r2 and its KB range. A single exponent across strata is a metric of corpus composition rather than of the parser (#2133), so it is not printed at all -- a number gets quoted and its caveat does not travel with it +- **`tri damage` classifies the corrupt annotations by shape instead of repairing them (#2154).** 125 lines in 65 files, 15 distinct shapes; one emitted fixture per shape. The first draft of the classifier reported 429 lines, of which 230 were `target : < 5000ns` -- a legitimate less-than bound. Two signals survive, `[[]` and an odd `"`, and the fix was deleting the bad signals rather than tuning a threshold +- **`scripts/ci/loop-tools-tracked.sh` makes the loss impossible rather than regrettable.** It fails when a loop tool is missing, when it exists but git does not track it, when anything under `scripts/tri_loop/` is untracked, or when the generic dispatch line is gone. Verified to fail in exactly the pre-loss state +- **The dispatcher looked for a built compiler before dispatching helpers that do not use one.** `tri triage` and `tri damage` read the tracker and the spec text; on a machine with no build they refused to run. Loop dispatch now precedes the binary lookup + # NOW -- BNF: the control that measures what ternary is worth (2026-08-09) Last updated: 2026-08-09 diff --git a/scripts/ci/loop-tools-tracked.sh b/scripts/ci/loop-tools-tracked.sh new file mode 100755 index 0000000000..fc491e050d --- /dev/null +++ b/scripts/ci/loop-tools-tracked.sh @@ -0,0 +1,114 @@ +#!/usr/bin/env bash +# Loss of a loop tool must be impossible, not merely regrettable. +# +# Two scripts -- scripts/tri_loop/cost.py and scripts/tri_loop/diffbin.py -- were +# written, used to produce numbers that were then quoted in a pull request, and +# lost. They were never committed to any branch, and when the working copy was +# re-cloned they went with it. Six recovery routes were checked and all came back +# empty: dangling git objects, the reflog, shell history, CI artifacts, PR and +# issue comments, and the session snapshot. Every number they had produced became +# unreproducible in one stroke. +# +# The failure was not carelessness at the keyboard. It was that nothing in the +# repository could tell the difference between a tool that exists and a tool that +# exists only in one untracked working directory. This script is that difference. +# +# It fails when: +# 1. a required loop tool is missing from the working tree +# 2. a required loop tool exists but git does not track it -- the exact state +# that preceded the loss +# 3. any file under scripts/tri_loop/ is untracked, so a NEW tool cannot be +# quietly added and then lost the same way +# 4. a required tool is not executable via the dispatcher, i.e. `tri ` +# does not resolve +# +# Not claimed: that this makes the tools correct, or that a tracked file cannot +# be deleted in a later commit. It closes the specific hole that swallowed these +# two files -- work that exists only outside version control -- and nothing wider. +set -uo pipefail + +cd "$(dirname "$0")/../.." || exit 2 + +REQUIRED_TOOLS=( + "scripts/tri_loop/triage.py" + "scripts/tri_loop/cost.py" + "scripts/tri_loop/diffbin.py" + "scripts/tri_loop/damage.py" +) +REQUIRED_SUBCOMMANDS=(triage cost diffbin damage) + +fail=0 +note() { printf ' %s\n' "$1"; } + +echo "loop-tools-tracked: the tools the measurement loop depends on" +echo + +echo "1. present in the working tree" +for f in "${REQUIRED_TOOLS[@]}"; do + if [[ -f "$f" ]]; then + note "ok $f" + else + note "MISSING $f" + fail=1 + fi +done + +echo +echo "2. tracked by git (this is the check that would have caught the loss)" +for f in "${REQUIRED_TOOLS[@]}"; do + [[ -f "$f" ]] || continue + if git ls-files --error-unmatch "$f" >/dev/null 2>&1; then + note "tracked $f" + else + note "UNTRACKED $f -- exists here and nowhere else; one re-clone loses it" + fail=1 + fi +done + +echo +echo "3. nothing under scripts/tri_loop/ is untracked" +untracked=$(git ls-files --others --exclude-standard -- scripts/tri_loop/ 2>/dev/null) +if [[ -z "$untracked" ]]; then + note "ok no untracked files under scripts/tri_loop/" +else + while IFS= read -r u; do + [[ -n "$u" ]] && note "UNTRACKED $u" + done <<<"$untracked" + fail=1 +fi + +echo +echo "4. reachable through the dispatcher" +# The dispatcher resolves `tri ` generically, to tri_loop/.py. +# So the honest check is that the generic exec line is still there AND that the +# file each subcommand resolves to exists. An earlier draft of this check grepped +# scripts/tri for the literal command name, which passed for `tri triage` only +# because the word appears in a comment -- a check that reports the presence of +# its own documentation is worth nothing. +if grep -q 'exec python3 "\$LOOP_DIR/\${cmd//-/_}.py"' scripts/tri; then + note "ok generic loop dispatch line present" +else + note "BROKEN generic loop dispatch line is gone from scripts/tri" + fail=1 +fi +for c in "${REQUIRED_SUBCOMMANDS[@]}"; do + target="scripts/tri_loop/${c//-/_}.py" + if [[ -f "$target" ]]; then + note "ok tri $c -> $target" + else + note "UNROUTED tri $c -> $target does not exist" + fail=1 + fi +done + +echo +if [[ $fail -eq 0 ]]; then + echo "PASS: every loop tool is present, tracked, and routed." + echo "This says nothing about whether any of them is correct." +else + echo "FAIL: a loop tool is missing, untracked, or unrouted." + echo "An untracked tool is the state that already destroyed two of these" + echo "scripts and every number they produced. Commit it before relying on" + echo "its output for any claim." +fi +exit $fail diff --git a/scripts/tri b/scripts/tri index e012eb1081..1313b0ed0e 100755 --- a/scripts/tri +++ b/scripts/tri @@ -2,6 +2,33 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +# Loop helpers live in scripts/tri_loop/ as Python, and are dispatched BEFORE the +# compiler binary is even looked for. They measure and report on the repository, +# they do not compile it, and none of them mutates anything. Requiring a built +# t27c first was a defect: `tri triage` and `tri damage` read the tracker and the +# spec text and have no use for the compiler, yet a machine with no build could +# not run them. Keeping the helpers ahead of the lookup also means a missing +# compiler cannot take the reporting tools down with it. +cmd="${1:-}" +LOOP_DIR="$SCRIPT_DIR/tri_loop" +if [[ "$cmd" == "loop-help" ]]; then + echo "tri loop helpers (report only, never mutate):" + for f in "$LOOP_DIR"/*.py; do + [[ -e "$f" ]] || continue + name="$(basename "$f" .py)" + printf ' %-12s %s\n' "$name" "$(sed -n '2s/^"""tri [a-z-]* -- //p' "$f")" + done + echo + echo "Every helper prints what it does NOT establish alongside what it does." + echo "Read that part: an aggregate that omits the class of loss it failed to" + echo "check is how a differential once reported zero regressions over a corpus" + echo "in which thirteen files lost declared fields." + exit 0 +fi +if [[ -n "$cmd" && -f "$LOOP_DIR/${cmd//-/_}.py" ]]; then + exec python3 "$LOOP_DIR/${cmd//-/_}.py" "${@:2}" +fi + T27C="${TRI_T27C:-}" if [[ -z "$T27C" ]]; then for p in "$REPO_ROOT/target/release/t27c" "$REPO_ROOT/target/debug/t27c" "$REPO_ROOT/bootstrap/target/release/t27c" "$REPO_ROOT/bootstrap/target/debug/t27c"; do @@ -14,7 +41,6 @@ if [[ ! -x "$T27C" ]]; then fi # `t27c suite` is the only subcommand that accepts --repo-root. # Map `tri test` to the suite subcommand; pass other commands through unchanged. -cmd="${1:-}" if [[ "$cmd" == "test" || "$cmd" == "suite" ]]; then # Forward everything after the subcommand to t27c suite, including --json. exec "$T27C" suite --repo-root "$REPO_ROOT" "${@:2}" diff --git a/scripts/tri_loop/cost.py b/scripts/tri_loop/cost.py new file mode 100644 index 0000000000..6381ca67b3 --- /dev/null +++ b/scripts/tri_loop/cost.py @@ -0,0 +1,274 @@ +#!/usr/bin/env python3 +"""tri cost -- parse cost per KB of spec, by stratum, with the spread shown. + +Why this is a rewrite and not a restore. + +An earlier version of this script was written, used to produce a scaling exponent +for the parser, and lost -- never committed, working copy re-cloned. Six recovery +routes were checked and came back empty (dangling objects, reflog, shell history, +CI artifacts, PR and issue comments, session snapshot). So this is a +reimplementation from a written contract. + +The defect the old output invited, which this one refuses to invite. + +Fitting one exponent across the whole corpus produces a number, and the number is +a property of the CORPUS COMPOSITION, not of the parser (this is #2133). The spec +families differ in kind, not just in size: a spec that is mostly a table of +constants and a spec that is mostly nested generic types both have a size in KB, +and mixing them means the fit is measuring which family happens to sit at which +size. A single alpha over a mixed sample is therefore not a scaling law, it is an +artefact of the sample, and printing it without that warning is how the previous +number got quoted as if it described the parser. + +So: + + * everything is reported per STRATUM, one stratum per spec directory family + * every stratum reports n, median, p95, min-max range of ms/KB, and CV + * the coefficient of variation is printed because it is the number that says + whether the median means anything: a stratum with CV above ~0.5 is not one + population and its median is a summary of a mixture + * alpha is printed ONLY per stratum, ONLY with n >= 8, ONLY with the size range + it was fitted over, and ONLY next to the CV. Cross-family alpha is refused + outright rather than printed with a caveat, because a printed number gets + quoted and the caveat does not travel with it. + +Also: absolute milliseconds from a debug build do not transfer to a release +build. Ratios and exponents transfer; absolute times do not, and this output +labels the build it measured so the distinction survives. + +Usage: + tri cost [glob-or-dir ...] [--limit N] [--repeat K] + [--json PATH] [--timeout SEC] [--min-kb F] + +Exit status: + 0 measured at least one stratum + 2 usage or setup error +""" +import glob as globmod +import json +import math +import os +import statistics +import subprocess +import sys +import time + +DEFAULT_TIMEOUT = 30 +MIN_N_FOR_ALPHA = 8 +CV_SUSPECT = 0.5 + + +def measure(binary, path, timeout, repeat): + """Return (ok, best_ms). Best of `repeat`, not the mean. + + Best-of is the right summary for a timing measurement whose noise is + one-sided: an interfering process can only make a run slower, never faster, + so the minimum is the least contaminated estimate of the cost. The mean would + fold the interference in and the p95 across files would then be reporting the + sandbox rather than the parser. + """ + best = None + for _ in range(repeat): + t0 = time.perf_counter() + try: + r = subprocess.run([binary, "parse", path], capture_output=True, + timeout=timeout) + except subprocess.TimeoutExpired: + return False, None + except OSError: + return False, None + dt = (time.perf_counter() - t0) * 1000.0 + if r.returncode != 0: + return False, None + best = dt if best is None else min(best, dt) + return True, best + + +def fit_alpha(sizes_kb, times_ms): + """Least-squares slope of log(time) against log(size). + + Returned with r2 so the caller can say how much of the variation the fit even + explains. A slope with r2 of 0.2 is a slope through a cloud. + """ + xs = [math.log(s) for s in sizes_kb] + ys = [math.log(t) for t in times_ms] + n = len(xs) + mx, my = sum(xs) / n, sum(ys) / n + sxx = sum((x - mx) ** 2 for x in xs) + if sxx == 0: + return None, None + sxy = sum((x - mx) * (y - my) for x, y in zip(xs, ys)) + slope = sxy / sxx + pred = [my + slope * (x - mx) for x in xs] + sst = sum((y - my) ** 2 for y in ys) + sse = sum((y - p) ** 2 for y, p in zip(ys, pred)) + r2 = None if sst == 0 else 1.0 - sse / sst + return slope, r2 + + +def stratum_of(path, corpus_root): + """The spec family: the directory holding the file, relative to the root.""" + rel = os.path.relpath(os.path.dirname(path), corpus_root) + return rel if rel != "." else "(root)" + + +def build_label(binary): + """debug or release, read off the path, so ms are never quoted context-free.""" + p = os.path.realpath(binary) + if f"{os.sep}release{os.sep}" in p: + return "release" + if f"{os.sep}debug{os.sep}" in p: + return "debug" + return "unknown-profile" + + +def summarise(rows): + per_kb = sorted(r["ms_per_kb"] for r in rows) + n = len(per_kb) + med = statistics.median(per_kb) + mean = statistics.fmean(per_kb) + sd = statistics.stdev(per_kb) if n > 1 else 0.0 + cv = (sd / mean) if mean else 0.0 + p95 = per_kb[min(n - 1, int(math.ceil(0.95 * n)) - 1)] + out = {"n": n, "median_ms_per_kb": med, "p95_ms_per_kb": p95, + "min_ms_per_kb": per_kb[0], "max_ms_per_kb": per_kb[-1], + "cv": cv, + "kb_min": min(r["kb"] for r in rows), + "kb_max": max(r["kb"] for r in rows)} + if n >= MIN_N_FOR_ALPHA: + a, r2 = fit_alpha([r["kb"] for r in rows], [r["ms"] for r in rows]) + out["alpha"] = a + out["alpha_r2"] = r2 + return out + + +def main(argv): + args = [a for a in argv if not a.startswith("--")] + if not args: + print(__doc__.split("Usage:")[1].strip(), file=sys.stderr) + return 2 + binary = args[0] + if not os.path.isfile(binary) or not os.access(binary, os.X_OK): + print(f"not an executable: {binary}", file=sys.stderr) + return 2 + targets = args[1:] or ["specs"] + + limit = None + repeat = 3 + out_json = None + timeout = DEFAULT_TIMEOUT + min_kb = 0.5 + for i, a in enumerate(argv): + if a == "--limit" and i + 1 < len(argv): + limit = int(argv[i + 1]) + if a == "--repeat" and i + 1 < len(argv): + repeat = int(argv[i + 1]) + if a == "--json" and i + 1 < len(argv): + out_json = argv[i + 1] + if a == "--timeout" and i + 1 < len(argv): + timeout = int(argv[i + 1]) + if a == "--min-kb" and i + 1 < len(argv): + min_kb = float(argv[i + 1]) + + files = [] + for t in targets: + if os.path.isdir(t): + for root, _d, names in os.walk(t): + files += [os.path.join(root, n) for n in names if n.endswith(".t27")] + else: + files += [p for p in globmod.glob(t, recursive=True) if p.endswith(".t27")] + files = sorted(set(files)) + corpus_root = targets[0] if os.path.isdir(targets[0]) else "specs" + if limit: + files = files[:limit] + if not files: + print("no .t27 files matched", file=sys.stderr) + return 2 + + rows, skipped, failed = [], 0, 0 + for path in files: + kb = os.path.getsize(path) / 1024.0 + if kb < min_kb: + skipped += 1 + continue + ok, ms = measure(binary, path, timeout, repeat) + if not ok: + failed += 1 + continue + rows.append({"file": path, "kb": kb, "ms": ms, "ms_per_kb": ms / kb, + "stratum": stratum_of(path, corpus_root)}) + + if not rows: + print("nothing measured: every file failed to parse or was below --min-kb", + file=sys.stderr) + return 2 + + strata = {} + for r in rows: + strata.setdefault(r["stratum"], []).append(r) + + profile = build_label(binary) + print(f"binary: {binary} [{profile} build]") + print(f"files: {len(rows)} measured, {skipped} below {min_kb} KB, " + f"{failed} failed to parse") + print(f"repeat: best of {repeat} runs per file\n") + + if profile != "release": + print("NOTE: absolute milliseconds from a non-release build do not") + print("transfer to release. Quote the exponents and the ratios; do not") + print("quote the ms.\n") + + hdr = (f"{'stratum':34s} {'n':>4s} {'med':>8s} {'p95':>8s} " + f"{'min':>8s} {'max':>8s} {'CV':>6s} {'alpha':>7s} {'r2':>5s} KB range") + print(hdr) + print("-" * len(hdr)) + results = {} + for name, rs in sorted(strata.items(), key=lambda kv: -len(kv[1])): + s = summarise(rs) + results[name] = s + a = f"{s['alpha']:7.2f}" if s.get("alpha") is not None else " -" + r2 = f"{s['alpha_r2']:5.2f}" if s.get("alpha_r2") is not None else " -" + flag = " *" if s["cv"] > CV_SUSPECT else "" + print(f"{name:34s} {s['n']:4d} {s['median_ms_per_kb']:8.2f} " + f"{s['p95_ms_per_kb']:8.2f} {s['min_ms_per_kb']:8.2f} " + f"{s['max_ms_per_kb']:8.2f} {s['cv']:6.2f} {a} {r2} " + f"{s['kb_min']:.1f}-{s['kb_max']:.1f}{flag}") + + print("\nms/KB columns are median, p95, min, max. CV is the coefficient of") + print("variation of ms/KB within the stratum.") + suspect = [n for n, s in results.items() if s["cv"] > CV_SUSPECT] + if suspect: + print(f"\n* {len(suspect)} stratum/strata have CV > {CV_SUSPECT}. In those the") + print(" median is a summary of a mixture and should not be quoted as the") + print(" cost of the stratum:") + for n in suspect[:12]: + print(f" {n} (CV {results[n]['cv']:.2f}, n={results[n]['n']})") + + noalpha = [n for n, s in results.items() if s.get("alpha") is None] + if noalpha: + print(f"\nalpha withheld for {len(noalpha)} stratum/strata with " + f"n < {MIN_N_FOR_ALPHA}.") + + print("\nNO CROSS-FAMILY ALPHA IS PRINTED, and this is deliberate.") + print("A single exponent fitted across these strata is a metric of corpus") + print("composition, not of the parser (#2133). It would be a number about") + print("which family happens to occupy which size band. Numbers get quoted") + print("and their caveats do not travel with them, so the number is not") + print("produced at all.") + print("\nWhere a per-stratum alpha IS printed, its applicability is exactly:") + print("this binary, this build profile, this stratum, this KB range, and") + print("nothing wider. Read r2 before reading alpha -- a slope with low r2 is") + print("a slope drawn through a cloud.") + + if out_json: + with open(out_json, "w") as fh: + json.dump({"binary": binary, "profile": profile, "repeat": repeat, + "files_measured": len(rows), "skipped": skipped, + "failed": failed, "strata": results, "rows": rows}, + fh, indent=2) + print(f"\nwrote {out_json}") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/scripts/tri_loop/damage.py b/scripts/tri_loop/damage.py new file mode 100644 index 0000000000..92b85e4417 --- /dev/null +++ b/scripts/tri_loop/damage.py @@ -0,0 +1,191 @@ +#!/usr/bin/env python3 +"""tri damage -- classify the mangled type annotations in the spec corpus (#2154). + +What this is for. + +A rewrite left behind lines whose type side is not a type: `public_key : [[]U8",` +instead of `public_key : U8`. Those lines are the floor under every field-set +measurement taken on this corpus -- 16 of the 18 files whose field sets moved +between two compiler binaries are in this set, which means the measurement was +reading the damage and not the parser. + +This tool only reports. It rewrites nothing. The point of a classifier is that a +later repair can be an inspectable diff of a stated size per class, instead of +one sweep over 115 lines that nobody can review. + +Class is decided by the shape of the type side after normalising every run of +identifier characters to `X` and every digit run to `9`, so `[[]U8",` and +`[[]Bool",` are the same class and get counted once. + +Usage: + tri damage [corpus-dir] [--json PATH] [--emit-fixtures DIR] [--class SHAPE] + +Exit status: + 0 no damaged lines found + 1 damaged lines found +""" +import json +import os +import re +import sys + +IDENT = re.compile(r"[A-Za-z_][A-Za-z0-9_]*") +DIGITS = re.compile(r"[0-9]+") +FIELD_LINE = re.compile(r"^\s*(?:pub\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*:\s*(.*)$") +# `r#"` opens a raw string that legitimately closes on a later line, so its lone +# quote is not damage. Found by reading the 3 lines the odd-quote signal flagged +# in specs/pins/parser.t27 rather than by trusting the signal. +RAW_STRING_OPEN = re.compile(r'\br#"') + + +def shape(rhs): + """Normalise a type side to its shape: identifiers -> X, digits -> 9.""" + s = IDENT.sub("X", rhs) + s = DIGITS.sub("9", s) + s = re.sub(r"\s+", " ", s).strip() + return s + + +def is_damaged(rhs): + """A type side that cannot be a type, with the reason. + + Two signals only, and the history of the third, fourth and fifth is the point + of this comment. + + The first draft of this function also flagged an unbalanced `<` or `>` and an + unbalanced `[`. Run over the corpus it reported 429 damaged lines. 230 of + those were `target : < 5000ns` -- a legitimate constraint bound where `<` is + the less-than operator, not a bracket. Another 17 were match arms + (`ConstraintType : :TEMPORAL => {`), 10 were the opening line of a multi-line + array literal, and 6 were function signatures that the field-line regex had + no business matching. So the headline 429 measured this function's regex, not + the corpus, and dropping the two bad signals is the fix -- not tuning a + threshold until the number looks right. + + What survives are the two signals that cannot occur in any well-formed type + annotation and cannot occur in a legitimate operator either: + + doubled-bracket `[[]` -- an empty slice inside a slice, which the language + has no syntax for + odd-quote an odd number of `"` on the line, so a string opens and + never closes + + Both are textual and never consult the parser: asking the parser whether its + own input was malformed is circular, since the malformed reading is exactly + what is in question. + """ + reasons = [] + if "[[]" in rhs or "[?[]" in rhs: + reasons.append("doubled-bracket") + if rhs.count('"') % 2 == 1 and not RAW_STRING_OPEN.search(rhs): + reasons.append("odd-quote") + return reasons + + +def scan(corpus): + rows = [] + for root, _dirs, names in os.walk(corpus): + for n in sorted(names): + if not n.endswith(".t27"): + continue + path = os.path.join(root, n) + try: + with open(path, "r", errors="replace") as fh: + lines = fh.readlines() + except OSError: + continue + for ln, line in enumerate(lines, 1): + m = FIELD_LINE.match(line.rstrip("\n")) + if not m: + continue + field, rhs = m.group(1), m.group(2).strip() + if not rhs: + continue + reasons = is_damaged(rhs) + if reasons: + rows.append({"file": path, "line": ln, "field": field, + "rhs": rhs, "shape": shape(rhs), + "reasons": reasons, + "family": os.path.dirname(path)}) + return rows + + +def main(argv): + args = [a for a in argv if not a.startswith("--")] + corpus = args[0] if args else "specs" + out_json = None + emit = None + only = None + for i, a in enumerate(argv): + if a == "--json" and i + 1 < len(argv): + out_json = argv[i + 1] + if a == "--emit-fixtures" and i + 1 < len(argv): + emit = argv[i + 1] + if a == "--class" and i + 1 < len(argv): + only = argv[i + 1] + + rows = scan(corpus) + if only: + rows = [r for r in rows if r["shape"] == only] + + files = sorted({r["file"] for r in rows}) + shapes = {} + for r in rows: + shapes.setdefault(r["shape"], []).append(r) + fams = {} + for r in rows: + fams[r["family"]] = fams.get(r["family"], 0) + 1 + + print(f"corpus: {corpus}") + print(f"damaged lines: {len(rows)} in {len(files)} files, " + f"{len(shapes)} distinct shapes\n") + print("by shape (this is the repair unit -- one reviewable diff per row):") + for sh, rs in sorted(shapes.items(), key=lambda kv: -len(kv[1])): + reasons = sorted({x for r in rs for x in r["reasons"]}) + print(f" {len(rs):5d} {sh!r:28s} {','.join(reasons)}") + print(f" e.g. {rs[0]['file']}:{rs[0]['line']} " + f"{rs[0]['field']} : {rs[0]['rhs']}") + print("\nby directory (top 12):") + for fam, n in sorted(fams.items(), key=lambda kv: -kv[1])[:12]: + print(f" {n:5d} {fam}") + + if out_json: + with open(out_json, "w") as fh: + json.dump({"corpus": corpus, "lines": len(rows), + "files": files, "shapes": {k: len(v) for k, v in shapes.items()}, + "rows": rows}, fh, indent=2) + print(f"\nwrote {out_json}") + + if emit: + os.makedirs(emit, exist_ok=True) + made = [] + for idx, (sh, rs) in enumerate( + sorted(shapes.items(), key=lambda kv: -len(kv[1])), 1): + r = rs[0] + name = f"damage_class_{idx:02d}" + body = (f"module {name}\n\n" + f"// shape: {sh}\n" + f"// {len(rs)} line(s) in the corpus share this shape\n" + f"// first seen: {r['file']}:{r['line']}\n" + f"// signals: {','.join(r['reasons'])}\n" + f"pub struct Damaged {{\n" + f" {r['field']} : {r['rhs']}\n" + f"}}\n") + p = os.path.join(emit, f"{name}.t27") + with open(p, "w") as fh: + fh.write(body) + made.append((p, sh, len(rs))) + print(f"\nemitted {len(made)} class fixtures into {emit}:") + for p, sh, n in made: + print(f" {p} ({n} lines, shape {sh!r})") + + print("\nNot claimed: that these are all the damage. This checks two textual") + print("signals on lines that look like field declarations. Damage that") + print("produces a balanced, quote-even, plausible-looking wrong type is") + print("invisible here, and nothing in this output bounds how much of that") + print("there is.") + return 1 if rows else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/scripts/tri_loop/diffbin.py b/scripts/tri_loop/diffbin.py new file mode 100644 index 0000000000..f372e2322c --- /dev/null +++ b/scripts/tri_loop/diffbin.py @@ -0,0 +1,333 @@ +#!/usr/bin/env python3 +"""tri diffbin -- compare two t27c binaries over a spec corpus, by category. + +Why this exists, and why it is a rewrite rather than a restore. + +An earlier version of this script was written, used to produce a "634 specs, 0 +regressions" figure, and lost: it was never committed to any branch, and the +sandbox holding the working copy was re-cloned. Six recovery routes were checked +and all came back empty -- dangling git objects (only a `git stash` WIP holding +`triage.py`), the reflog (which shows the clone, i.e. the loss), shell history +(absent), CI artifacts (only FPGA build outputs), PR and issue comments (no +pasted source), and the session snapshot (prose about the script, not the +script). So this is a reimplementation from an explicit written contract, not a +reconstruction from memory of what the old one did. + +That is the better outcome anyway, because the old aggregate was wrong. + +The defect in the previous tool. + +It reported "0 regressions" over 634 specs. It was also true that 17 files lost +struct fields: on input like `a : Map [corpus-dir] [--limit N] + [--jsonl PATH] [--timeout SEC] [--include-scratch] + +Exit status: + 0 no field-loss and no unknown + 1 at least one field-loss or unknown file + 2 usage or setup error +""" +import json +import os +import subprocess +import sys +from collections import Counter + +CATEGORIES = ( + "unchanged", + "field-loss", + "strict-improvement", + "malformed-input-tradeoff", + "unknown", +) +DEFAULT_TIMEOUT = 25 + + +def _unquote(line, key): + t = line.strip() + if not t.startswith(key): + return None + rest = t[len(key):].strip() + if not rest.startswith('"'): + return None + rest = rest[1:] + end = rest.rfind('"') + if end < 0: + return None + return rest[:end] + + +def parse_fields(binary, path, timeout): + """Return (status, [(qualified_name, type_text)]). + + status is one of ok / error / timeout. On error or timeout the field list is + empty, and the categoriser treats a status change as its own outcome rather + than reading an empty list as "all fields lost". + + Only an ExprIdentifier whose PARENT node is a StructDecl counts as a struct + field. `t27c parse` prints the same node kind for every identifier in the + tree, including every identifier in a function body, so matching on the kind + alone would count expression operands as fields and make the corpus totals + meaningless. Parenthood is read off the indentation of the `kind:` lines: the + parent of a node is the nearest preceding `kind:` line with strictly smaller + indentation. Names are qualified with the struct name, so that two structs in + one file each declaring `value` stay distinguishable. + """ + try: + r = subprocess.run([binary, "parse", path], capture_output=True, + timeout=timeout, text=True, errors="replace") + except subprocess.TimeoutExpired: + return "timeout", [] + except OSError as e: + return f"spawn-error: {e}", [] + if r.returncode != 0: + return "error", [] + lines = (r.stdout + r.stderr).splitlines() + out = [] + stack = [] # (indent, kind, name) + for i, line in enumerate(lines): + stripped = line.strip() + if not stripped.startswith("kind: "): + continue + indent = len(line) - len(line.lstrip()) + kind = stripped[len("kind: "):].rstrip(",") + name = _unquote(lines[i + 1], "name:") if i + 1 < len(lines) else None + while stack and stack[-1][0] >= indent: + stack.pop() + parent = stack[-1] if stack else None + if kind == "ExprIdentifier" and parent and parent[1] == "StructDecl": + ty = _unquote(lines[i + 3], "extra_type:") if i + 3 < len(lines) else None + if name is not None and ty is not None: + out.append((f"{parent[2]}.{name}", ty)) + stack.append((indent, kind, name or "")) + return "ok", out + + +def truncated(ty): + """A type text with an unclosed bracket, angle or paren.""" + return (ty.count("<") > ty.count(">") + or ty.count("(") > ty.count(")") + or ty.count("[") > ty.count("]")) + + +def input_is_malformed(path): + """Decide malformed-ness from the SOURCE, never from parser behaviour. + + Deciding it from the parser would make the categoriser circular: any change + in output would justify itself as "the input must have been malformed". + These are textual signals only, and each one is a thing that cannot appear in + a well-formed field declaration: + + * an unbalanced `"` on a field line + * `[[` immediately before `]` + * a field line whose type side has more openers than closers + """ + try: + with open(path, "r", errors="replace") as fh: + lines = fh.readlines() + except OSError: + return False, "unreadable" + signals = [] + for n, line in enumerate(lines, 1): + if ":" not in line: + continue + if line.count('"') % 2 == 1: + signals.append(f"{n}:odd-quote") + continue + if "[[]" in line: + signals.append(f"{n}:doubled-bracket") + continue + rhs = line.split(":", 1)[1] + if truncated(rhs.rstrip().rstrip(",")): + signals.append(f"{n}:unclosed-type") + return bool(signals), ",".join(signals[:6]) + + +def categorise(base, cand, path): + """Return (category, reason, detail dict).""" + bstat, bfields = base + cstat, cfields = cand + + if bstat != cstat: + if bstat == "ok" and cstat in ("error", "timeout"): + return "field-loss", f"base parsed, candidate {cstat}", {} + if bstat in ("error", "timeout") and cstat == "ok": + return "strict-improvement", f"base {bstat}, candidate parsed", {} + return "unknown", f"status {bstat} -> {cstat}", {} + if bstat != "ok": + return "unchanged", f"both {bstat}", {} + + if bfields == cfields: + return "unchanged", "identical field set", {} + + bnames = [n for n, _ in bfields] + cnames = [n for n, _ in cfields] + btypes = dict(bfields) + removed = [n for n in bnames if n not in cnames] + added = [n for n in cnames if n not in bnames] + + phantoms = [n for n in removed if btypes.get(n, "") == ""] + real_lost = [n for n in removed if btypes.get(n, "") != ""] + + detail = {"removed": removed, "added": added, + "phantoms": phantoms, "real_lost": real_lost, + "base_n": len(bfields), "cand_n": len(cfields), + "base_trunc": sum(1 for _, t in bfields if truncated(t)), + "cand_trunc": sum(1 for _, t in cfields if truncated(t))} + + if real_lost: + return ("field-loss", + f"declared field(s) gone with non-empty base type: {real_lost}", + detail) + + if phantoms: + return ("strict-improvement", + f"phantom field(s) removed (empty base type): {phantoms}", + detail) + + if detail["cand_trunc"] < detail["base_trunc"]: + return ("strict-improvement", + f"truncated type texts {detail['base_trunc']} -> {detail['cand_trunc']}", + detail) + + malformed, signals = input_is_malformed(path) + if malformed: + return ("malformed-input-tradeoff", + f"field set moved on malformed input ({signals})", detail) + + return ("unknown", + "field set moved on well-formed input with no loss and no improvement", + detail) + + +def main(argv): + args = [a for a in argv if not a.startswith("--")] + if len(args) < 2: + print(__doc__.split("Usage:")[1].strip(), file=sys.stderr) + return 2 + base_bin, cand_bin = args[0], args[1] + corpus = args[2] if len(args) > 2 else "specs" + for b in (base_bin, cand_bin): + if not os.path.isfile(b) or not os.access(b, os.X_OK): + print(f"not an executable: {b}", file=sys.stderr) + return 2 + + limit = None + jsonl = None + timeout = DEFAULT_TIMEOUT + include_scratch = "--include-scratch" in argv + for i, a in enumerate(argv): + if a == "--limit" and i + 1 < len(argv): + limit = int(argv[i + 1]) + if a == "--jsonl" and i + 1 < len(argv): + jsonl = argv[i + 1] + if a == "--timeout" and i + 1 < len(argv): + timeout = int(argv[i + 1]) + + files = [] + for root, _dirs, names in os.walk(corpus): + if not include_scratch and f"{os.sep}scratch" in root: + continue + for n in sorted(names): + if n.endswith(".t27"): + files.append(os.path.join(root, n)) + files.sort() + if limit: + files = files[:limit] + if not files: + print(f"no .t27 files under {corpus}", file=sys.stderr) + return 2 + + counts = Counter() + rows = [] + fh = open(jsonl, "w") if jsonl else None + for path in files: + b = parse_fields(base_bin, path, timeout) + c = parse_fields(cand_bin, path, timeout) + cat, reason, detail = categorise(b, c, path) + counts[cat] += 1 + row = {"file": path, "category": cat, "reason": reason, **detail} + rows.append(row) + if fh: + fh.write(json.dumps(row) + "\n") + fh.flush() + if fh: + fh.close() + + print(f"corpus: {len(files)} specs under {corpus}" + f"{'' if include_scratch else ' (scratch excluded)'}") + print(f"base: {base_bin}") + print(f"candidate: {cand_bin}\n") + for cat in CATEGORIES: + print(f" {counts.get(cat, 0):5d} {cat}") + + for cat in ("field-loss", "unknown", "strict-improvement"): + sel = [r for r in rows if r["category"] == cat] + if not sel: + continue + print(f"\n{cat} ({len(sel)}):") + for r in sel[:40]: + print(f' {r["file"]}') + print(f' {r["reason"]}') + if len(sel) > 40: + print(f" ... {len(sel) - 40} more (use --jsonl for the full list)") + + lost = counts.get("field-loss", 0) + unknown = counts.get("unknown", 0) + print() + if lost or unknown: + print(f"NOT CLEAN: {lost} field-loss, {unknown} unknown.") + print("These are NOT to be aggregated away. field-loss means a field the") + print("author declared is gone from the parse. It may still be the right") + print("trade to make -- that is a language decision about what malformed") + print("input should mean -- but it is a decision, not a measurement, and") + print("it does not belong inside a count of zero.") + else: + print("CLEAN: no field-loss and no unknown.") + print("This says nothing about categories the tool does not check:") + print("generated code, type inference, diagnostics, or timing.") + return 1 if (lost or unknown) else 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/scripts/tri_loop/triage.py b/scripts/tri_loop/triage.py new file mode 100644 index 0000000000..12cf64c90e --- /dev/null +++ b/scripts/tri_loop/triage.py @@ -0,0 +1,172 @@ +#!/usr/bin/env python3 +"""tri triage -- classify every open issue into one of five ordered classes. + +Why this exists. + +The plan in #1697 measured "~47 of 102 open issues are autonomous wave-loop +journal noise". The ratio has moved since: of 237 open issues, roughly 80% +record work that was already done rather than work that is waiting. + +A record with no completion condition is not a task. Counting it as backlog is +the same composition error #2133 identified in the ratchet, applied to the +tracker instead of the corpus. This tool reports the split so that any statement +about "N open issues" can be read correctly. + +What changed, and why three classes were not enough. + +The earlier version had three classes -- journal, plan, actionable -- and +everything that did not look like a journal entry fell into `actionable` by +default. That default is the flaw. An issue waiting on hardware, an issue that +asks a research question with no defined answer, and an issue that duplicates +another are all things a loop cannot pick up and finish, yet all three were +counted as available work. The count of "~26 actionable" was therefore an upper +bound being read as an estimate. + +Five ordered classes, first match wins: + + actionable a defect or change with a checkable completion condition + research an open question; done is a matter of judgement, not a check + tracking a journal entry, plan or epic; records or aggregates, never closes + blocked cannot proceed without something outside the tool: hardware, an + upstream release, a human decision, an expired credential + duplicate superseded, obsolete, or the same subject as an earlier issue + +Ordering matters and is deliberate: `blocked` is tested before `actionable`, so +a blocked defect is never advertised as available. `duplicate` is tested last, +because a duplicate that is also blocked is more usefully seen as blocked. + +Autoclose is forbidden. This tool prints and exits; it never mutates the +tracker. The classification reads titles and labels, and reads bodies only for +the blocked and duplicate signals, where the evidence is not in the title. It +remains a composition estimate, never a verdict on an individual issue. + +Usage: + tri triage [owner/repo] [--json] [--class NAME] [--bodies] +""" +import json +import re +import subprocess +import sys +from collections import Counter + +CLASSES = ("actionable", "research", "tracking", "blocked", "duplicate") + +TRACKING = re.compile( + r"^(wave\s|wave\s*loop|feat\(igla\):\s*wave|formal:.*\(prop\.|tick\s" + r"|\[plan\]|epic\b|census\b|report:)", re.I +) +RESEARCH = re.compile( + r"(\?$|^(research|investigate|explore|study|survey|consider|should we|why\s)" + r"|\bopen question\b|\bhypothes)", re.I +) +BLOCKED = re.compile( + r"\b(blocked\s+(on|by)|waiting\s+(on|for)|needs\s+hardware|requires\s+the\s+board" + r"|routing[- ]pending|deferred\b|upstream\s+bug|awaiting\s+review" + r"|needs\s+a\s+human|cannot\s+proceed\s+until)\b", re.I +) +DUPLICATE = re.compile( + r"\b(duplicate\s+of|superseded\s+by|obsolete\b|closed\s+in\s+favour|see\s+instead)\b", + re.I +) +LABEL_MAP = { + "blocked": "blocked", "wontfix": "duplicate", "duplicate": "duplicate", + "question": "research", "research": "research", "journal": "tracking", + "epic": "tracking", "plan": "tracking", +} + + +def klass(row, use_bodies=True): + """Return (class, reason). First match in CLASSES order wins.""" + title = row.get("title") or "" + body = (row.get("body") or "") if use_bodies else "" + labels = {(l.get("name") or "").lower() for l in row.get("labels") or []} + + for name, cls in LABEL_MAP.items(): + if name in labels: + return cls, f"label:{name}" + + m = BLOCKED.search(title) or (BLOCKED.search(body[:2000]) if body else None) + if m: + return "blocked", f"phrase:{m.group(0).strip().lower()[:40]}" + + m = TRACKING.search(title) + if m: + return "tracking", f"title-form:{m.group(0).strip().lower()[:40]}" + + m = RESEARCH.search(title) + if m: + return "research", f"title-form:{m.group(0).strip().lower()[:40]}" + + m = DUPLICATE.search(title) or (DUPLICATE.search(body[:2000]) if body else None) + if m: + return "duplicate", f"phrase:{m.group(0).strip().lower()[:40]}" + + return "actionable", "default: no tracking, research, blocked or duplicate signal" + + +def main(argv): + repo = "gHashTag/t27" + as_json = "--json" in argv + use_bodies = "--bodies" in argv or True + want = None + if "--class" in argv: + i = argv.index("--class") + if i + 1 < len(argv): + want = argv[i + 1].lower() + if want not in CLASSES: + print(f"unknown class {want!r}; expected one of {', '.join(CLASSES)}", + file=sys.stderr) + return 2 + positional = [a for a in argv if not a.startswith("--")] + positional = [a for a in positional if a != want] + if positional: + repo = positional[0] + + fields = "number,title,labels,createdAt,comments" + if use_bodies: + fields += ",body" + out = subprocess.run( + ["gh", "issue", "list", "--repo", repo, "--state", "open", "--limit", "1000", + "--json", fields], + capture_output=True, text=True) + if out.returncode != 0: + print(out.stderr.strip(), file=sys.stderr) + return 2 + rows = json.loads(out.stdout) + + for r in rows: + r["class"], r["reason"] = klass(r, use_bodies) + r.pop("body", None) + counts = Counter(r["class"] for r in rows) + + if as_json: + payload = {"repo": repo, "open": len(rows), + "counts": {c: counts.get(c, 0) for c in CLASSES}, + "issues": sorted(rows, key=lambda r: r["number"])} + if want: + payload["issues"] = [r for r in payload["issues"] if r["class"] == want] + print(json.dumps(payload, indent=2)) + return 0 + + total = len(rows) or 1 + print(f"{repo}: {len(rows)} open") + for c in CLASSES: + v = counts.get(c, 0) + print(f" {v:5d} {c:<11s} ({100 * v / total:.0f}%)") + + for c in ([want] if want else ["actionable", "blocked"]): + sel = sorted((r for r in rows if r["class"] == c), key=lambda r: r["number"]) + print(f"\n{c} ({len(sel)}):") + for r in sel: + print(f' #{r["number"]:<6d} {r["createdAt"][:10]} {r["title"][:88]}') + print(f' {r["reason"]}') + + print("\nNOTE: a composition estimate from titles, labels and body prefixes -- not a") + print(" verdict on any single issue. This tool never closes anything, and") + print(" nothing here licenses a bulk action. `blocked` is tested before") + print(" `actionable` on purpose, so no blocked item is advertised as available.") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:]))