chore(gemma4): stale comments, dead scaffolding, and self-checking gates retire - #1025
Merged
FeathBow merged 15 commits intoSep 1, 2026
Merged
Conversation
The crate header, the gather-rows provenance (12B-measured, 26B calibration pending), the admit_and_prefill lane arm, the emit-rows newcomer note and the mixed-step multi-prompt contract now describe the code as it is. Signed-off-by: Feathbow <feathbow@gmail.com>
MoeScratch.expert_cursor allocated one device element the wrapper never passed on, and model_line ran a weak identity check the authoritative probe repeats completely. Both go; the C ABI keeps its null parameter. Signed-off-by: Feathbow <feathbow@gmail.com>
…fp8 profile stops double-running Signed-off-by: Feathbow <feathbow@gmail.com>
Retiring the residency gate left LoadStats with no reader outside the loader: every remaining caller destructured the tuple and dropped it. The stats stay - they feed the resident info line - but they are built, logged and forgotten inside from_safetensors, which now returns the weights alone. The timing-window docs also stop overclaiming: both walls cover the shared loader's BF16 staged tensors only, and the A4B expert upload, repack and scale preparation run after them, inside the total alone. Signed-off-by: Feathbow <feathbow@gmail.com>
Review round two on the comment commit. MIX_GATHER_ROWS is documented as what it is - the unchunked follower-gather budget, with an unbounded leader and a chunked bypass - and its hardware measurements move out of production source into the benchmark records. The coalesce-door note follows suit. The crate header names the overlap lane exception to the chunk knob. The mixed-step contract speaks of prefill entries and segments rather than whole prompts. The two-pool atomicity note moves from the arithmetic helper to admit_tokens, where the rollback lives. Signed-off-by: Feathbow <feathbow@gmail.com>
The gate exercised SuppressIds, the shared suppression kernel and a generic logits buffer - dependency behavior end to end, with no Gemma policy, admission or sampling wiring in the loop. It now lives beside the kernel in pegainfer-kernels as the same ignored GPU test, and its runner row retires with it; the Gemma crate keeps the policy bound test that covers its own side of the contract. Signed-off-by: Feathbow <feathbow@gmail.com>
tokenizer_parity exercised the vendored vllm chat renderer against the HF reference and no Gemma production code, so the frontend crate now owns it (the fixture path and the checkpoint guard travel unchanged; run it with -p pegainfer-frontend --test gemma4_tokenizer_parity). The Gemma runner retires its integration target and the chat-golden preflight with it; the moved test names its own invocation. Signed-off-by: Feathbow <feathbow@gmail.com>
…group Signed-off-by: Feathbow <feathbow@gmail.com>
Moving the parity test gave pegainfer-frontend a sha2 dev-dependency the lock file did not record, which fails every --locked CI step. One line: sha2 joins the frontend entry; the vllm-chat/vllm-text moves leave the lock untouched. Signed-off-by: Feathbow <feathbow@gmail.com>
… all Review round two holds the line the earlier retention commit drew: the public MoeAlignScratch promised that struct literals written against the old shape keep compiling, and deleting expert_cursor broke that promise without a declared source-stability contract for the crate. The ignored field returns exactly as it was, the Gemma scratch allocates its one dead element again, and settling whether the kernels crate owes downstream source compatibility moves to its own decision rather than riding a cleanup. Signed-off-by: Feathbow <feathbow@gmail.com>
The load stats say what they sample: elapsed_ms is a submission total taken after the A4B expert kernels are enqueued but not drained, the validate window starts after the caller has already parsed the config, and no figure waits on the expert preparation. The mixed-step contract says the mid-walk row is sampled and discarded, with only the final segment supplying the request's first pick. Signed-off-by: Feathbow <feathbow@gmail.com>
The tokenizer page names the moved gate, its owner and the working invocation, and stops describing the retired token-id probes as live coverage. The serving page states the gather bound as what it is - a follower budget with an unbounded leader and a chunked bypass - and sends the hardware figures to the benchmark notes. Signed-off-by: Feathbow <feathbow@gmail.com>
Signed-off-by: Feathbow <feathbow@gmail.com>
…ay the same thing as the code Round three of the comment cleanup. The serving page describes the runner as it is - three owned scopes held by exact name - instead of the two-crate router-only picture. The resident log line calls its figure a submission total and says the expert kernels are enqueued, not drained, so the log and the source comment stop disagreeing. The parity header drops the claim that the Gemma engine supplies the fixture and states the invocation once. Signed-off-by: Feathbow <feathbow@gmail.com>
…ixture The token-id comparisons retired earlier, but the dumper kept producing their probes and special-token table and the golden kept carrying them for no reader. Both go: the dumper writes provenance, file hashes and chat cases only, the fixture drops the two dead payloads (rewritten with the dumper's own serialization so a fresh dump is byte-identical), and the tokenizer page attributes the add_special_tokens observation to the retired probes instead of presenting it as live coverage. The page's claim that everything in the fixture is asserted is true again. Signed-off-by: Feathbow <feathbow@gmail.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
1 participant
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.
Description
Closes #1024
Post-landing audit cleanup in three commits, each deleting or correcting something the serving campaign left behind. No production behavior changes; the one production-code touch is removing a dead allocation.
Comments say what the code does. The crate header, the gather-rows provenance note (the measured floor is a 12B number; the 26B calibration is pending and now says so), the
admit_and_prefilllane-arm description, the emit-rows newcomer note, and the mixed-step contract inserve.rsnow match the code.Dead scaffolding retires.
MoeScratch.expert_cursorallocated a device element the wrapper never passed on (the C ABI keeps its null compatibility parameter), andmodel_lineran a weak identity pre-check that the authoritative config probe repeats completely. Both go.Self-checking gates retire.
faultless_checkpoint_is_acceptedvalidated a manifest generated from itself;the_global_door_is_defensiveproved a page-ceil monotonicity tautology; the constant-pool geometry probe is strictly superseded by the varied page-unique probe; the residency loader gate duplicated the load path every checkpoint gate already exercises. All four retire with their runner manifest rows. The fp8 profile drops the two storage-fixed gates the default suite already runs, and the agreement gate loads each storage once for both window cases instead of four times — halving its checkpoint loads without changing what it measures.Test Env
--release --features gemma4, Gemma-4-26B-A4B NVFP4 checkpoint (12B checkpoint for the gate runner env).Verification
ignore_eos) produce byte-identical outputs on main and on this branch — the dead-cursor removal does not touch serving.cargo fmt --checkandclippy -D warningsgreen.