diff --git a/.github/workflows/loop-tools-gate.yml b/.github/workflows/loop-tools-gate.yml new file mode 100644 index 000000000..e61f59861 --- /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 000000000..f58109de5 --- /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 000000000..67ab30ab1 --- /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 000000000..f8f3acf3f --- /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 000000000..5f6e2e55e --- /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 000000000..61147193b --- /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 000000000..cccdb197b --- /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 000000000..da3ec5735 --- /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 000000000..d81e4e9be --- /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 000000000..c070a80be --- /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 000000000..096cf0e18 --- /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 000000000..1cd48eca6 --- /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 000000000..689a32ea5 --- /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 000000000..5c6756ca4 --- /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 000000000..0b4bfaf39 --- /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 000000000..85a5082ae --- /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 000000000..6bbe35ead --- /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 000000000..a08c37c7c --- /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 000000000..1977f275d --- /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 000000000..0eb0e5fb4 --- /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 000000000..fc491e050 --- /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 e012eb108..1313b0ed0 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 000000000..6381ca67b --- /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 000000000..92b85e441 --- /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 000000000..f372e2322 --- /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 000000000..12cf64c90 --- /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:]))