Repository navigation
jev_vl: process inline images with the shared native vision backend - #139
linear3735 wants to merge 27 commits into
Conversation
…v-vl-online-upstream
hsliuustc0106
left a comment
There was a problem hiding this comment.
Scope note (head moved mid-review): findings below were verified at pinned head 953b6914fb; the remote head moved to f88d918221 (merge of current main 71857e0) during review. I verified every reviewed PR file is blob-identical at both heads except .github/workflows/ci.yml (which takes main's version plus this PR's one authored line), so all findings and evidence apply to the live head f88d918221. Spot-verified independently: the P2's blob arithmetic (merge 20b480a — both parents d9f3355, result 9557810) and the P3-3 readiness gate (main.rs ensure!(decide(&engine, ONLINE_VISION_WARMUP)…) under JEV_VL_VISION).
Independent review — PR #139 "jev_vl: process inline images with the shared native vision backend"
Reviewer: independent code review (ZCode agent), 2026-10-12. Not posted to GitHub; local report only.
This review grants no approval. Missing GPU/tooling results are evidence gaps, not passes.
Verdict
4 actionable findings: 1 × P2, 3 × P3. No P0/P1. The authored integration itself (bounded decode → CPU
preprocess → serialized vision forward → language prefill → CPU head) is well-scoped, ownership-clean,
and defensively tested; the P2 is a provenance problem in the dependency stack, not a code defect in the
authored commits.
Provenance
| Item | Value |
|---|---|
| Base (live, fetched this review) | main @ 71857e033b2d2d0587742c1c1a5260d683ab17cd (71857e0, unchanged from last check) |
| Pinned review head | 953b6914fbb2a667a7939dd2ad832136750a11ad (matches GitHub headRefOid at review start) |
| Remote head after recheck | f88d9182219b243a523850783289bb9b26c0d794 — branch moved during review: one new commit, Merge upstream main and retain online vision export CI, merging main 71857e0. Verified the merge preserves every reviewed PR file blob-identical (processing.rs 317c75f9, json.rs 95578103, recipe README 96264686); only .github/workflows/ci.yml resolves to main's version plus the single authored unittest line, plus main-side files (src/backends/cuda/qwen3_5/README.md, qwen3_5.backend.json) taken from main. Findings below apply verbatim to both heads. |
| Merge base (pinned head vs main) | 6b51652b69c761994f9c944505d2527c92b5d254 (PR #108 merge). GitHub's 3-dot diff = 6b51652..953b691. |
| PR state | OPEN, DRAFT, author linear3735, GitHub mergeable = CONFLICTING (cleared in-tree by f88d918's main merge; GitHub marker needs refresh) |
| Skill load record | .agents/skills/system1-omni-review/SKILL.md read from local working tree (repo HEAD ca5f69b5c104); SHA-256 of bytes read 58be3bc60806c4c5af7038a0c6a80fadb94f3360d714556a3e965c8903dde108 (matches pinned value); blob 7f245253cd identical across worktree / main / PR head. Unpublished local skill (repository-committed, content hash recorded). |
| References actually read | references/repository-contracts.md (blob b6ace00bf8, identical in all three revisions); docs/architecture.md — initially worktree version 7710c734, then the PR-head/main version 9fb69e1165 (they differ; findings use the PR-head text); CONTRIBUTING.md at worktree/PR-head 1aa9639a2e (main differs, b092bbaacc); .agents/skills/self-review/SKILL.md artifact-hygiene section (worktree b6dcf04a5d); post-#138 .github/PULL_REQUEST_TEMPLATE.md at main 71857e0. |
Findings
P2-1 — Unattributed rewrite of shared Python-compatible float rendering (float_repr) enters via dependency merge commits, owned by no named commit
- Files:
src/models/qwen3_5/native/src/json.rs:132-167(head), teststests/qwen3_5/json.rs:37-71. - Trigger: In the #117 line, merge
20b480a("Merge main and preserve shared vision graph regressions") has
both parents at json.rs blobd9f3355(6480b6band6b51652) but the merge result is blob
9557810— a rewrittenfloat_reprplus two new tie-to-even test functions that exist in no parent and
no named commit (git log --all --find-object=9557810…returns nothing). PR #139 pulls this in through
its dependency mergeb7442fd(merge of #117 headc15a989), so the change appears in this PR's GitHub
diff (json.rs +18/−12, tests/qwen3_5/json.rs +36) although none of the five authored commits touches
json.rs. - Consequence:
float_repr/dumps()render structured state into the prompt for every shared-Qwen
consumer (Cua-S1, Open-Jev, JEV-VL). If this PR (or #117) merges, main gains a cross-model serialization
semantics change that no review ever examined and no commit owns — it is invisible to blame, bisect, and
the #117 review that baseline-approved6480b6b. The change itself looks correct and is an improvement:
the old code's own comment admitted it did not implement Python's tie-to-even ("Python breaks the rare
exact ties … to even; this does not"); the new code delegates shortest-digit selection to
serde_json::to_string(ryu) and fixes both the tie rule and the doc comment. The oracles are genuine:
I verified all 15 expected strings infloat_repr_matches_python_examplesand the five bit-exact tie
cases match local Python 3.12.8repr()exactly. - Action: give the change a named home (a commit in #117, or a separate small PR) and disclose it in this
PR's review scope; "the shared vision implementation belongs to #117" is inaccurate for json.rs, which is
not vision. Alternatively exclude it from this stack.
P3-2 — docs/architecture.md left stale: "Image features must be prepared offline"
- File:
docs/architecture.md:64-69at head (identical to main's): "The experimental JEV-VL worker …
Image features must be prepared offline." - Trigger: any reader of the architecture contract after this PR merges. The PR's entire purpose is inline
images encoded inside the worker (JEV_VL_VISION), andsrc/models/jev_vl/README.md+
recipe/jev_vl/README.mdare updated, but the architecture paragraph — added when the worker landed
offline-only — is not.mkdocs build --strictpasses because it cannot catch semantic staleness. - Consequence: the repository's own integration contract contradicts the shipped behavior. CONTRIBUTING's
documentation rule ("Update affected shared-component documentation") applies. One sentence fix.
P3-3 — Recipe readiness claim stale in online mode: /health gate now includes an image warmup that the recipe doesn't document
- Files:
recipe/jev_vl/README.md("Build and launch" section) vssrc/models/jev_vl/native/src/main.rs:145-150. - Trigger: the recipe says "
/healthbecomes available after model loading and a successful text warmup;
that health check does not validate image assets." WithJEV_VL_VISIONset,mainnow additionally runs
ONLINE_VISION_WARMUP— a full inline-image decision (1×1 PNG → decode → preprocess → vision forward) —
andensure!s on it before binding the port. The "Online images" section documents neither the
warmup nor its readiness effect (the PR body does mention "startup image warmup"). - Consequence: an operator debugging a slow/failed startup in online mode reads the recipe and concludes
image health is never exercised; readiness semantics are part of the operator contract this recipe owns. - Note: the warmup itself is correct and desirable (real warmup before readiness, per the backend
lifetime contract); only the documentation lags.
P3-4 — No per-request bound on in-flight online-image buffers (transient ~0.7 GB worst case)
- Files:
src/models/jev_vl/native/src/processing.rs:127-147(load_images),processing.rs:83-108
(load_image),executor.rs(resolve_images/pending_images). - Trigger: one request with ~250 distinct minimal inline images. Each 1×1 PNG is resized to 256×256
(min_pixels 65,536) → 256 patches → ~1.57 MB ofpixel_values(256×1536 f32) held in
ImageInput::Inlinefor every image of the request until the serialized forward finishes; post-vision
embeddings add ~1.3 MB per image (64 tokens × 5120 × f32) held inpending_imagesuntil request end.
4 MiB body + 16,384-token budget admit ~250 such images (≥64 tokens each) ≈ 0.4 GB pixels + 0.3 GB
embeddings, simultaneous, on the worker process that also hosts the model. The L2 budget deliberately
"excludes in-flight request inputs" (recipe cache-controls), so nothing caps this. - Consequence: memory-pressure/OOM risk under a hostile or careless multi-image request; bounded and
serial (one request at a time), so degradation not corruption. Partially pre-existing (prepared mode
also retains all of a request's assets at once); inline mode roughly doubles the footprint and makes
tiny-but-expensive images trivially scriptable. - Action (cheap): document the per-request image bound next to the other limits, or cap images per
request (e.g., a small constant) pending the GPU validation.
Notes (not findings)
- Ownership is clean. Shared bounded decoding (
image_decode.rs), shared preprocessing
(image_preprocess.rs), and vision execution stay in the shared layer (src/models/qwen3_5/native,
src/backends/cuda/qwen3_5); jev_vl keeps policy (image-source selection, pinned-processor validation,
cache policy, warmup, response assembly) insrc/models/jev_vl. ReusingImageLimits::cua()as the
shared default is safe here only becausevalidate_exportpins the processor metadata to exactly those
values at startup (min 65,536 / max 16,777,216 pixels, patch 16, merge 2, mean/std 0.5) — a mismatch
cannot start the worker. - Wire contract preserved. Same
POST /v1/systemonesingle-question shape, same kinds
choice/noul/score, same 4 MiBDefaultBodyLimit, same response fields; inline data URIs were already
the image addressing scheme (prepared mode required the exact URI), so online mode changes where
embeddings come from, not the envelope. Prepared mode remains the default;JEV_VL_VISIONand
JEV_VL_IMGCACHEare mutually exclusive with a startup error;ImageInputkeeps the two worlds typed
apart, and vision forward errors ("online vision is not loaded") cannot occur for prepared inputs. - Empty-suffix skip cannot silently drop an image. The skip (
suffix_pads == 0→suffix_asset = None) happens only in the L3-state Continue arm, where the image's contribution already lives in the
cached prefix device state and the continuation ids still carry<|vision_end|>+ tail. When no live
state exists, the rebuild arm reloads the image (match suffix_asset { None => self.load_image(...) }),
andtests/jev_vl/processing.rs::aligned_image_end_still_loads_images_without_a_live_prefixpins exactly
that case.structure_keyhashes every state part including the image URL, so a warm record can never
bypass validation of a different image, and a malformed image can never reach a warm record (its cold
pass would have failed). Suffix image placeholders are rejected. - Bounded decoding verified line-by-line. MIME prefix whitelist +
guess_formatcross-check; base64
and decoded 4 MiB caps; 2048-side / 1,048,576-pixel / 200-aspect checks matching the HF processor's
rejection rule; image-crateLimitsduplicated defensively; APNG rejected viais_apng(); the JPEG
header-segment MPF/MPO walk is correct (fill-byte skipping, standalone markers, SOS/EOI break, APP2
MPF\0sniff only inside real segment bounds); overflow-checked geometry (checked_multhroughout,
tokens ≤ 32768,gt == 1). - Pillow parity of the 16-bit conversions personally verified. Reproducing the test's exact cases with
Pillow 11.3.0: 16-bit grayscale (I;16) converts with clamp-to-255 ([128,256,65535] →
128/255/255 triples — matchingmin(255)), gray+tRNS loads asI;16(matching thepng_grayscale
color-type-0 branch), 16-bit gray+alpha / RGB / RGBA take the high byte (matching>> 8), alpha
dropped. Thepillow_rgbemulation matches local Pillow exactly for every mode the tests exercise. - Executor lifetime/ownership contract respected. Vision forward resolves under the same model mutex
inside the scheduler admission (one question at a time); in-request duplicate images share one forward
viapending_images; vision output validated (len == image_tokens * 5120, all finite) before use;
on error both language and vision drains are still attempted (language_drain?; vision_drain?;with
both futures materialized first); L2 and L3 publishes happen only after success; admission retained
until drains complete. - Bounds fix is real and shared.
DeviceBuffer::atdebug_assert!→assert!(cuda.rs:266) makes
the offset bound release-effective for all consumers (Cua included), with a registered weight-free test
(tests/qwen3_5/device_buffer.rs). Vision-export bounds (0a3f38e) add the 27B dimension checks to
export_vision.py(before any torch/safetensors import) and mirror them in Rustvalidate_config
(hidden 1152 / out 5120 / 64 layers / 24 heads / 4 KV heads, vision==language config, exact frozen
processor metadata), plus file-hash pins; the manifest is written last so a partial export is not
loadable. - ABI: no bump.
CS1_ABI_VERSIONstays 6 (ops.h:16 identical at merge-base and head). The three v2
vision symbols (cs1_vision_position_v2/rope_v2/attention_v2) are declared additive/optional and
resolved withlib.get(...).ok(); the error surfaces only when online vision is actually used
("27B vision needs additive v2 CUDA symbols; rebuild libqwen3_5_cuda.so"). The recipe's "rebuilt ABI 6"
requirement is therefore: same ABI version, library must be rebuilt from this revision to include the
new symbols. Rust declarations and ops.h agree. - Warmup. Text warmup unchanged; the new online warmup is a genuine end-to-end inline-image decision
before bind, pinned bytests/jev_vl/http.rs::online_vision_startup_warmup_is_inline_image_decision
(grid [1,16,16], 64 tokens) — but see P3-3 for the doc gap. - Artifact hygiene over the full 39-file inventory: clean. No generated run output, no binary
fixtures (all image fixtures are inline base64 in test bodies), no validation artifacts; Cargo.lock in
lockstep; new deps minimal (imagewithdefault-features = false, features = ["png","jpeg"],
base64, serdederive); new Rust test bodies all live under roottests/with explicit[[test]]
registrations or#[cfg(test)] #[path]wiring, per CONTRIBUTING. - PR-body numbers reproduce exactly (personally recomputed): GitHub 3-dot 39 files +3105/−1031;
authored-code changed lines (excl..md/Cargo.lock) 4050 vs #117-merged base and 1675
(27 files +1498/−239) vs fixed #117 headc15a989— both match the body's "Review scope" section.
Stacked accounting
- Total (GitHub,
6b51652..953b691): 39 files, +3105/−1031; code-only 3043+/1007− = 4050 lines. - Authored (this PR's five commits:
2a41f7d,c47a909,635c389,0a3f38e,953b691): raw per-commit
sum 26 file-touches, +1569/−310 (code-only +1519/−298); net increment vs #117 headc15a989: 27 files,
+1498/−239 (code-only 1675 lines). - Inherited:
- #117's reviewed content (
6480b6b"qwen: share vision execution for 4B and 27B models", 12 files
+1327/−653) and its stack merges below the merge base. - #117's post-review advance (
6480b6b..c15a989): the FIFO-eviction regression, doc tweaks, and —
carried inside merge20b480awith no named commit — thefloat_reprrewrite and its tests (see
P2-1). This advance is approved-in-baseline only up to6480b6b;c15a989content beyond it has no
review trail. - Main-side content below the merge base flows through the stack's merges.
- #117's reviewed content (
- 3k-rule state (draft): 4050 authored-code lines > 3000. The body provides split rationale ("The
dependency inflates the main diff; rebase after #117 merges"), a component map + review order ("Review
the #117 shared backend separately, then image decoding/export validation, JEV processing/execution,
and their root-level tests"), and per-area validation with unverified items disclosed. The four
self-review checkboxes are unchecked — consistent with the stated intentional draft. Preparation is
substantively present; completed self-review remains outstanding. - PR body vs the current post-#138 template: the four "Required human contributor answers", the
PR-necessity decision, and the bug-fix evidence section are absent (the body uses the older
Purpose/Test Plan/Test Result/Demo/Self-review structure). DRAFT, so reported as state, not gated.
Commands run and results
Source inspected: all five authored commits' full diffs; head-state reads of image_decode.rs,
image_preprocess.rs, vision.rs, processing.rs, executor.rs, images.rs, caches.rs, engine.rs,
main.rs, lib.rs, ops.h, cuda.rs (diff), export_vision.py, preencode.py, recipe/model READMEs,
tests/{jev_vl,qwen3_5}/*, PR body, current template. Merge/blob forensics for json.rs provenance
(git rev-parse <rev>:<path>, --find-object, parent blob comparison).
| Command | Result | Class |
|---|---|---|
git fetch origin main |
71857e033b2d |
personally run |
shasum -a 256 review SKILL.md |
58be3bc6…dde108 (match) |
personally run |
git fetch origin pull/139/head (twice) |
953b6914fbb2 pinned; later f88d9182219b |
personally run |
gh pr view 139 |
OPEN/DRAFT, CONFLICTING, 39 files, body text | observed |
gh pr checks 139 / gh run view |
first query: no checks; ~30 min later: 4 pass + deploy skip at f88d918 (rust 1m38s, build 19s, cuda-contract 5s, benchmarks 13s); aggregated rust tests 212 passed / 0 failed / 28 ignored |
observed |
python -m unittest discover -s tests/jev_vl -p 'test_export_vision.py' -v (worktree @ 953b691) |
1 test, OK | personally run |
mkdocs build --strict (worktree @ 953b691) |
success (0.27 s, no warnings) | personally run |
| Pillow 11.3.0 conversion probes for the 16-bit PNG modes | match Rust test expectations (see Notes) | personally run |
Python 3.12.8 repr() oracle check for the 15 float_repr expectations |
15/15 match | personally run |
Code-line accounting (git diff --numstat, per scope) |
4050 / 1675 — matches PR body | personally run |
Author-reported local Rust checks at 953b691 |
fmt/clippy/test/build pass, 157 tests / 23 ignored (--offline --features omni-laya/serve) |
author-reported |
CUDA/GPU correctness + latency for online vision vs autotrust/JEV-27B-VL@f34b598… |
pending per author | author-reported (pending) |
CI observed
At the pinned head 953b691: no runs existed. During the review the branch moved to f88d918 (merge of
main 71857e0; every reviewed PR file blob-identical, ci.yml = main's + the one authored line), where
GitHub CI is green: rust, build, cuda-contract, benchmarks pass; deploy skipped (runs
38247490258/38247490263). The 28 ignored tests are the GPU/checkpoint-gated ones; ignore ≠ GPU evidence.
The author's local command set (--features omni-laya/serve) matches main's current CI flags.
Material unverified checks
- No Rust toolchain on this host —
cargo fmt/clippy/test/buildnot independently run at either
head; author's local results and CI atf88d918are the only executions of the Rust suite. - No GPU — the online vision path's numerical parity (shared v2 kernels on the 27B layout) and
latency vs the pinned checkpoint are unverified; the previously validated H800/2×2-corpus evidence
covers the offline preencoded path only, exactly as the PR body concedes. Thecuda-contractjob is
file-text checking, not kernel execution. - Pillow parity beyond the tested modes (Adobe CMYK JPEG inversion, palette PNGs, exotic chunks)
not verified; low likelihood, no evidence either way. - The
f88d918merge of main introduces main's Laya/LFM/Valen/OmniJev content into the branch tree;
those are main-reviewed contents and were not re-reviewed here, but the combined tree's integration
(e.g., workspace builds across new crates) has only CI evidence atf88d918, not at the pinned head. - Self-review checklist and human-intent template sections incomplete (draft state, disclosed).
…bounds Address PR InferMatrixLab#139 review: the architecture page still claimed image features must be prepared offline; the recipe documented neither the online vision warmup that now gates readiness nor the transient host memory a many-image online request holds outside the cache budgets. Co-Authored-By: Claude Code <noreply@anthropic.com>
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-check at new head d5246e22d: one new commit since my review at f88d918221 — "docs: record online image support, warmup readiness, and per-request bounds" (3 files, docs-only); the eight reviewed commits are patch-identical (git range-diff, named refs, cat-file verified).
All four findings from my review are addressed, verified at the new head:
- P2 (stealth shared-contract change) — addressed by disclosure: the PR body now carries a "Disclosed provenance exception" section that states the
float_reprrewrite (+18/−12 insrc/models/qwen3_5/native/src/json.rswith its tests) enters via #117's merge20b480awith the result blob in neither parent and no named home, that it is shared by every Qwen consumer and not covered by #117's baseline approval, and commits to either giving it a named commit or excluding it before this stack merges. That is the attribution action the P2 required; the provenance item stays open until that named home exists, but it is now tracked in the PR itself rather than invisible. - P3 (architecture doc) — fixed:
docs/architecture.mdnow reads "Images are either prepared offline by Transformers or supplied inline and encoded inside the worker by the shared native vision backend." - P3 (undocumented readiness gate) — fixed:
recipe/jev_vl/README.mddocuments that withJEV_VL_VISIONthe worker binds only after a successful inline-image warmup (1×1 PNG through decode/preprocess/forward), so/healthimplies the online path ran once at startup, while still not validating per-user images. - P3 (unbounded in-flight buffers) — documented as suggested: the recipe states the ~1.5 MiB pixels + ~1.25 MiB embeddings per minimum image, the ~250-image/4 MiB-body worst case at ~0.7 GiB transient, that cache budgets exclude in-flight inputs, and sizes workloads accordingly.
mkdocs build --strict passes on the full tree at this head; fork CI (rust/build/benchmarks/cuda-contract) is green at this exact head. Zero actionable findings remain. Verdict: COMMENTED — still a draft, so no approval. Unverified (unchanged): no GPU on this host — online-vision numerical parity/latency vs the pinned reference remains author-run and pending (prior H800 evidence covers the offline path only); Rust suite covered by the exact-head fork CI; body template state (human answers/checklist) remains to be completed before leaving draft per the post-#138 template.
cua_s1 consumes the shared TensorStore header/offset/path checks instead of a private duplicate (27B layouts stay rejected for the 4B worker); the JEV cache controls document l2_hit/l2_miss as lookup counters and pin the LRU-refresh on admission rechecks; the recipe pins explicit eager vision (CUA_S1_VISION_GRAPH=0) for the documented path. Adds the byte-level vision checkpoint regression suite, the non-UTF8 JEV_VL_IMGCACHE fallback test, and GPU-gated 27B-layout v2 position/rope/attention kernel checks. (Prepared as tree 60a8fbe by the codex jev session; landed after independent review.) Co-Authored-By: Claude Code <noreply@anthropic.com>
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-check at new head ceadfd7d3b: one new commit since my review at d5246e22d — "docs: correct online image memory and cache semantics" (docs-only); the nine reviewed commits are patch-identical (git range-diff, named refs, cat-file verified).
The correction improves its own documentation's accuracy, reviewed and confirmed: embeddings are BF16 and 0.625 MiB (not 1.25 MiB as my earlier note echoed), pixel buffers are released as image inputs are consumed during vision resolution rather than held to request completion, cache hits may reuse validated assets or retained image-prefix state (refining the warmup wording), and the memory statement is now explicitly labeled "not a measured peak-memory bound". Zero actionable findings at this head.
Verdict: COMMENTED — still a draft. Unverified (unchanged): online-vision numerical parity/latency vs the pinned reference remains author-run and pending (no GPU on this host); the float_repr provenance exception tracked in the body stays open until the change has a named home; body template state to be completed before leaving draft.
The sticky drain checks ran after `language_drain?; vision_drain?` would have been fine only when the request succeeded; on a failed request the first `?` on drain results replaced the root-cause error with a drain error and dropped the original context entirely. Merge drain failures into the existing result instead: the request error wins, and a drain failure on an already-failed request is logged rather than substituted. Co-Authored-By: Claude Code <noreply@anthropic.com>
Re-listing all thirteen VisionConfig fields in a json! literal could silently drop any member added later; serialize the parsed struct instead so the shared model configuration always sees the full set. Co-Authored-By: Claude Code <noreply@anthropic.com>
The single existing test covered only three of the four backbone dimension checks. Extend it to the top-level model_type and cover the remaining gates in execution order: pre-existing output, the exact eight-key processor contract (drifted value, extra key, missing key), non-language adapter tensors, an empty vision tensor set, shard paths escaping the model directory, and non-BF16 or non-finite weights; each asserts weights are never saved and no partial output is left behind. A happy-path test pins the manifest: exact format and model id, three output file hashes and the three checkpoint pin hashes recomputed from disk. Co-Authored-By: Claude Code <noreply@anthropic.com>
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-review #4 — PR #139 at 08a13924
Independent incremental re-review of four new commits since my last check at ceadfd7d3b. Local, no GitHub posting; draft state and evidence gaps reported as-is.
Incremental scope (pinned)
- Live head re-fetched and pinned:
git fetch origin "pull/139/head:refs/remotes/origin/pr/139" --force→08a13924d03e70bc089b3866e79956bce1303e3e(matches GitHubheadRefOid; branchcodex/jev-vl-online-upstream). Review baseline pinned:git fetch origin ceadfd7d3...:refs/remotes/pin/139→ceadfd7d3bc2d4189a44c0a43280a20cd2959f08, which is the direct parent of the first new commit (git rev-parse 08a13924~4=ceadfd7d3), so all eleven previously reviewed commits are unchanged by construction; the four-commit range-diff shows the new commits only. Delta: 15 files, +1051/−200. - New commits:
f0febb0(tests: cache counting + shared checkpoint regressions),ea48a42(fix: error precedence),79c3b17(fix: config derivation),08a1392(test: exporter rejection gates). - Body Review-scope claim re-verified exactly: after
git fetch origin main, merge-base(head,origin/main=3e9cc33a) =71857e033b2d2d0587742c1c1a5260d683ab17cd, andgit diff 71857e0..08a13924= 45 files, +4154/−1211 — matches the body's stated counts. - Status change: the PR is no longer draft (
isDraft: false, OPEN). The body discloses this ("left draft for review intake on 2026-10-12") with the #117 dependency and current-head GPU validation still tracked as open, but all four self-review checkboxes remain unchecked.
Per-commit review
ea48a42 — keep the original request error when post-error drains also fail (src/models/jev_vl/native/src/executor.rs, +20/−3)
Before: language_drain?; vision_drain?; let probabilities = result?; — both drains always ran, but on a failed request the first failing drain error replaced the root-cause error via ?, dropping the failing stage's context. After: the request result is carried first; a drain error only becomes the returned error when the request otherwise succeeded (now with "language drain"/"vision drain" context, which the old raw ? also lacked); a drain failure on an already-failed request is logged (eprintln!, the established convention in this file at line 361) instead of substituted. Both drains are still always attempted, preserving the sticky-drain/admission contract, and the success path (L2 insert, prefix publish) is unchanged. Ordering is deterministic (language before vision). Sound.
Test coverage: none in the commit, and none exists — the drain path requires an initialized device model (model.synchronize()), so it is not CPU-testable on this host; consistent with repo routing of device paths to #[ignore]-gated tests, but no such test was added here. Noted as a residual gap, not a defect.
79c3b17 — derive the shared vision config from the parsed struct (src/models/cua_s1/native/src/vision/{mod,model}.rs, +13/−13)
Before, VisionModel::load re-listed all 13 VisionConfig fields into a json! literal before handing them to the shared omni_qwen3_5_native::vision::VisionConfig::from_value — a duplicated schema where any later field addition would be silently dropped. After: serde_json::to_value(c) serializes the parsed struct (Serialize derived). Verified non-divergent today: the struct has plain field names with no serde(rename)/defaults, so the serialized key set is byte-identical to the old literal, and both paths funnel through the same from_value (parse + pinned-layout validate). Behavior-identical; removes only the silent-drop risk, exactly as claimed. No dedicated test pins the round-trip; the new checkpoint suite exercises VisionConfig::load on valid configs indirectly. Acceptable.
f0febb0 — cache counting pins, shared checkpoint regressions, non-UTF8 fallback, GPU-gated v2 kernel checks (11 files, +796/−184)
tests/jev_vl/caches.rs(new, +73, registered[[test]] jev_vl_caches): three weight-free tests. Mutation check:l2_cold_rechecks_count_each_lookupcatches removing the miss counter on cold lookups or on the disabled/over-budget early-return (an undercount that would silently corrupt/v1/cache/stats-based sizing decisions);l2_recheck_sees_a_queued_publication_and_refreshes_lrucatches loss of LRU refresh on hit — without the refresh, thenewerinsert would evictqueuedinstead ofolderand the hit/miss assertions flip;l2_disabled_or_over_budget_rechecks_remain_counted_missescatches counter loss on all three disabled modes and confirms no record/byte accounting leaks. These pin real, breakable behavior.- Source riding in a "tests:" commit: (1) cua_s1's private
TensorStore/validate_header/inside/is_visual/UniqueMap(−182) deleted; the shared qwen3_5 implementations are madepuband consumed. I compared the deleted private code against the shared code side-by-side: textually identical logic (same signature, index/shard check, 100 MB header cap, checked offset arithmetic, duplicate/missing/unexpected rejections), so the dedup is behavior-preserving; and (2)JEV_VL_IMGCACHEis now carried asOsStringand a non-UTF8 value is treated as unset (falling back to the default prepared cache dir), matching the prepared worker'senv::varparity; the mutual-exclusion check consequently no longer fires for non-UTF8 imgcache + online vision. Both changes are disclosed in the commit message and the body. See findings for the labeling issue. tests/cua_s1/vision_checkpoint.rs(new, +404): 8 weight-free regression tests building synthetic checkpoints (sparse zero payloads, hard links, no weights).assert_errorpanics on unexpectedOkand checks the message — fail-loud. Theboth_rejectpattern runs each malformed input against both the sharedQwenCheckpointand cua'sVisionCheckpoint, which is precisely the regression pin for the dedup above; coverage includes header length bounds, duplicate JSON keys,usize::MAXoffsets on ignored language tensors, dtype/shape/missing/unexpected inventories (base BF16 and LoRA F32), index duplicates/mismatch/escaping shards, unix symlink escapes, andqwen_27b_layout_does_not_expand_cua_4b_support(shared validate accepts both layouts; cua's local pin stays 4B). Also pins inventory sizes (297 base / 100 adapter names) and marker bytes at exact offsets.tests/qwen3_5/kernels.rs(+234): two new tests, both#[ignore = "needs a GPU and CUA_S1_CUDA_LIB with vision v2 symbols"]inside the already-registered[[test]] kernelstarget — matches repo precedent exactly and stays unexecuted here (no GPU). Bit-exact (to_bits) pins for v2 position-gather and RoPE at dh=72 over n ∈ {1, 3, 65} with pinned bf16 rounding points, and a padding-attention test whose workspace is pre-poisoned with 9.0 to catch missing zero-writes to padded columns, checked bit-exact against an unpack reference plus the existing FlashAttention relative-error metric and 1.6e-2 budget including tail rows. Good mutation-sensitive design; none of it has ever run.tests/jev_vl/vision.rs(+29):non_utf8_imgcache_preserves_default_selection_and_online_mode(unix-only) pins the source semantics change above: non-UTF8 imgcache retains the default directory in prepared mode and does not conflict with online mode, while a non-UTF8 vision path still errors.- Docs:
caches.rsdoc comments definel2_hit/l2_missas lookup counters ("not unique images or encoding attempts"; cold single image = 1 lookup prepared, 2 online), mirrored inrecipe/jev_vl/README.md(counters, retained-prefix skip, same-path comparison advice) and a cross-reference insrc/models/jev_vl/README.md. The recipe additionally pinsCUA_S1_VISION_GRAPH=0in the launch command and documentsCUA_S1_VISION_GRAPH/CUA_S1_GRAPH_TRACEas== "1"-enabled, default-off — verified against code (src/models/qwen3_5/native/src/vision/model.rs:130,576), so the eager-path claim is accurate.
08a1392 — pin every vision exporter rejection gate with mocks (tests/jev_vl/test_export_vision.py, +228/−4)
All eight rejection gates of recipe/jev_vl/export_vision.py are now pinned, in execution order: (1) top-level model_type + the four-way backbone gate (the old test covered three of the four; the drift values are the 4B numbers against the 27B VALID_CONFIG, so they are non-vacuous — I verified tests/qwen3_5/data/config.json is the 1152/5120/5120 layout the exporter requires); (2) pre-existing output, asserted before any weights/dependencies load (the test does not even patch sys.modules, so it also pins the lazy-import ordering — any reordering fails loudly); (3) the exact eight-key processor contract with drifted-value, extra-key and missing-key variants plus read_weights.assert_not_called(), pinning whole-dict comparison; (4) non-language adapter tensors; (5) empty vision tensor set; (6) shard path escaping the model directory; (7) non-BF16 and non-finite weights (sentinel-object dtype identity, faked isfinite); each rejection additionally asserts save_weights.assert_not_called() and no partial output directory; (8) a happy-path manifest pin: format jev-vl-vision/1, model id autotrust/JEV-27B-VL, three output file hashes and three checkpoint pin hashes recomputed from disk with real hashlib. The mocks are minimal but sufficient and fail-loud: if the exporter changed the tensor API it uses or stopped raising, the tests error rather than pass. Commit message's "four backbone dimension checks" resolves as model_type + three sizes, all four now covered.
Still-open items re-verified at 08a13924
- P2
float_reprprovenance exception — STAYS OPEN (tracked). The delta does not touchsrc/models/qwen3_5/native/src/json.rsortests/qwen3_5/json.rs(0 changed lines inceadfd7d3..08a13924), so the +18/−12 rewrite remains with no named home. The body's "Disclosed provenance exception" section is still present and still commits to "a named home (a commit in #117 or a separate small PR) or … excluded" before this stack merges. The commitment is intact but unfulfilled; this remains the blocking provenance item for merge. - P3 architecture offline wording — still fixed.
docs/architecture.mdhas zero delta lines sinceceadfd7d3(the corrected "supplied inline and encoded inside the worker by the shared native vision backend" wording stands). - P3 readiness gate documentation — still fixed.
recipe/jev_vl/README.mdlines 56–57 and 100–102 still document that withJEV_VL_VISIONthe worker binds only after a successful 1×1-PNG inline warmup. - P3 in-flight memory documentation — still fixed. The "Cache budgets exclude in-flight inputs … not a measured peak-memory bound" passage is present (recipe lines ~117–121), still with the corrected BF16/0.625-MiB embedding semantics from the
ceadfd7d3docs commit. - Draft/template state — changed. Not draft anymore (disclosed "review intake" exit on 2026-10-12). The human-contributor answers section is filled; the four self-review checkboxes are all unchecked and the body itself says current-head GPU evidence and contributor/maintainer review remain outstanding. Per the post-#138 template expectation I recorded earlier (checkboxes completed before leaving draft), this is incomplete: the PR is now reviewable/approvable in principle while its own self-review section declares the outstanding items.
New findings
- P3 (process, no code defect):
f0febb0is labeledtests:but carries two source changes — the shared-TensorStore dedup (−182 lines of cua_s1 checkpoint-validation code, now consuming newly-pubshared code) and the non-UTF8JEV_VL_IMGCACHEsemantics change (engine.rs,vision.rs). Both are disclosed in the commit message and the body, and both reviewed sound (textual-identity dedup; documented prepared-worker parity), but a conventional-commit type that understates src changes misleads changelog/bisect tooling. Splitting or relabeling (e.g.refactor:+fix:) would have been correct. - Observation (no action required): test-registration deviation.
tests/cua_s1/vision_checkpoint.rsis wired via#[cfg(test)] #[path = ...] mod tests;insidevision/mod.rsrather than the repo's[[test]]precedent. This is technically necessary — the suite exercises the privateVisionConfig::loadviause super::*, which a root-level[[test]]target cannot — and it runs undercargo test -p omni-cua-s1-native/workspace runs, but it is a second registration idiom in one tree. - No new P0–P2 findings. Both fixes are correct and minimal; the tests genuinely pin (fail-loud) the behaviors they claim.
Evidence
Commands I ran personally (host: macOS arm64, no Rust toolchain, no GPU):
shasum -a 256 .agents/skills/system1-omni-review/SKILL.md→58be3bc60806c4c5af7038a0c6a80fadb94f3360d714556a3e965c8903dde108(matches pinned value; skill read from the working tree at/Users/hsliu/thinkflowlab/system1-omni, unpublished local copy, content-hash recorded).- Read
docs/architecture.mdand.agents/skills/system1-omni-review/references/repository-contracts.md(audited-baseline map; target code re-checked at head for all touched components). - Ref pinning and structure: fetch +
rev-parseas above;git log/diff --stat/merge-base/range-difffor the delta; per-commitgit showfor all four commits; side-by-side read of deleted vs sharedTensorStorecode. - Weight-free Python tests (the files the commits touch), run against the exact head tree: working tree is not at head, so I extracted
tests/jev_vl/test_export_vision.py,recipe/jev_vl/export_vision.py,tests/qwen3_5/data/config.jsonfromrefs/remotes/origin/pr/139viagit archiveinto/tmp/pr-review-2026-10-10/p30-py/(repo untouched) and ran/Users/hsliu/miniconda3/bin/python …/test_export_vision.py -v→ Ran 8 tests … OK (8/8 pass, 0.009 s), matching the body's claim. Host Python has no usabletorch(import fails), which the suite is designed around (lazy exporter imports +sys.modulesmocks). mkdocs build --strict: not re-run — the delta contains zero changes underdocs/ormkdocs.yml(verified by empty diff); the touched READMEs are not part of that build's input delta.- CI at the live head:
gh pr checks 139→ "no checks reported on the 'codex/jev-vl-online-upstream' branch"; API confirms 0 check runs and commit statuspendingwith 0 statuses for08a13924. The body discloses the same. Green fork CI evidence ends atceadfd7d3(body-linked runs 38265875700/38265875936).
Material unverified checks
- No CI at the live head. The four new commits have zero GitHub check runs. The author reports local
cargo check --locked --testsclean foromni-jev-vl-native/omni-cua-s1-native, 19 Cua CPU tests, 8/8 exporter tests, and thef0febb0tree's 40 CPU tests — all author-run, none corroborated by CI or by me (no Rust toolchain on this host). The Rust compile/test claims forf0febb0/ea48a42/79c3b17/08a1392therefore rest solely on author-reported evidence. - The two new v2 GPU kernel tests have never executed (GPU-gated
#[ignore], no GPU here or in CI); they pin dh=72 27B-layout kernels whose parity remains author-claimed via the pending H800 run. - Current-head GPU numerical parity and latency remain pending (unchanged): no online-vision comparison against pinned
autotrust/JEV-27B-VL@f34b598dhas been reported; historical prepared-image H800 evidence does not cover this path. - The
float_reprprovenance item stays open until executed: the body's commitment (named home or exclusion before merge) is intact but not yet actioned;json.rsremains home-less in the diff. - Self-review checkboxes unchecked; #117 dependency still open (the PR includes #117's shared backend in its displayed diff by the author's own accounting).
Verdict: COMMENTED. The four new commits are sound and their tests are genuine pins; no new P0–P2 findings. The PR leaving draft with unchecked self-review boxes, no CI at the head, the still-unexecuted provenance commitment, and pending current-head GPU evidence keep this at comment level — none of these imply approval or completed validation.
Resolve native_vision_graph.md in favor of upstream's CUA_S1_GRAPH semantics (text and multimodal language execution) while keeping this branch's FIFO scratch/Graph residency and the shared-crate regression commands. Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
The 4 MiB axum DefaultBodyLimit rejected every request carrying even one maximum-size image (4 MiB raw ≈ 5.33 MiB base64) before the structured validators ran, so the advertised per-image caps were unreachable and their BadRequest rejections dead code. Over-limit clients also got axum's plain-text 413 instead of the worker's error envelope. Raise the transport cap to 8 MiB (the number the Cua-S1 vision worker already uses) so at least one maximum-size image fits, and map transport rejections into the official envelope with status 413. Pin both halves with raw-HTTP tests through the production limit layer: an over-limit body returns the 413 envelope, and a 5 MiB body now reaches request validation instead of dying in transport. Co-Authored-By: Claude Code <noreply@anthropic.com>
Engine::load streamed the full vision.safetensors through sha256 on a Tokio worker thread. Move validate_export into spawn_blocking so the GB-scale startup pass no longer occupies the async runtime; the executor's device upload already re-reads the same file once. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ring ImageLimits and preprocess_rgb8_with_limits had public visibility with a single consumer (the cua() preset behind preprocess_rgb8); keeping them crate-private keeps the limit surface single-sourced so a future caller cannot compose caps the model side never validated. url_key and the export pin check each hand-rolled digest-to-lowercase-hex formatting; route both through contract::hex_digest so the L2 keys, imgcache directories and manifest pin hashes cannot drift apart. Co-Authored-By: Claude Code <noreply@anthropic.com>
The body-mapping fallback mapped every non-413 transport rejection to 400; a mid-read connection failure is axum's 500, not a client error. Add Reject::internal and route that branch through it. Co-Authored-By: Claude Code <noreply@anthropic.com>
A truncated upload surfaces as axum's UnknownBodyError, which is 400; mapping it to 500 relabeled an interrupted client delivery as a server fault (and broke the status the un-mapped route already returned). Keep the envelope but mirror axum's statuses: 413 over the cap, 400 for other buffer errors, so Reject::internal is removed again. Returning Reject from the mapper, with each handler rendering the response, also keeps the helper's error type small enough for clippy::result_large_err under -D warnings. Pin the 400 path with a truncated-upload regression. Co-Authored-By: Claude Code <noreply@anthropic.com>
The recipe README moved to 8 MiB with the transport fix; the model contract page still said 4 MiB. Co-Authored-By: Claude Code <noreply@anthropic.com>
There was a problem hiding this comment.
Incremental re-check at new head 034117ed4b after the rebase/merge onto main@32bdd2a (which lands the jemm integration, #155). Own-commit series patch-identical through the rewrite and merges (git range-diff, named refs, cat-file verified; the large merge list is the absorbed stack/main history, consistent with the pass-57 wave verification). My prior approval and its carried notes apply unchanged: online-vision design verified, float_repr named home present via #117's 900ad7a, template note (post-#138 answers/checklist) unchanged. Fork CI at this exact head: benchmarks/build/cuda-contract/rust green.
APPROVED (verdict carries — content-identical rebase). Stack-order note: this PR sits on the unmerged jemm stack; merge order remains with the maintainer.
… parity
float_repr renders structured state into the prompt of every
shared-Qwen consumer (Cua-S1, Open-Jev, JEV-VL). Rust's {:e} picks the
upper digit at some exact shortest-decimal ties, while Python repr()
breaks them to even — a divergence the old doc comment admitted.
Select digits via serde_json's default formatter (ryu) instead, and
pin 15 Python repr() examples and five bit-exact tie cases in tests.
Provenance: the change arrived inside #117's merge 20b480a ("Merge
main and preserve shared vision graph regressions", authored by
levius); its result blobs match neither parent and belong to no named
commit. Extracted unchanged here to give it a named, bisectable home,
as requested by the #139 review.
Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: linear3735 <heshming@qq.com>
|
@linear3735 Heads-up: the upstream dependency PRs #117 and #118 have now merged (merge commits Branch boundary: #117 merged into The current ABI8 full-27B/JEMM qualification at #118 source You can now refresh #139 against the merged dependency, resolve the shared-vision/backend overlap, and review the remaining JEV-VL integration diff. The current #139 description still records the older ABI6 integration and #117 HOLD; please update those dependency/version statements as part of that refresh and rebuild the CUDA library and its consumers together for ABI8. The JEMM results do not qualify #139's online JEV-VL path: its refreshed source still needs its own appropriate CPU/GPU correctness checks, with historical H800 results kept under their original source identity. The reviewer-accessible raw JEV-VL GPU evidence and maintainer review already tracked in #139 remain separate acceptance items. Thanks! |
Conflicts resolved: - src/models/qwen3_5/native/Cargo.toml: keep both sides' [[test]] targets (our image_decode alongside upstream's device_features). - recipe/cua_s1/native_vision_graph.md: take the upstream wording and keep our clarifying sentence that vision Graph alone does not enable language Graph replay. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ice binding InferMatrixLab#117 binds every DeviceBuffer to its allocation device: DeviceBuffer::new now reads the current device through the loaded CUDA library before any allocation, so a zero-byte buffer can no longer be constructed on a CUDA-free host and the two CPU-only offset tests cannot run. The assertion they pinned is carried by upstream itself (assert!(offset <= self.bytes) in DeviceBuffer::at), so no product behavior is lost; the test target and file are removed. Co-Authored-By: Claude Code <noreply@anthropic.com>
The JEMM integration (InferMatrixLab#155, main 32bdd2a) consumes ImageLimits and preprocess_rgb8_with_limits from omni-jemm-native, so the second consumer the crate-private note was waiting for now exists on main; restore pub for both. No semantics change. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
@Levius-Fubuki Refresh done at head
Thanks for the heads-up on the branch boundary and the ABI 8 note! |
hsliuustc0106
left a comment
There was a problem hiding this comment.
Incremental re-review at head 034117ed4b75fb2ad5bdb8608509e1295a25f89d (remote head re-pinned immediately before posting; PR OPEN, non-draft). Prior posted reviews: five rounds 2026-10-12/13, last at 26dc76bf6. A later re-check of the 2026-10-21 merge had been recorded by editing that older review's body instead of posting a new review; this review re-establishes the visible trail at the current head with fresh verification of everything claimed.
Delta since the last posted-review head 26dc76bf6
The branch is rebased onto current main 32bdd2aa (landing the #155 jemm integration and the approved #117/#118/#122/#123/#143 stack) via merges 172fad73, 12601261, d92965bd, plus two authored commits:
5901747a— dropstests/qwen3_5/device_buffer.rs(CPU-only zero-byte DeviceBuffer checks) and its Cargo.toml registration, superseded by #117's device binding /device_featurestests.034117ed— restorespubonImageLimitsandpreprocess_rgb8_with_limits(3 lines, +doc-comment update naming both consumers: cua-s1 and the jemm consumer #155 brought to main). No semantics change.
Verification (commands on this host)
- Merge hygiene:
git merge-tree --write-tree 26dc76bf6 32bdd2aayields a conflicted auto-merge tree (531cc9dfa); the actual merge tree differs in exactly 4 files and I read all 4 resolution hunks. Each is a clean union of both sides —.github/workflows/ci.ymlretains BOTH the branch'stests/jev_vlvision-export unittest step and main's Decider validation block;recipe/cua_s1/native_vision_graph.mdkeeps the branch's fuller wording (main's side was a truncated duplicate);src/models/qwen3_5/native/Cargo.tomlkeepsimage_decode(branch) +device_features(main); thedevice_buffer.rsdeletion is consistent with authored5901747a. No novel conflict-resolution content. - Reference consistency at the final head: no dangling
device_bufferregistrations in CI/src/Cargo.toml;tests/qwen3_5/device_features.rs(main side) andtests/jev_vl/test_export_vision.py(referenced by the retained CI step) both exist;device_buffer.rsabsent. - Authored commits read in full as shown above; both minimal and correctly motivated.
- The PR's own online-vision series is untouched by the merge (own commits unchanged on the first-parent line; the PR's footprint vs main remains the previously reviewed online-image path, verified across the five prior rounds).
- CI at the exact head
034117ed4b: benchmarks / build / cuda-contract / rust all green (fork runs 39624221106, 39624221114). - Template gate: the four human contributor answers and the four declarations authorized 2026-10-13 remain in the body; the self-review section honestly lists remaining acceptance items (ABI-8 rebuild of this PR's own GPU evidence, fresh reviewer-accessible GPU evidence, maintainer review).
- ABI note: the body states the native ABI in this PR remains 6; the merged tree compiles green under CI. ABI-8 runtime qualification remains the contributor's disclosed remaining item, consistent with the cross-stack reconciliation disclosed in #112.
Verdict
APPROVED — zero findings in the delta. Material unverified items unchanged and disclosed: the H800/GPU evidence is historical and bound to the older source (stated in the body), no GPU re-run is possible on this host, and the ABI-8 rebuild of this PR's own evidence remains outstanding with the contributor.
Required human contributor answers
Reused from the contributor's explicit instructions supplied during PR preparation.
Is a PR needed? Yes. The contributor explicitly requested submitting the current Omni integration before proceeding with Agents integration.
Observed bug evidence: The online-image path is a feature addition. Its HTTP transport fixes are covered by the raw-HTTP regressions: bodies above the old 4 MiB cap reach the worker's structured handler; oversized bodies return a structured 413; a truncated upload returns a structured 400. The truncated-upload probe previously returned 500 at
4ea23c6; the regression passed at the then-current26dc76band also passes in the CPU-tested merge described below.Purpose
JEV-27B-VL currently requires images to be encoded offline before a native request. This change accepts inline PNG/JPEG images and runs decoding, preprocessing and the shared vision tower inside the Rust/CUDA worker, retaining prepared-image compatibility and the existing choice/noul/score contract.
The integration adds pinned vision-export validation, startup image warmup, online/prepared input handling and cache-boundary tests. The HTTP body cap is 8 MiB so a 4 MiB decoded image can fit after base64 encoding. Oversized and interrupted uploads retain the worker's structured 413/400 errors. Export pin-checking runs on Tokio's blocking pool before executor initialization. Per-image limits remain unchanged.
Depended on #117, which merged into
mainas0410e841d3ec72c1462609fa7721d0e424ac356con 2026-10-21, releasing its hold. The 2026-10-21 merge update below integrates that merged ABI 8 shared-vision backend (and the subsequent #155 JEMM integration); the remaining integration diff against merged main needs a fresh review, and this PR's own approval requirement is unchanged. The namedfloat_reprprovenance entered #117 without rewriting its published history and is an ancestor of that merge.Current merge update (2026-10-21)
Head:
034117ed4b75fb2ad5bdb8608509e1295a25f89d. Merged main:32bdd2aade0e0f3f0452d04ca5022fcaaa4783c9(includes the merged #117 shared vision backend and the #155 JEMM integration).Two ordinary two-parent merges from
172fad7319f70e6b9e03038bbd5ca44e466ad4c0(via12601261, thend92965bd); no rebase or history rewrite. Textual conflicts, both resolved by retention:src/models/qwen3_5/native/Cargo.tomlkeeps both sides'[[test]]targets (ourimage_decodealongside upstream'sdevice_features), andrecipe/cua_s1/native_vision_graph.mdtakes the upstream wording (including its new ABI 8 and GPU-resident-feature sections) while keeping our sentence that vision Graph alone does not enable language Graph replay.Two semantic follow-up commits:
5901747adrops the two CPU-onlyDeviceBufferoffset tests: qwen: share vision execution for 4B and 27B models #117 binds everyDeviceBufferto its allocation device, soDeviceBuffer::newnow reads the current device through the loaded CUDA library before any allocation, and a zero-byte buffer can no longer be constructed on a CUDA-free host. The assertion those tests pinned is carried by upstream itself (assert!(offset <= self.bytes)inDeviceBuffer::at); no product behavior is lost.034117edrestorespubforImageLimitsandpreprocess_rgb8_with_limits: the JEMM integration on main now consumes both fromomni-jemm-native, so the second consumer the crate-private note was waiting for exists. No semantics change.The native ABI is now 8: the merged backend adds
cs1_get_deviceandcs1_bf16_finiteand rejects older libraries. The CUDA library and its consumers must be rebuilt together for ABI 8 before any GPU run of this head; no fresh GPU, CUDA compilation, model HTTP replay or browser run was performed on this merge head. The H800 and browser evidence below stays bound to its original source (26dc76b/ Agents0def927) and its stated ABI 6 environment. Per the collaborator's 2026-10-21 integration note, the JEMM ABI 8 A800 qualification does not qualify this PR's online JEV-VL path; this refreshed source still needs its own GPU correctness checks, which remain an open acceptance item.Local CPU checks on this merge head passed:
cargo fmt --all --check;cargo clippy --workspace --locked --all-targets --features omni-laya/serve -- -D warnings;cargo test --workspace --locked --features omni-laya/serve(exit 0, zero failures across all reported suites); the Python vision-export and benchmark suites (8/8 and 51/51 after installingbenchmarks/requirements.txt; the pre-installhttpximport failure is environmental and retained in the local record).Earlier merge update (2026-10-18, superseded)
Head:
172fad7319f70e6b9e03038bbd5ca44e466ad4c0. Merged main:e66ff34996760a84ad56690f0ae4cf434e669e65.This is a normal two-parent merge from
26dc76bf65c544815e7b285866791da1e703816d; no rebase or history rewrite. The sole textual conflict was in.github/workflows/ci.yml: both the JEV-VL vision-export CPU tests and main's Decider validation-helper tests are retained. Main also brings separate Laya/Decider host changes.All 88 audited JEV/Qwen/Cua/Open-Jev/CUDA/Cargo source blobs match
26dc76b. The H800 and browser evidence below remains bound to that older source and its stated environment; no fresh GPU, CUDA compilation, model HTTP replay or browser run was performed on this merge head. “Current-head” in the retained contributor answer refers to the original bounded validation, not new execution of this merge. The native ABI in this PR remains 6.Test Plan
Local checks on the merge head:
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/serveAlso run: the frontend API target; benchmark manifest validation and benchmark tests; vision-export rejection tests; Decider helper tests; CUDA checker/backend helper tests and text contract checking; JEV CLI help; strict MkDocs build.
The retained single-H800 run rebuilt the ABI 6 CUDA library from
26dc76b, compared all three decision types against pinnedautotrust/JEV-27B-VL@f34b598d4ef4bcefd337bee8d8e7ddd3b7733ccc, checked changed images/cache boundaries, and measured complete frontend HTTP requests for fresh images, retained images with new questions, and exact repeats. It retained the per-probability absolute tolerance of 0.025 and matching decision fields. Opt-in vision Graph was outside that default-path validation.Test Result
034117ed4b75fb2ad5bdb8608509e1295a25f89dand is MERGEABLE (overall merge stateCLEAN). Exact-head Actions: CI: rust/benchmarks/cuda-contract SUCCESS; Docs: build SUCCESS, deploy skipped. An earlier same-day rust failure at5901747(run 39622908384) is retained here for completeness: main advanced to the JEMM integration (Integrate JEMM native serving and Cua-S1 precision fixes into main #155) between merge and check, and its consumer needed the two symbols restored topubby034117ed.httpxfailure is retained in the local record; the checks were rerun successfully in a personal isolated environment.172fad7passed within their source/merge-integration scope. Kimi reviewed frozen source and supplied command records; it did not rerun tests. These are contributor reviews, not maintainer approval or GPU acceptance.172fad7319f70e6b9e03038bbd5ca44e466ad4c0and is MERGEABLE (no conflicts; overall merge stateCLEAN). Exact-head Actions: Docs: SUCCESS; CI: SUCCESS. Head check snapshot: 1 SKIPPED, 4 SUCCESS.26dc76b, with PR deployment skipped. They do not establish CI success for172fad7.26dc76bsingle-H800 evidence: native primitive 2/2; correctness 114 blocks / 612 requests, maximum probability delta0.00263093220749; cache eviction 12 blocks / 36 requests, maximum delta0.00231216420586. A separate fresh performance run completed 60 episodes / 1,680 raw / 1,080 timed requests. These are local audited results, not GitHub GPU CI. Broad model quality, every task/frontend, and end-to-end browser acceleration remain unverified.Demo / evidence
The pinned recipe provides export and launch commands; the model contract documents request, memory and cache boundaries. The HTTP regressions exercise the production fallback and shared body mapper without requiring a model or GPU. They do not establish real-model inference.
The same-trial browser evidence is public through the related Agents #78, which remains Draft:
58550ee293bbfb9c5b1f563c03e8cbdb6f6ac2a3ef23080f73ec3435b932a9e4.8e3242a6c275c4c1528f8a7643d2b8dcac2db3e9e2ebef4a1195cc8a095d78c6.The ZIP contains browser R3 records and service-support evidence. It is not the full raw 612-request correctness, 36-request cache or 1,080-timed-request GPU dataset. Publication of those raw GPU records remains a review gap. The Agents source changes are outside this PR.
Bounded runtime evidence (2026-10-14)
The measurements below use exact Omni source
26dc76bf65c544815e7b285866791da1e703816d; browser execution uses Agents0def92771746ec94adc38bf41be9720bccfbf19d. Agents #78 has since merged main; its 11 JEV-specific Python/test blobs remain unchanged, while inherited main changes are separate. Local code reviews and raw-runtime checks are contributor evidence, not maintainer approval.214.79 → 165.93 ms(1.29×); retained image/new question100.06 → 25.97 ms(3.85×); exact repeat99.15 → 25.84 ms(3.84×). Preserve the negative subgroup: cold score/JPEG native P95 was about 5.33% slower. These figures cover complete HTTP requests, not end-to-end browser task speed.hello worldPOST, four non-forced operation choices, seven visual HTTP 200 responses; six forced targets do not count as learned choices. Chat/value provided text and summary; its backend checkpoint and full cost were not independently established.58550ee293bbfb9c5b1f563c03e8cbdb6f6ac2a3ef23080f73ec3435b932a9e4. No exact frame/UTC mapping or per-request server logs are claimed.The original run-local records remain under
20261012T114800Z-online-vision-agents, revision26dc76b-agents-0def927. The public browser assets above cover only their stated subset; unavailable local paths are not offered as public evidence.Review scope
Against main
32bdd2aade0e0f3f0452d04ca5022fcaaa4783c9(merge-base equals base), the head diff is 37 files, +2720/−453; excluding*.mdandCargo.lock, 31 files, +2625/−431 (git diff --numstat 32bdd2a..034117e; the three-dot diff's SHA-256 ise4f1a5af572fbf5978b8c16674db4ae75a4cec5804b5b65cf8ccdab07ffc99c7). Review the remaining integration diff against the merged ABI 8 backend: image decoding/export validation, JEV processing/execution and their root-level tests, plus the two merge follow-up commits above.The path is opt-in through
JEV_VL_VISION. Source checkpoint and processor settings remain pinned, and prepared-image semantics are retained. The shared decoder, vision implementation and release DeviceBuffer bounds affect Cua/shared-Qwen/JEMM consumers; rebuild the CUDA library and consumers together for ABI 8. Further frontend orchestration, schedulers and the Agents adapter are outside this change.Disclosed provenance exception: the
float_reprrewrite insrc/models/qwen3_5/native/src/json.rs(+18/−12) and its tests (+36) originated in #117's merge20b480aand entered this PR throughb7442fd. It changes shared-Qwen prompt serialization and is outside the vision scope; #117's baseline approval at6480b6bpredates it. The unchanged patch was extracted into named fork commit900ad7a. The contributor selected #117 as the upstream home. That named commit is an ancestor of #117's merge commit0410e841(verified withgit merge-base --is-ancestor), but is not an ancestor of #139: this PR already carries the content through its older history. The provenance handoff used a normal merge that changed no source content and rewrote no published history; this is not a claim that #117's entire source tree is unchanged. #117 has merged; its approval does not substitute for approval of #139.Self-review
Remaining acceptance items: rebuild the CUDA library and consumers for ABI 8 and run this refreshed source's own GPU correctness checks (the JEMM ABI 8 qualification does not cover this PR's online path); a fresh review of the remaining integration diff after the 2026-10-21 merges; publish reviewer-accessible raw GPU evidence; obtain maintainer review for #139. The historical bounded GPU evidence keeps its original source identity and limitations. Exact-head hosted checks are reported separately above; older green CI does not cover this merge. The contributor's four answers and the four declarations authorized on 2026-10-13 are retained as existing attestations, without adding a new human attestation for this merge.