fix(parity): main is RED — Nemotron35LightningSnapshot() calls HfSnapshot before its declaration (#546, #551) - #556
Merged
Conversation
…napshot 11 lines before its declaration (#546, #551) FOLLOWING_AGENTS_PROTOCOL My own regression, from af81701. I added the accessor directly beneath the revision constant it uses, and the revision constants sit ABOVE the HfSnapshot definition they are passed to. Name lookup in a non-template function is immediate, so every TU including tests/parity/hf_snapshot.h failed to compile: hf_snapshot.h:52:10: error: 'HfSnapshot' was not declared in this scope That is 14 parity TUs including the SACRED gates, plus the CPU-only test_hf_snapshot_pinning. Two other sessions hit it and filed #546 and #551 before I noticed, which is the part that stings: main was red for everyone while I was reporting progress. The fix is a pure relocation of the 13-line accessor from above HfSnapshot to below it, beside the other accessors. No behavior, no revision, no golden changes -- git diff is 13 insertions and 13 deletions of the same block. How it got past my gate, recorded so the next person does not repeat it: scripts/agent-preflight.sh does not compile the parity TUs, and I chained the merge push directly to a green preflight. A header-ordering break is invisible to every checker in that set. VERIFIED THIS TIME BY ACTUALLY COMPILING: a 2-line syntax-only TU including the header reproduces the error before and compiles clean after, and a full clean Release -Werror build of test_hf_snapshot_pinning (390/390 targets) links and runs 4 cases / 19 assertions Status: SUCCESS. The rule that would have caught it is already in AGENTS.md -- chain the applicable gate to the push -- but "applicable" for a change to a header that 14 TUs include means a build, not a checker sweep. Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
localai-bot
pushed a commit
that referenced
this pull request
Aug 13, 2026
…hat cannot exist, a CAPTURE that prints 1, and two env vars gating one checkpoint (#517) WIP land-prep for row/MODEL-NEMOTRON-H-W1. Committed early and deliberately: the previous attempt at this task completed every item and lost all of it to an out-of-session worktree sweep. LOW-1 -- the fnmatch divergence spans 27 patterns, not 20. modelopt_mixed_precision.h:253 and the same sentence in the spec both said the 6,291,453-pair sweep found 108 mismatches "spanning 20 distinct patterns". 20 is impossible on the sentence's own numbers: every divergent pattern translates to `(?s:.)\Z`, which matches a length-1 name and nothing else, so a pattern can contribute at most the 4 one-character names in the sweep alphabet and 20 caps out at 80. Re-measured independently on this branch against CPython 3.12 `fnmatch.fnmatchcase`, compiling the REAL header: SWEEP 1 names<=3 over {a,b,.,0} x patterns<=5 over {a,.,?,*,[,],!,-} cxx: names=85 patterns=37449 pairs=3183165 s1: 3,183,165 pairs, 0 mismatches, 0 distinct patterns SWEEP 2 names<=2 over {a,b,.,0} x patterns<=6 over {a,.,?,*,[,],!,-} cxx: names=21 patterns=299593 pairs=6291453 s2: 6,291,453 pairs, 108 mismatches, 27 distinct patterns direction (cpython, ours): {(True, False): 108} distinct translations: {'(?s:.)\\Z': 108} mismatching name lengths: {1: 108} 27 x 4 = 108, and all 27 have the shape `[X-Y!]` with X > Y. Both pair counts reproduce the recorded sweep exactly, so only the pattern tally was wrong. `FnMatch`'s BEHAVIOR is untouched; the divergence stays recorded and unfixed for the reason already given. LOW-2 -- `CAPTURE` on a `const char*` variable prints `1`. doctest 2.5.2 has no stringifier for a `const char*` lvalue. Two loops in test_modelopt_mixed_precision_checkpoint.cpp compared FIVE prefixes each through one, so a failure named none of them. RED, with the CHECKs inverted in a scratch copy: :172: ERROR: CHECK( c.Resolve(p).how != Resolution::kExcluded ) is NOT correct! logged: p := 1 :248: ERROR: CHECK( a.algo != b.algo ) is NOT correct! logged: p := 1 GREEN, the identical inversions after switching both loops to `const std::string` (matching :149 and :161, which already did): 1 logged: p := backbone.embeddings 1 logged: p := backbone.layers.0.mixer.conv1d 1 logged: p := backbone.layers.0.mixer.in_proj 1 logged: p := backbone.layers.1.mixer.experts 1 logged: p := backbone.layers.1.mixer.gate 1 logged: p := backbone.layers.42.mixer.o_proj 2 logged: p := backbone.layers.5.mixer.q_proj 1 logged: p := lm_head 1 logged: p := mtp.layers.0.eh_proj LOW-3(a) -- a documentation claim that was half true. The test said `.env.example` AND `.agents/environment.md` document the loader as `set -a; . ./.env; set +a`. `.env.example:8` does, verbatim. `.agents/environment.md` does not contain the string at all (`grep -n 'set -a'` exits 1); it points at `.env.example` at :16. The comment now cites `.env.example:8` and says so. LOW-3(b) -- two env vars reached one checkpoint and NEITHER carried the pin. `CHECKPOINT_ROOT` (this test, joining the staging directory by hand) and `VT_NEMOTRON35_SNAPSHOT` (`parity::Nemotron35LightningSnapshot()`) resolved the same NAS `local_dir`. The cache spelling in the accessor is unreachable for a `local_dir` tree -- there is no `snapshots/<rev>/` whose NAME carries the revision -- and an env override is deliberately never revision-checked, so `kNemotron35LightningNvfP4Revision` named the goldens' revision and could refuse nothing. That is the failure `kQwen27NvfP4Revision` exists because of. `hf download --local-dir` does record the revision, just not in the path: it writes a per-revision file manifest at `<dir>/.cache/huggingface/trees/<revision>.json`, verified present on the NAS tree. `Nemotron35LightningSnapshot()` is now the single resolver for both spellings and gates the `CHECKPOINT_ROOT` path on that manifest. Proven by construction, the manifest being the ONLY thing that changes between A and B: A) staged dir carries ONLY deadbeef....json EXIT=77 (loud skip) B) add 29f2d174....json, nothing else EXIT=0, 3/12181 C) VT_NEMOTRON35_SNAPSHOT at the real NAS dir EXIT=0, 3/12181 D) neither env var set EXIT=77, banner names the export AND the required manifest E) VT_ set to a nonexistent dir, CHECKPOINT_ROOT ok EXIT=77 (refuses, never falls back) `VT_NEMOTRON35_SNAPSHOT` keeps `HfSnapshot`'s documented escape semantics unchanged and is checked first: naming ONE directory outright is a deliberate different-checkpoint run, naming a ROOT is not, which is why only the root path is revision-gated. `tests/scripts/test_check_snapshot_pins.py` stays green -- `kNemotron35LightningNvfP4Revision` is still passed to `HfSnapshot` exactly once (19 passed, 80 subtests). `tests/parity/hf_snapshot.h` was RED on main as recently as #556 for an accessor declared before its dependency, and preflight does not build the parity TUs, so the checkpoint TU that now includes it was compiled with `-Wall -Wextra -Werror` as part of this change. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
localai-bot
pushed a commit
that referenced
this pull request
Aug 13, 2026
Picks up the hf_snapshot.h build repair (#556) so this record-only PR builds. Its CPU, sanitizer and Windows-compile jobs were red for that reason alone -- the branch changes four markdown files and cannot affect a build. No overlap: #556 touches tests/parity/hf_snapshot.h, this branch touches the DSpark spec, the benchmark record, STATUS and BENCHMARKS. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
localai-bot
added a commit
that referenced
this pull request
Aug 13, 2026
…T, not by "this revision was downloaded here once" (#569) (#579) FOLLOWING_AGENTS_PROTOCOL Closes #569. The Nemotron revision pin was existence-only: it gated on the presence of .cache/huggingface/trees/<rev>.json, which records "this revision was downloaded here once", not "these bytes are that revision". A directory holding {"architectures": ["LlamaForCausalLM"], "hidden_size": 8}, every sidecar naming a different revision, an EMPTY touched manifest and a decoy manifest beside it, resolved as the pinned Nemotron checkpoint. Reproduced independently by the reviewer: RESOLVED at the old header, "" at the new one. The open question is answered and the answer is the bad one. huggingface_hub's _tree_cache.py never invalidates -- its own docstring says a tree listing "can be cached forever without any invalidation logic", write_tree_cache only os.replaces <commit>.json, and nothing in the library removes one. Manifests ACCUMULATE, so an old revision's manifest can vouch for new bytes. The worst case was the real case. The fix sweeps the per-file download sidecars, whose commit_hash tracks the bytes: verified that file_download.py writes it on all four outcomes that move bytes (:1378, :1398, :1425, :1464), and that the two early returns which write nothing are the only ones that skip it. Per-file rather than config-only because `hf download <repo> <shard> --revision B` into a tree fetched at A leaves a correct config beside wrong shards. Demonstrated against the REAL checkpoint on the gate host: a symlink mirror with one shard's sidecar flipped REFUSES, while the old header RESOLVED the same tree. The 69-sidecar sweep costs 0.217 s over SMB against a 21 GiB load. Siblings checked and deliberately unchanged: the four cache-layout accessors resolve models--org--name/snapshots/<rev>/, where the revision IS the directory name and a re-download creates a new directory rather than replacing bytes. Five mutations, no survivors -- including the two that initially survived and were closed by asserting the refusal MESSAGE and by reaching the vacuous branch through a config.json that is a directory. All 16 TUs including hf_snapshot.h recompiled from an empty build dir, which matters because this header RED'd main today (#546/#551/#556) and preflight does not compile the parity TUs. Carried forward as an operating condition, not a defect: VT_NEMOTRON35_SNAPSHOT remains an ungated escape hatch, uniform with the sibling accessors. W6 must run with it UNSET and must record the resolved directory in its evidence. 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 #546. Closes #551.
maindoes not compile, and it is my regression.af8170154addedNemotron35LightningSnapshot()directly beneath the revision constant it uses —but the revision constants sit above the
HfSnapshotdefinition they arepassed to. Name lookup in a non-template function is immediate, so every TU
including
tests/parity/hf_snapshot.hfails:That is 14 parity TUs including the SACRED gates, plus the CPU-only
test_hf_snapshot_pinning. Two other sessions hit it and filed #546 and #551before I noticed.
The fix
A pure relocation of the 13-line accessor from above
HfSnapshotto below it,beside the other accessors. No behavior, no revision, no golden changes — the
diff is 13 insertions and 13 deletions of the same block.
How it got past my gate
scripts/agent-preflight.shdoes not compile the parity TUs, and I chained themerge push straight to a green preflight. A header-ordering break is invisible
to every checker in that set.
Verified this time by actually compiling:
-Werrorbuild oftest_hf_snapshot_pinning./tests/test_hf_snapshot_pinningStatus: SUCCESS!scripts/agent-preflight.sh --stagedThe rule that would have caught this is already in
AGENTS.md— chain theapplicable gate to the push. What I got wrong is "applicable": for a header 14
TUs include, that means a build, not a checker sweep.
🤖 Generated with Claude Code