Repository navigation
docs(fixtures): document the required migrate_model_io --reemit regeneration step - #1902
Merged
Merged
Conversation
…eration step #1883 fixed tiny-deepseek-v4-qmoe/inference_metadata.yaml and tiny-glm52-full-attention/inference_metadata.yaml by re-emitting them with `migrate_model_io --reemit`, dropping ten hand-authored, never-real auxiliary policy components whose `policies/*.onnx` artifacts were never committed. That fix is correct and already reviewed/merged. What was missing: nothing on disk is broken today (verified: all three tiny fixtures -- tiny-deepseek-v4-qmoe, tiny-glm52-full-attention, tiny-glm52-qmoe-indexshare -- load and decode correctly against their anchor IDs on both native CPU and native CUDA, see verification below). The gap is regeneration determinism: re-running `generate.py` [+ `--full-attention`] alone, against each fixture's pinned Mobius commit, reproduces `model.onnx.textproto`/`tokenizer.json` byte-for-byte but NOT `inference_metadata.yaml` -- it reproduces the pre-canonicalization document `write_onnx_genai_config` emits, not the canonical single-`decoder`-component shape actually committed. This holds for all three fixtures, not just the two #1883 touched. Anyone who regenerates a fixture from scratch without also running `migrate_model_io --reemit` would silently reintroduce the exact bug #1883 fixed. Fix: document the required two-step recipe (generate.py, then `migrate_model_io --reemit <output-dir>`) directly in each generator's docstring. Docstring-only change; no functional/behavior change, no fixture artifacts touched. Verified this session (2026-08-23, Mobius worktree /home/justinchu/dev/mobius/.worktrees/glm52-e2e-fixture, pinned commits e71f4751 and d33c33b3): - generate.py alone: model.onnx.textproto/tokenizer.json byte-identical to committed, inference_metadata.yaml differs from committed, for all 3 fixtures (tiny-deepseek-v4-qmoe, tiny-glm52-full-attention via --full-attention, tiny-glm52-qmoe-indexshare default). - generate.py + `migrate_model_io --reemit`: inference_metadata.yaml byte-identical to committed, for all 3 fixtures. - cargo test -p onnx-genai-metadata --test decoder_recognizer_agreement: 14/14 pass -- the existing classification matrix (added by #1888) already guards the committed shape; no new test needed for this documentation-only change. - cargo test -p onnx-genai-engine --features native-cuda --test deepseek_v4_tiny_qmoe_e2e --test glm_tiny_full_attention_e2e --test glm_tiny_qmoe_native_cuda_e2e: 12/12 pass on an idle A100 (CUDA_VISIBLE_DEVICES pinned, hostlock-gated), CPU/CUDA tokens byte-identical to ANCHOR_IDS for all 3 fixtures. No fixture regeneration was performed or is needed; the ten policy components were correctly determined by #1883 to have never been real, and recreating them would reverse that already-reviewed decision. Independent review flagged and this commit fixes: a `\` vs `\\` line-continuation bug in the deepseek docstring that silently collapsed the documented two-line shell command onto one line inside the non-raw docstring (now matches the glm52 file's correct `\\` escaping); and confirmed (by direct regenerate+reemit-vs-committed diff, not by diffing against a nonexistent "pre-#1883" blob for glm52-qmoe-indexshare, which #1883 never touched) that the default GLM-5.2 IndexShare variant also needs the reemit step, not just the two fixtures #1883 patched. Signed-off-by: Sebastian <sebastian@squad.local> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1902 +/- ##
==========================================
- Coverage 80.80% 80.71% -0.09%
==========================================
Files 415 415
Lines 204440 204440
Branches 204440 204440
==========================================
- Hits 165201 165018 -183
- Misses 33664 33847 +183
Partials 5575 5575
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
Context
This is a follow-up investigation, not a fix for a currently-broken state. On fresh
main(post #1884/#1893), bothtests/fixtures/tiny-deepseek-v4-qmoeandtests/fixtures/tiny-glm52-full-attentionalready load and decode correctly — the missing-policies/*.onnxproblem I'd flagged in a prior cycle was already fixed by #1883 (re-emittinginference_metadata.yamlwithmigrate_model_io --reemit, dropping ten hand-authored, never-real auxiliary policy components) and #1888 (test-matrix follow-up). That fix is correct, already reviewed, and this PR does not revisit it.What this PR actually fixes
Regeneration determinism. Re-running
generate.py(and its--full-attentionvariant) against each fixture's pinned Mobius commit reproducesmodel.onnx.textproto/tokenizer.jsonbyte-for-byte, but notinference_metadata.yaml— it reproduces the pre-canonicalization documentwrite_onnx_genai_configemits, not the canonical single-decoder-component shape actually committed. This is true for all three tiny fixtures (not just the two #1883 touched), meaning a naive future regeneration would silently reintroduce the exact bug #1883 fixed.Fix: document the required two-step recipe (
generate.py, thenmigrate_model_io --reemit <output-dir>) directly in each generator's docstring. Docstring-only change — no functional/behavior change, no fixture artifacts touched.Verification (2026-08-23, idle A100,
CUDA_VISIBLE_DEVICESpinned,hostlock-gated)generate.pyalone vs committed: textproto/tokenizer byte-identical, metadata differs — confirmed for all 3 fixtures.generate.py+migrate_model_io --reemitvs committed: metadata byte-identical — confirmed for all 3 fixtures.cargo test -p onnx-genai-metadata --test decoder_recognizer_agreement: 14/14 pass (existing matrix from test(metadata): follow the re-emitted DeepSeek-V4/GLM-5.2 fixtures in the matrix (main is red) #1888 already guards the committed shape; no new test needed for a docstring-only change).cargo test -p onnx-genai-engine --features native-cuda --test deepseek_v4_tiny_qmoe_e2e --test glm_tiny_full_attention_e2e --test glm_tiny_qmoe_native_cuda_e2e: 12/12 pass, CPU/CUDA tokens byte-identical toANCHOR_IDSfor all 3 fixtures.captures=1 replays=10 fallbacks=0(native e2e test) /captures=5 replays=310 fallbacks=0(profile_native, n=3 steady). GLM full-attention CUDA:captures=1 replays=10 fallbacks=0in a small run /captures=3 replays=186 fallbacks=0(n=3 steady) — capture state recorded, not forced. GLM IndexShare CUDA:captures=0 replays=0 fallbacks=0, growing-KV-prefix decline (unchanged from prior cycle).Independent review (code-review agent) caught and this PR fixes: a
\vs\\line-continuation bug that silently collapsed a documented shell command onto one line in the deepseek docstring, and confirmed the default GLM-5.2 IndexShare variant also needs the reemit step (verified directly via regenerate+reemit-vs-committed diff, since #1883 never touched that fixture so there's no "pre-1883 blob" to diff against for it).Scope
No fixture regeneration performed or needed. No Mobius changes. No coarse-residency files touched. Independent of #1854.
Closes the "regenerate missing policies" instruction as superseded — see full task report for the baseline-measurement portion of this cycle.