Repository navigation
jemm: load and validate checksum-covered calibration - #122
Conversation
hsliuustc0106
left a comment
There was a problem hiding this comment.
Reviewed at head 82cbe79282 (stacked on #118; 5 files, +45/−20). Small, well-scoped contract hardening, verified in code:
- Calibration now loads from checksum-covered
decision_config.jsonwith finite/positive-temperature/threshold-range validation and cross-checked manifest agreement; the pinned constants are gone fromcontract.rsand the exporter validates and records the file's values (so old exports stay loadable — the old exporter already copied and hashed the file). - The error envelope is typed and bounded exactly as the body claims —
{"error": "ValueError"|"TypeError"|"RuntimeError", "detail": ≤200 chars}with 422/413→ValueError, 415→TypeError, else RuntimeError (verified atsrc/models/jemm/native/src/server.rs:48-56) — matching upstream serve.py's envelope shape with the documented 503-vs-500 deviation.
No actionable findings.
Approving the code review; merge in stack order (#117 → #118 → #122 → #123). The multi-image parity question is tracked on #118 and is not re-counted here. Unverified: GPU revalidation of the new calibration source is author-reported; no fresh checkpoint re-export was run (policy stated in the body; old-export compatibility verified by code reading). Provenance: canonical .agents/skills/system1-omni-review/SKILL.md (SHA-256 58be3bc6…dde108) and its repository map at trusted base 4a79980d8a75; remote head rechecked before posting.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-check at new head 394844f768 after the branch rebase (prior review: APPROVED at 82cbe79282, 2026-10-08). Series identity: git range-diff of the previously reviewed head vs the new head shows every commit patch-identical and none added or dropped — a rebase, not new work. Provenance: review skill at main@4a79980d (SHA-256 58be3bc6…dde108); remote head re-pinned immediately before posting.
Verdict unchanged: APPROVED. Same unverified checks as before: GPU revalidation of the checksum-covered calibration source is author-reported; no fresh checkpoint re-export was run; no CI has reported on the rebased head yet (identical series passed at the prior head). Stack order (#117 → #118 → #122 → #123) still applies.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-check at new head 5efe07c37a — the branch rebased after my approval at 394844f768. git range-diff against that reviewed head: all commits patch-identical, none added or dropped — no content change; the approval carries. Fork CI at this exact head: rust/docs/benchmarks all pass. Unverified (unchanged): GPU revalidation of the calibration source author-reported; no fresh checkpoint re-export run; no Rust toolchain on this host. Verdict: APPROVED (unchanged). Stack order #117 → #118 → #122 → #123.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Series-identity correction at head 5efe07c37a (unchanged since the approval; found in the 2026-10-11 audit). My approval (id 5481557128) said "all commits patch-identical, none added or dropped — no content change". The true range-diff against the previously reviewed 394844f768 shows 7 commits added — the #118 v2 unmerged-export series (9d4436e…91cc5c2), absorbed when the stack rebased onto the reworked #118; 91cc5c2 is #118's approved head and an ancestor of this head. Those exact patches were reviewed and approved under #118 (id 5481557073, same SHAs), so the approval's substance is unaffected — but the "no content change" statement was false; the sweep compared against unfetched refs (tooling since fixed). Verdict: APPROVED stands, on the corrected basis. Unverified (unchanged): GPU revalidation of the calibration source author-reported; no GPU/Rust toolchain on this host. Stack order #117 → #118 → #122 → #123.
Provenance: review skill at ca5f69b (SHA-256 58be3bc6…dde108); fork CI green at this exact head.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-check at new head after the rebase wave (prior review: APPROVED). Wave-verification note: this pass reviewed the whole 10-branch rebase wave systematically — every range-diff shows the own-commit series patch-identical; all post-pin merge resolutions were checked file-by-file against both parents (source/test/config/docs) with zero dropped lines; approved-content markers verified present in every tree (jemm v2 export, decider fixture READMEs, serving-matrix docs, vision FIFO regressions). Fork/upstream CI states as noted per PR.
All commits are patch-identical to the approved series (range-diff, named refs, cat-file verified); the stack merges' resolutions were checked file-by-file with zero dropped content, and the approved markers (the v2-series and calibration content) remain in the tree. Exact-head upstream maintenance workflows (ci + docs) pass at this head, posted by the author; earlier exact-head fork CI also green.
APPROVED (verdict carries — content-identical rebase). Transition note: the body predates the post-#138 template — add the four human contributor answers and the completed self-review checklist when next touched. Standing unverified checks unchanged (GPU evidence author-reported; no Rust toolchain on this host).
|
Re-checked this PR at head The current maintenance CI and Docs statuses pass. I found no new actionable issue. The previous APPROVED verdict remains valid for this documentation-only update. |
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-check at new head 5d630bc2fb after the rebase wave onto main@e66ff34 (prior review: APPROVED). Wave-verification note (this pass): the rebase wave was verified systematically — every own-commit series is patch-identical per range-diff; every post-pin merge resolution was diffed against both parents with zero dropped lines; approved-content markers verified present in every tree; the key new merge 3be46fa (shared-vision line × jemm v2 tip) carries #143\u2019s new test files intact and is ABI 8/8 consistent at every descendant head.
This branch inherits the verified 3be46fa2 merge plus trivial merges; the jemm v2 markers (export.py without safe_merge, jemm-native/2, untied head, recipe docs) are present in the tree, and the FP32-GDN/resident-feature content from the shared line is unioned, not dropped. Exact-head maintenance workflows (ci + docs) pass, annotated "no GPU qualification" as usual.
APPROVED (verdict carries — content-identical rebase). Transition note unchanged: post-#138 human answers and checklist to be added when next touched. Standing unverified: GPU evidence author-reported; no Rust toolchain on this host. Stack order #117 → #118 → #122 → #123 still applies.
Post-merge calibration validation — cases and resultsTested integrated main: Full campaign, commands, environment, evidence identity and exclusions: #155 regression report. Calibration/export cases that executedThe Rust calibration cases, including All eight exporter tests passed in the final pytest run:
These cover copied/checksum-covered calibration, file/manifest agreement, finite/range rejection, original BF16 base plus separate FP32 LoRA, byte-preserving fallback/resume and invalid adapter/export rejection. Actual integrated GPU/HTTP follow-through
This newly covers the calibration-containing integrated main on device for the pinned export/configuration. Alternate calibration values retain validation/response-math coverage; this is not a GPU accuracy claim for arbitrary calibration settings. No thresholds were relaxed and no performance result is claimed. Project-wide unexecuted cases are listed in the linked summary; the full raw archive remains local, not uploaded as a GitHub artifact. |
Integration completed — 2026-10-21
Delivered to main: #155 merged the collected stack at
32bdd2aade0e0f3f0452d04ca5022fcaaa4783c9. Main's complete tree is byte-identical to the locally verified candidate and the successful PR CI/Docs head.Merged into
codex/qwen-vision-27basd4e3c49103920522e2cc062be79babd883a66512, from unchanged reviewed head5d630bc2fb59830a4baf5e93be9248afa4355c58. #118 was already present in that target; this merge carries the checksum-covered calibration increment without rewriting its reviewed head.#117 is already in main and its former HOLD has ended. #118/#122/#123/#143 are now collected in the shared branch; main integration PR #155 has completed the delivery into main. Earlier dated maintenance sections retain their original revisions and results; their HOLD, pending-parent and old target statements are historical, superseded by this status.
The local combined rehearsal passed 257 Rust tests (50 ignored), strict Clippy/format, workspace release build, 8 JEMM exporter tests, 58 benchmark tests, 16 Decider helper tests, 8 CUDA helper tests, the CUDA contract check and strict MkDocs. These are integration CPU checks, not a fresh GPU qualification. The 2026-10-21 full JEMM ABI 8 campaign remains source-pinned to #118; it does not qualify this calibration increment on-device or add a performance claim. Existing contributor answers and declarations are preserved.
CPU maintenance and merge handoff — 2026-10-15
Current head:
5d630bc2fb59830a4baf5e93be9248afa4355c58.Actual target:
codex/jemm-nativeat5bdd625b4c27efb02f43a1f71150e1a7d7c0f071; the PR target was not changed.Normally merged the maintained #118 parent. The complete delta from this PR's previous head is the shared CUDA contract documentation only; checksum-covered calibration loading, rejection gates and all non-Markdown bytes are unchanged. Landing order remains #117 → #118 → #122 → #123, after the #117 hold is released. Current builds use ABI 8; this sync does not refresh historical GPU evidence.
Personally ran strict MkDocs and
git diff --checkon this head. Verified that every changed file since the previous published head is Markdown; all non-Markdown bytes are unchanged. Rust/device suites were not repeated for this documentation-only delta.This pass follows the contributor's 2026-10-15 instruction to maintain these existing PRs, complete CPU-only fixes and merge preparation, and keep #117 unmerged. Existing contributor declarations are preserved; this update adds no new human attestation or maintainer approval. No PR was merged and no GPU/model/performance run was performed. Source-pinned historical results remain historical. Current-head hosted workflows: CI run 38667515611, Docs run 38667519326. Read their live results separately from the personally executed checks above; these CPU workflows do not execute the skipped GPU tests.
Historical records below
Earlier maintenance/review/test sections retain their named revisions and environments. Their old ABI or build commands are historical reproduction instructions, not commands for deploying this current head. Use the current branch recipes for builds; the CPU checks above do not qualify skipped device tests.
Historical maintenance — 2026-10-15
Current head:
64712ee8ec11c75e8b8d735d87ff9db2357a0cef.Synced onto the maintained #118 parent at 3be46fa by a normal, conflict-free merge. The checksum-covered decision_config.json calibration loading and its rejection/consistency checks remain intact. This branch now carries the parent's ABI-8 resident-feature integration, repaired CPU CUDA fixtures and v2 kernel regressions. Rebuild the CUDA library and all consuming workers together; this sync does not extend the historical A800 numerical qualification to the new integrated head.
Personally run: format, strict workspace Clippy, workspace tests (241 passed, 0 failed, 47 ignored), release build, benchmark tests, CUDA checker/backend tests, source contract check, smoke validation and strict MkDocs all passed. Device-dependent tests were compiled and skipped on this macOS host. No new GPU, numerical-parity or performance pass is claimed.
Current-head hosted workflows: ci.yml, docs.yml. Linked hosted results are separate from personally executed local checks. Earlier evidence sections retain their named original revisions and scope; they are not new qualification claims.
No PR merge was performed. #117 remains on its explicit merge hold.
Required human contributor answers
These answers reuse the contributor's explicit 2026-10-12 maintenance instruction:
keep the existing PRs mergeable and CI passing; eliminate conflicts and review
issues; serve maintainers who need clear, verified changes that are easy to merge;
repair, validate and push to the existing PRs. The source is the contributor's
local maintenance conversation on that date; private conversation details are
not published.
What outcome are you trying to achieve?
Keep the existing PRs mergeable and CI passing.
What is the highest-leverage variable for that outcome?
Eliminate merge conflicts and resolve review issues.
How will this work move that variable rather than create busywork?
Repair the identified conflicts and review issues, validate the affected
integration, and push the verified changes to the existing PR branches.
Who is the customer or user, and what do they actually need?
Repository maintainers, who need clear, verified changes that are easy to merge.
After answering the four questions: is a PR needed at all to achieve this outcome?
The contributor explicitly directed updates to the existing PRs. Maintain these
PRs; no new PR is needed for the maintenance work.
Bug-fix evidence: N/A for this primary contract addition: it introduces file-driven, checksum-covered calibration and explicit file/manifest agreement. No newly observed runtime bug is claimed by this template-only update. The maintained rejection/export tests and their recorded results are linked and described below.
Purpose
Load finite, in-range calibration from checksum-covered
decision_config.jsonand require exact agreement with the export manifest. This follow-up preserves the v2 original BF16 language shards and separate FP32 LoRA introduced by #118.Integration and review scope
System1-Omni Version / Commit:
1eab32bac8cb19f0fca925d69c14dff03c0c4c2e.Incremental baseline: #118
ed9f9d6eb566d226e2441c5e9d84187abe1e4d63; integrated main:3e9cc33a61ddf110d97512fb6f296cf540377c61. Review order: #117 → #118 → #122 → #123.The merge keeps the v2 format/rank/adapter checks and this PR's file-driven calibration policy. Temperatures must be finite and positive; threshold must be finite and in
[0,1]. Both v2 rejection and file/manifest disagreement tests remain. Tiny real-safetensors export tests cover copied calibration/checksums and invalid values rejected before output creation.Review map: exporter calibration → required artifact checksum coverage → processor agreement/response calibration → root-tree Rust/Python regressions. Executor, kernels, LoRA and Graph behavior are identical to final #118; this PR changes calibration loading and validation.
Historical Test Plan
System1-Omni Version / Commit:
1eab32bac8cb19f0fca925d69c14dff03c0c4c2e.Reproduction commands from the repository root; recorded local feature switches and device prerequisites remain scoped to the original results above. This PR-body update runs no additional source or device checks.
cargo fmt --all --check cargo clippy --workspace --locked --all-targets --features omni-laya/serve -- -D warnings cargo test --workspace --locked --features omni-laya/serve cargo build --workspace --release --locked --features omni-laya/serve python3 -m mkdocs build --strictWith the tiny-export test dependencies installed:
python3 -m unittest discover -s tests/jemm -p 'test_export.py' -vCheck required artifact checksum coverage, finite positive temperatures, threshold in [0,1], exact file/manifest agreement, v2/rank rejection and real-safetensors export behavior. Runtime execution is inherited from #118; do not report a fresh full-checkpoint campaign.
Historical Test Result
Local maintenance checks passed formatting, strict workspace/all-target Clippy, locked workspace tests, release build and strict MkDocs. Workspace tests passed 232 unit/integration cases plus one compile doctest, with 37 device/checkpoint tests ignored. Python 3.12 tiny-export tests passed all eight cases.
Independent read-only merge review found no actionable regressions. Agent assistance does not replace contributor or maintainer review.
Demo / evidence
The historical #118 A800 v2 campaign passed 16 requests / 25 questions with maximum probability drift
0.0166172279642417 < 0.02. Its recorded source, binaries, configuration and pinned calibration define that evidence. The fresh #118 RTX 4090 campaign passed eleven fixed-seed reference-kernel comparisons; it did not load the 27B checkpoint.This follow-up retains #118 execution code and tests calibration loading on CPU. No fresh full-checkpoint export/parity or HTTP campaign of exact
1eab32bawas performed. Other calibration values have validation/response-math coverage, not a GPU accuracy claim. Ignored GPU tests and old hosted workflows are not current-head passes.No tolerance was relaxed and no new performance claim is made.
Exact-head hosted verification — 2026-10-13
1eab32bac8cb19f0fca925d69c14dff03c0c4c2e.1eab32bac8cb19f0fca925d69c14dff03c0c4c2e.These are upstream workflow-dispatch results for the exact source head. Ignored GPU/checkpoint tests did not execute; maintenance status contexts link the actual runs.
Video: N/A for this nonvisual API, validator, shared executor or documentation change. Source-pinned tests and the linked validation records provide the relevant evidence; existing hardware and unverified-measurement limits remain unchanged.
Self-review
The contributor explicitly confirmed completion of self-review and authorized this checklist/template update on 2026-10-13 in the maintenance conversation. This records the human confirmation; it does not turn unverified measurements into passes or replace maintainer review.