Guard the parity pin header against declaration-order breaks (#558) - #594
Merged
Conversation
…header main already fixed (#558) FOLLOWING_AGENTS_PROTOCOL `main` fixed the declaration-order break itself at `fafa16f0f` (PR #556) while this branch carried its own repair, and #551/#546 are CLOSED. Two independent moves of one block do not conflict: merging the two repairs auto-merged clean into a header that DEFINED `Nemotron35LightningSnapshot()` twice and re-broke the same 14 TUs. So this branch was RESET onto `fafa16f0f` and re-applies only the part that is still owed -- the guard, now tracked by #558. `git diff origin/main -- tests/parity/hf_snapshot.h` is EMPTY: main's header survives byte-for-byte (md5 048c84eb5cc20146f4be45f7747b8a34). WHY THE GUARD IS STILL WANTED. Nothing else in the Python lane sees the defect -- delete this method and `scripts/check-snapshot-pins.py` exits 0 on a header no TU can include. A full C++ build does catch it, and that is exactly the point: the commit that broke it was records and evidence and never built C++ at all, and all 14 TUs that include the header are checkpoint-gated, so a serial `ctest` reported `***Not Run`, which on a box with no snapshots reads as a missing checkpoint rather than a build break. This suite already runs from `scripts/agent-preflight.sh` and from CI's record lane (`ci.yml:172`), needs no CMake, no build tree, no GPU and no checkpoint, and costs ~0.3s. TWO REVIEW FINDINGS CLOSED WITH IT. 1. A SKIP WAS INVISIBLE. `skipTest` exits 0 and `agent-preflight.sh`'s `run()` prints a green `ok` while swallowing stdout, so a compiler-less box turned the guard into a silent no-op. Measured before: `env -u CXX CI=1 PATH=/nonexistent python3 -m unittest ...` -> `OK (skipped=1)`, exit 0. It now FAILS when `CI` is set and skips otherwise: with `CI=1` and no compiler, `AssertionError: CI must be able to syntax-check tests/parity/hf_snapshot.h ... A skip here exits 0 and reads as a pass`, exit 1; without `CI`, `OK (skipped=1)`, exit 0. 2. `$CXX` LOST ITS LAUNCHER. `configured[:1]` kept only the first token, so `ccache g++` / `sccache clang++` / `distcc g++` ran the LAUNCHER with the compiler's flags. Measured before, with `CXX="env g++"`: `AssertionError: 0 != 125 ... /usr/bin/env: invalid option -- 's'` -- a red naming this header for a defect in the environment. The whole token list is now the argv prefix; the same `CXX="env g++"` is green. A candidate that cannot be LAUNCHED (126/127) falls through to the next; every other status is a compiler's verdict and is reported, so `CXX` exiting 1 is still RED and is not masked by the fallback. NEGATIVE MUTATION, against main's fixed header, in a scratch tree (the repo copy was never touched -- md5 unchanged before and after): * M1, the #551 ordering restored: FAILED, `'HfSnapshot' was not declared in this scope`, quoted verbatim by the assertion message. * M2, the auto-merge hazard above (the block defined twice): FAILED, `redefinition of 'std::string parity::Nemotron35LightningSnapshot()'`. No other check in the repo sees that one. The intake row is #558 and not #551: the table is for OPEN issues, #551 closed at 2026-08-12T23:53:23Z, and the row `f2fe496f` added went with the reset. `Row` is `—` -- the guard owns no capability, so no matrix row and no count that `check-agent-record.py` enforces changes. Placement is sorted, not appended: within the `—` entries the file orders by issue number ascending and 558 is the highest, so its sort position is the last row. The anchor was asserted unique (count == 1) before the insert; the result is 1 insertion, 0 deletions. Gates on the final tree: clean out-of-tree `-Werror` Release build on local disk (`build-gate/`, CPU: no CUDA toolkit on this box) exit 0; SERIAL `ctest` exit 0; `scripts/agent-preflight.sh` exit 0 before the edits and `--staged` before this commit. The 13 checkpoint-gated parity suites emit their loud checkpoint-absent SKIP with 0 assertions and are NOT presented as coverage; no GPU gate and no checkpoint gate exists on this box. Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
FOLLOWING_AGENTS_PROTOCOL Bring the guard branch current before opening its PR. No conflict; the roadmap issue table auto-merged to exactly 1 added line with no other key disturbed. Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
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
2 participants
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 the guard half of the now-closed #551/#546. The header defect itself is already fixed on
mainbyfafa16f0f; this branch was re-scoped onto that fix and its own header hunk dropped, sotests/parity/hf_snapshot.his untouched here (git diff origin/main -- tests/parity/hf_snapshot.his empty).Issue: #558
What broke, and why the build did not catch it
af8170154addedNemotron35LightningSnapshot()callingHfSnapshoteleven lines before its declaration. 14 TUs that include the header failed to compile. Because those suites are checkpoint-gated,ctestreported them as***Not Run— which reads as missing checkpoints, not a build break.A full C++ build does catch this. The commit that broke it was records-and-evidence work that never built C++ at all, and
agent-preflight.shdoes not build. That lane is what the guard covers.The guard is not redundant — measured
With
af8170154's broken header in place and this method deleted, the entire preflight Python lane is green: all 41 checkers and 40 mutation suitesok. The break is seen by this method alone.It is a real compiler invocation, not a pattern match, so it generalizes. Review mutations against main's fixed header:
'HfSnapshot' was not declared in this scoperedefinition of ...Nemotron35LightningSnapshot()#endifunterminated #ifndef#include <cstdlib>A merge hazard git cannot see
Two independent repairs of the same defect, relocating the same block to different places, auto-merge with no conflict into a header defining
Nemotron35LightningSnapshot()twice, re-breaking the same 14 TUs:That is precisely how this branch would have re-broken
mainhad it landed unrescoped. The guard catches it.Two review findings closed
skipTestexits 0 andagent-preflight.shprints green while swallowing stdout, so a compiler-less box silently no-opped the guard. It now FAILS whenCIis set, and still skips otherwise. Both branches proven.$CXXwas truncated to one token, soccache g++/env g++gave a spurious RED. Fixed. The fallback is deliberately bounded: only exit 126/127 falls through (the launcher could not exec the compiler); a compiler that ran and rejected the header — exit 1 or 4 — is reported RED and never masked. Verified in both directions.Scope and gate
Two files,
+117/−0. Nosrc/, noinclude/, no C++ tests, no goldens, no checker weakened, no assertion deleted, inventory counts unchanged (scripts/check-snapshot-pins.pyitself is byte-identical).Merged tree at
ea43379ce:test_check_snapshot_pins20/20 OK,check-snapshot-pinsOK +SELF-TEST OK,check-agent-recordOK,agent-preflight.sh --stagedgreen. Main's current header compiles standalone, so the guard passes against it.Known limitations, recorded not hidden
Hardcodes
-std=c++20(matchesCMakeLists.txt:35today), carries no-Wall -Wextra -Werror, and does not prove include self-containment. It guards this one header — a new header undertests/parity/inherits nothing; the honest generalization is a per-header sweep, noted in #558 alongside #408.🤖 Generated with Claude Code