Fix sharded GGUF evidence edge cases - #675
Merged
Merged
Conversation
Performance Comparison
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens GGUF sharded-artifact validation and runtime-evidence hashing by making diagnostics deterministic and ensuring local sharded GGUF symlinks are resolved to regular-file targets for identity hashing (while keeping logical shard filenames in the digest).
Changes:
- Make missing
split.count,split.no, andsplit.tensors.counterror messages deterministic by sorting and consistently formatting shard filenames. - Resolve local shard symlinks to their regular-file targets for artifact identity hashing via
_regular_file_identity_paths, and wire this intobuild_from_gguf. - Add regressions for deterministic diagnostics and for local-symlink shard identity binding/hashing behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/mobius/integrations/gguf/_shard_set.py | Deterministic formatting for “missing split metadata” shard filename diagnostics. |
| src/mobius/integrations/gguf/_builder.py | Replace cache-specific identity path resolution with regular-file target resolution for local shard symlinks during identity hashing. |
| src/mobius/integrations/gguf/_shard_set_test.py | Add tests covering deterministic missing-metadata errors and local-symlink identity-path binding for sharded GGUF builds. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
justinchuby
force-pushed
the
justinchuby-audit-copilot-reviews
branch
from
August 27, 2026 16:01
d3e9b1d to
536f4e8
Compare
Resolve local shard symlinks to verified regular-file identity targets so artifact evidence remains hashable outside the Hugging Face cache. Make missing split metadata diagnostics deterministic by sorting and formatting shard filenames. Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pin every GGUF reader to a retained regular-file descriptor and use serialized duplicates of that descriptor for artifact and checksum hashing. Detect path replacement and restored in-place mutation with handle-level change identity, including Windows ChangeTime, while preserving symlink and logical shard-name behavior.\n\nAdd deterministic ABA, restored-mutation, checksum, FIFO, symlink-retarget, and concurrent-hashing regressions.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Bind reuse planning and verification to one retained GGUF reader, route Tencent Q1_0 range reads through the owning pinned descriptor, and revalidate reuse sources immediately before verifier return and package publication.\n\nMake TOCTOU regressions portable to Windows with same-size writes and narrowly gated symlink tests.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 8be62ed7-f80c-44f1-a286-72d8328e6fa7 Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Use same-size in-place mutation instead of truncating an open GGUF, and simulate the post-comparison symlink state without renaming a memory-mapped file. Keep explicit assertions that the intended identity and symlink checks execute.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 8be62ed7-f80c-44f1-a286-72d8328e6fa7 Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
justinchuby
force-pushed
the
justinchuby-audit-copilot-reviews
branch
from
August 27, 2026 18:05
e912cca to
fb7f278
Compare
justinchuby
added a commit
that referenced
this pull request
Aug 27, 2026
Preserve both the sharded-import errno dependency from #675 and the cohort fingerprint dataclass dependency after rebasing PR #677. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 71862fcb-bc9a-4734-b047-b5376afabc3e Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 27, 2026
Preserve both the sharded-import errno dependency from #675 and the cohort fingerprint dataclass dependency after rebasing PR #677. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 71862fcb-bc9a-4734-b047-b5376afabc3e Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 27, 2026
Derive Granite routed expert width from feed_forward_length exactly, reject contradictory serialized expert widths without lossy numeric conversion, and reconcile the authoritative route census with closed #652 plus merged #656 and #675. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 71862fcb-bc9a-4734-b047-b5376afabc3e Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 28, 2026
## Summary - Promote only the immutable Apertus v1.1 1.5B Instruct BF16 GGUF exact-float CPU route to final-package runtime-supported status. - Accept pinned llama.cpp v1.1 serialization: unqualified `xielu.*` metadata and factorless default RoPE, while rejecting conflicting xIELU keys, incomplete/malformed factors, unsupported scaling metadata, ignored `original_context_length`, and lossy factor storage. - Preserve graph identity independently from the subsequently written `export_report.json`, then validate the complete final runtime package including that report. - Auto-select exact immutable runtime evidence without requiring callers to repeat its evidence-pinned tokenizer repository/revision. Matching explicit flags remain accepted; conflicting explicit identities fail before serialization. - Reconcile #697 tokenizer policy without adding a broad Tekken promotion: unmatched artifacts remain deferred and tokenizer-omitted. - Replace a build-time deferred tokenizer report with supported/exported after exact materialization, so the validated package is truthfully complete and end-to-end runnable. - Use an independent raw `gguf.GGUFReader` + `dequantize` oracle with Mobius loading, mapping, tensor-processing, and normalization helpers patched to raise. ## Immutable evidence - GGUF: `MrMeOrYou/Apertus-v1.1-1.5B-Instruct-GGUF@88c75ad49566d3c2157d03709bf772262c3241ed` - File: `Apertus-v1.1-1.5B-Instruct-BF16.gguf` - Size/SHA-256: `3,028,052,608` / `f9ec154d0ec29dad1f6465b458b7f27bd25ad7b9a3899233ae98ca6d358501c2` - Official config/tokenizer: `swiss-ai/Apertus-v1.1-1.5B-Instruct@9e9d01154446a645d30f04174cf1515a38058be7` - Tensor census: 163 tensors (`BF16`: 98, `F32`: 65) - Graph fingerprint: `4bb91bade19d41559cb524e28692453851fe67274cc58109787ee968df3e0fe5` - Final runtime-package fingerprint: `97582549e9c5b4114f3bcfa81c92f16aba9d14dd0ea0d2b20acaefd9060e6486` The real-artifact test invokes runtime packaging without tokenizer source flags and proves automatic evidence-pinned materialization, full-logit prefill, fresh and cached decode parity, full-sequence replay, rollback, batch reorder, 20 exact greedy decode steps, deterministic ORT GenAI generation, tokenizer vocabulary/special-token/chat-template identity, graph/config/package fingerprints, package/report roundtrip, complete validated export reporting, and fail-closed artifact/source/tokenizer/runtime mismatches. Observed prefill max absolute difference was `0.00016784668`; cached-vs-fresh differences remained below `4e-5`. ## Truthful support boundary `source_fidelity=true` because BF16 values are exactly representable after import to float32. `storage_quantized=false` because the emitted ONNX package stores float tensors. Runtime support is evidence-gated to this exact artifact, official tokenizer revision, CPU import route, ONNX Runtime 1.29.0, and ORT GenAI 0.15.2. Other Apertus artifacts retain the generic best-effort advisory behavior and cannot inherit this evidence. This does not claim a generic independent Tekken tokenizer promotion, preserved-quantized runtime, CUDA, or other runtime versions. No Apertus model-ID/revision production gate was added, and the #675 source/import-route identity remains intact. ## Validation - Flag-free immutable Apertus runtime probe — 1 passed, 13 deselected; an initial successful run hit an intermittent ORT GenAI process-exit mutex error, and the clean replay passed with exit 0 - Focused Apertus/runtime-evidence/runtime-package/tokenizer/docs/census/schema tests — 436 passed - `python -m pytest tests/build_graph_test.py tests/cli_test.py src/ -q -k 'not phi4mm and not apply_weights_unknown' --tb=short` — 9,675 passed, 64 skipped, 12 deselected, 1 subtest passed - Exact evidence unit coverage: no tokenizer flags succeeds; matching explicit flags succeeds; wrong repository/revision conflicts before save; unmatched artifact does not auto-materialize - `lintrunner f --output oneline --all-files && lintrunner -a` — clean - `python scripts/generate_gguf_support_docs.py --check` — current - Focused mypy — clean - Final independent medium review found one legacy-case assertion-scope issue; it was fixed, and the follow-up review found no issues. - Both existing GitHub review threads were answered and resolved before the final push. ## Waivers - Preserved-quantized runtime: not claimed; representative evidence is BF16 imported exactly into float storage. - CUDA, Foundry Local, and Olive probes: outside this CPU ORT GenAI evidence promotion and not claimed. - Separate L4/L5 golden files: waived in favor of the immutable real-artifact harness, which asserts full logits and all 20 generated token IDs rather than prefix-only output. Semantically rebased and validated at exact base `2c981f2b631a907f43c57d3235647c22cc888eae`; exact head `256c2b15f17ec327634ef5cae1d1e902a07f5309`. Do not merge as part of this task. --------- Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com> Copilot-Session: 75ccb9e3-3ba3-4bee-b5ff-c395a88af1ca
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.
Summary
split.count,split.no, andsplit.tensors.countdiagnostics deterministicbuild_from_ggufsymlink path, artifact hashing, missing targets, and reversed shard orderingThis follows up on the still-applicable Copilot findings from #652 after its sharded import functionality was superseded on
mainby #656.Validation
python -m pytest src/mobius/integrations/gguf/_shard_set_test.py -q --tb=short(51 passed)python -m pytest tests/build_graph_test.py tests/cli_test.py src/ -q -k "not phi4mm and not apply_weights_unknown" --tb=short -n auto(8627 passed, 64 skipped)lintrunner f --output oneline --all-files