feat: 2.1.0 — read what the tool was handed - #47
Merged
Merged
Conversation
Six findings that share a shape: something was consumed on a contract other than the one it is written to. **JSONL.** reconverge prints one findings document per analyzed *target*, and this reader handed the whole of stdout to a single `from_str` — so a kernel crate with a `src/main.rs` beside its library, the ordinary shape of a GPU crate, hard-stopped every candidate with `trailing characters at line 2 column 1`. The fail-safe held; it held against a format the analyzer documents. Stdout is read as JSONL now and the findings are unioned: a deny finding in any target is a reason to refuse, and the bin target's document is harmless to merge. A parse failure carries the first two hundred bytes of what was received, because the serde message told the reporter everything and would tell a user nothing. **The scratch copy.** It took only the entries of `src/` that are files, so `mod util;` with `src/util/mod.rs` produced a scratch crate that could not compile — and the gate reported `error: could not compile` against a crate whose own `cargo check` is clean, telling its author to fix errors they do not have or reinstall their toolchain, and never saying that what it compiled was not what they were looking at. It copies recursively now, carries `build.rs` (or whatever `package.build` names), and a tool error names the scratch directory so the next person can go and look. **The diagnosis filter.** rustc's primary diagnostics begin `error[E0583]:` — a code before the colon — so a filter on `error:` kept cargo's summary and dropped the line that names the failure. "See the errors above": the one above was the one the filter removed. Both forms are accepted, in the prune runner and in the compile executor, which had the tail-eight heuristic #19 removed next door for exactly this reason. **`--budget`.** `split_at(text.len() - 1)` on a trimmed-empty argument is `0usize - 1`, so `--budget ""` panicked at exit 101. Worse, `NaN` and `1e400` parsed: the guard is `elapsed >= budget`, which is false for every value against NaN and never true against infinity, so a value that looked accepted produced an unbounded sweep on real silicon — the one failure a budget exists to prevent. Rejected now, with the accepted forms named, and `min`/`hr` accepted alongside `m`/`h` because that is what people type. **`apply`.** Verification ran *after* the params.rs was printed, so "refusing to emit" arrived after the emission. It now decides first. On a Metal run — which has no convergence gate, deliberately — it refuses by name and points at `--no-verify` or `prune --cc <target>`, instead of handing the `metal` sentinel to reconverge and reporting the correct answer to the wrong question as a regression. `--verify` is a real switch with a `--no-verify` counterpart, so a machine without the analyzer can emit, with the notice carried into the output beside the Metal one. And `Verdict` has a `Display`, so no user-facing message is a struct literal. **`results.json`.** A truncated file, an empty one, `null`, `[]`, a `results.v2` and a *directory* all read as "the box has not run yet": exit 0, nothing on stderr, a schema-valid report saying `unmeasured`. The run directory is the hand-off between two machines and those two conditions call for opposite actions — wait, or go and look. Only `NotFound` is `Ok(None)` now; everything else names the path and the cause, the way `verdicts.v1` already did fifteen lines away. Also: the reconverge pin moves to 0.5.0 at all seven sites, and scripts/check-pins.sh asserts they agree with no network — 2.0.0 moved four of six, which left the watcher measuring drift from a version nothing installs and #17 describing a pin two releases old. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
#24 fixed this on the chosen line and left it standing in the two views below. The ranking lost every closing bracket at eighty columns, so each interval read as a number with no upper bound and a dangling comma. The rejections view — the one the README calls the point of the tool — lost the half of the reason that says what to do: the reader got as far as `splits a 64-threa` and never reached `safe only at one warp (<= 32 threads)`. Nothing marked the cut, so it read as the whole reason. The two want different treatment and that is the substance of the fix. An interval is a field, so the ranking drops it whole when it does not fit, through the same helper the chosen line uses — shared so the two views cannot disagree about it again, which is how this came to be fixed once and missed twice. A rejection reason is a sentence whose actionable clause is at the end, so it wraps instead, and the panel spends the two or three rows. The guard is the scan that found both: no golden line may reach the panel border with a character that means the value continued — a digit, `,`, `[`, `(`, `=`, `-` — or with a cut word, unless an ellipsis marks it. Confirmed red against the shipped pre-fix golden, which is also what it would have done to #24. Alongside: `prune`'s verdict line now says what it checked and what it did not. The gate answers convergence and shared-memory capacity at a `--cc`; it has no view of instruction availability, so a crate using an `sm_80+` intrinsic under `needs_cc = "7.5"` prunes to `12 clean` at `--cc 7.5` and fails only when something finally lowers it. That is defensible and was nowhere stated, so "3 clean" read as "this kernel is fine at cc 7.5". docs/LIMITATIONS.md carries the long form, including why the two stronger fixes were not built. Tests: the JSONL union and every way it can fail, a scratch copy over a crate with `src/util/inner.rs` and a manifest-named build script, a `error[E0583]:` line surviving the diagnosis filter, and each of the six unreadable `results.json` shapes. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
A property of the rendered grid rather than of a string: the reason is wrapped across rows now, so the sentence exists only as a sequence of cells. Sixty columns is where wrapping has to do real work and no golden covers it — which is also how the test found that `ready` cannot be used at that width, because the footer is cut before it reaches `q quit`. Confirmed red with the wrap removed. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
Thirteen findings, all reported against 2.0.0 with a measured reproduction, and the lockstep pin bump to reconverge 0.5.0 that #46 and #17 were waiting on. Verified end to end against the *published* cargo-reconverge 0.5.0 rather than a local build: the two shapes that hard-stopped the gate — a kernel crate with a `src/main.rs`, and one with a `src/util/mod.rs` — now give `3 clean, 0 with caveats, 8 refused, 0 tool errors` at exit 0, identical to the control with neither. The whole corpus prunes unchanged, and the three gate tests pass, so the pin bump is behaviour-preserving where it should be. Also here: `prune`'s "what was and was not checked" note is printed once for the run rather than once per kernel — over a six-kernel corpus the per-kernel form was six repetitions of the same paragraph, which is the kind of user-facing noise #27 was about. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
…laims Three defects in the gate I added, two of which it exists to prevent. `declare -A` and `local -n` are bash 4. macOS ships bash 3.2, where an associative array is silently an indexed one, so the script passed on Linux and died on the macOS runner with `rust: unbound variable` — the same defect as the GNU-only `sed -i` this project's sibling had in its own gate scripts. Rewritten with tab-separated lines and a pipeline, which is portable. `agree` runs in a pipeline and therefore a subshell, so setting `status=1` inside it would have been discarded: the gate would have printed the disagreement and exited 0. It returns a status now and the caller collects it. That is precisely the class of bug this gate is for. And the nightly was never compared at all. The action writes `default: nightly-2026-04-03` unquoted, the extractor required quotes, and the old code treated an unreadable site as nothing to check — so the one pin with only two recorded sites had neither of them verified. An extractor that silently matches nothing is a stale pin one level up, so an empty read is now a failure by name. Each of the three pin families verified red independently, with exit 1. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #17, closes #32, closes #33, closes #34, closes #35, closes #36, closes #37, closes #38, closes #42, closes #43, closes #44, closes #45, closes #46.
All 13 open findings, each verified against the reproduction it carries, plus the reconverge 0.5.0 pin bump and the 2.1.0 release.
The theme
Most of these reduce to one shape: something was consumed on a contract other than the one it is written to, and the mismatch was reported as a fault of whatever it was pointed at.
src/main.rsbeside a kernel library — the ordinary shape of a GPU crate — hard-stopped every candidate, and the message pointed at the analyzer's output, so the next stop was reconverge's tracker, where nothing was wrong.src/; the crate is a tree. A crate withsrc/util/mod.rsgoterror: could not compileagainst acargo checkthat is clean, and was told to fix build errors it does not have.error:; rustc writeserror[E0583]:. "See the errors above" — the one above was the one the filter removed.applyhanded a Metal run'sgate_cc: "metal"sentinel to a compute-capability parser, and reported the correct answer to the wrong question as a regression.results.jsonwas read with.ok(), so a truncated file and an absent one both meant "the box has not run yet" — two conditions calling for opposite actions.Verified against the published analyzer, not a local build
The two shapes that hard-stopped the gate now give
3 clean, 0 with caveats, 8 refused, 0 tool errorsat exit 0 — identical to the control with neither. The whole corpus prunes unchanged and the threejust gatetests pass againstcargo-reconverge 0.5.0installed from crates.io.Watched going red first
scripts/check-pins.shagainst a single reverted pin site.no_golden_line_is_cut_at_the_panel_borderagainst the shipped pre-fix goldens — the same scan would have caught The chosen configuration's confidence interval is cut in half at eighty columns #24.a_refusal_reason_survives_a_narrow_terminal_wholewith theWrapremoved.Notable scope calls
--budgetrejects non-finite values, and that is the substance.NaNand1e400both parsed, and the guard iselapsed >= budget— false for every value against NaN, never true against infinity. A value that looked accepted produced an unbounded sweep on real silicon, which is the one failure a budget exists to prevent. The panic on""was the visible half.--verifycannot be turned off, and its help does not say it is already on #36 took option 2, not option 1.--verifyis a real switch with--no-verify, rather than dropping the flag and keeping verification mandatory. That is what makesapplycan never emit for a Metal run, and says a regression happened when nothing did #34's Metal refusal actionable — there is now something to do about it — and lets a machine with the run directory but not the analyzer emit, with the notice carried into the output beside the Metal one.--cchelp anddocs/LIMITATIONS.mdnow say so, and the note prints once per run rather than once per kernel. Thecargo oxide build --archprobe needs a toolkit, andpruneis deliberately the part a laptop can run. A static scan against the catalog'sAvailable on sm_NN+lines needs the catalog — the sibling checkoutpruneexists not to require — and an embedded copy would go stale silently, which is the failure mode that document is about. Both are stated as not-built rather than left implied.What this leaves out
#37also notesstderr.lines().rev().take(8)in the compile executor. Fixed — but the missing-cargo oxidepath is reproduced with a shim, not a real absent checkout, and the compile-failure path below it is reasoned from the code, as the issue itself was.