omnijev: add the native eager worker

[twu3202]

Required human contributor answers

1. What outcome are you trying to achieve?

   Serve OmniJev-4B v1.1 on system1-omni's native Rust/CUDA path.

2. What is the highest-leverage variable for that outcome?

   answer agreement with the reference first, asn then per-request latency.

3. How will this work move that variable rather than create busywork?

   omnijev: add the native eager worker #129 tries to minimize the difference from the reference, omnijev: reuse the prefix within a request #134 makes it faster without changing any answer.

4. Who is the customer or user, and what do they actually need?

   Anyone who wants OmniJev decisions rom system1-omni's server, and they need correct answers with low latency.

After answering the four questions: is a PR needed at all to achieve this outcome?

yes, to answer #114.

For a bug fix: link actual observed failure evidence.

N/A,, not a bug fix.

Purpose

Second PR for #114, the "native eager worker" step of the plan. It serves OmniJev-4B v1.1 on /v1/systemone with the vision tower and the language model on the shared Qwen3.5 CUDA path, one plain pass per (question, option) row, and the heads on the CPU. It builds on #120 (the contract and CPU side, merged), #98 and #99 (the fixed-algorithm GEMMs and run_window) and #117 (the shared vision tower and image preprocessing), and should be merged after them, in the order #98, #99, #117, then this PR; until then the diff here includes them. Its own changes are three commits: fc27b75 (33 files, +2,274/−267, of which 2,047 lines are code, without documentation, fixtures and the question file), 5cf7c4e, which keeps the worker's admission slot until a request's work ends, and a8f523a, which lists the worker in the qwen3_5 backend manifest that #77 added.

Shared Qwen crate:

• Model::forward_multimodal_readout(input, rows, targets): one multimodal prefill with the fixed-algorithm GEMMs, returning the final-norm hidden states at rows and, for each (row, token), the log-probability of token after row from the tied embedding (BF16 logits over the vocabulary in chunks of 64 rows, then a log-softmax on the CPU). With the fixed GEMMs a result doesn't depend on what else the call reads, which the request-local reuse in the next PR relies on.
• image_decode::decode_rgb8 and inputs::image_positions, moved from Cua-S1's image_request.rs and multimodal.rs, which now call them, and replacing OmniJev's own image_grid and image_positions from omnijev: add the request contract, preparation, heads and export #120.
• VisionConfig accepts qwen3_5_vision as well as qwen3_5: the Hub's Qwen3.5 configs say qwen3_5, and Transformers writes its class's qwen3_5_vision when it saves one, as export.py does.

OmniJev (src/models/omnijev/native):

• processing.rs decodes the image as Pillow's convert("RGB") does and preprocesses it with MSO1's 602,112-pixel budget.
• export.rs checks the export's manifest against the pinned release and every file it lists, the language weights included, against its SHA-256, and reads the calibration.
• executor.rs runs the vision tower once per request, then one pass per row over the shared prefix and that row, reading zq, u and, except for Score, the option-text log-probabilities; then the heads and MSO1._finish.
• main.rs is the worker (omni-omnijev-native): /health after a real warmup request, 400/413/422 with detail, serial admission, two requests preparing at a time and 503 beyond 16 in the worker. Until the rows reuse the prefix, a request may use at most 262,144 tokens in its rows' passes and read 16,384 option-text tokens.

recipe/omnijev/native.md builds, runs and compares the worker; validate.py runs the reference (also merged, or in float32) and the worker on the same images and compares every answer, and gate_questions.json adds eight harder questions for it. The model README and the supported-models rows describe the worker.

Test Plan

System1-Omni Version / Commit: b220f47, which merges #117 18094f1 and main 426e158 into the earlier head dac6ad6 (a merge of #99 18cbd99, with #98 d8cd04f, into a8f523a); 5cf7c4e is on #99 2ff1396 (with #98 d717e49) and #117 c15a989, on main 7cbce19. The comparison and the latency runs used the tree of fc27b75 (before the rebase onto #120's merge, which didn't change it); the HTTP checks ran on 5cf7c4e. On b220f47 the CPU checks pass, and so do the readout and worker GPU tests (one RTX 6000 Ada, CUDA 13.2).

• CPU: the five Rust steps in CI (format, Clippy, frontend tests, workspace tests, release build) and mkdocs build --strict. New fixtures from the reference's own image loading and processor: decoded RGB and pixel values for nine images (patterns at four sizes including 1920×1080 and 2000×20, RGBA, grayscale, palette, 16-bit grayscale and JPEG). Export-manifest checks on a stand-in export.
• GPU, one RTX 6000 Ada (sm_89), CUDA 13.2, driver 595.91.07, with the export of omnijev: add the request contract, preparation, heads and export #120:
  • all kernel tests;
  • tests/qwen3_5/readout.rs: for text, the readout equals forward_fixed bit for bit; hidden states and log-probabilities don't depend on which other rows and targets a call reads (130 targets, over two vocabulary chunks); a log-probability against a float64 CPU computation over the whole vocabulary;
  • tests/omnijev/worker.rs: repeatable answers, and the same answers with the questions in reverse order;
  • the opt-in CPU tests against the checkpoint and the export (token ids, heads).
• Comparison with the reference (validate.py): the reference's seven v1.1 demo stills with bench/speed_bench.py's twelve questions and the eight in gate_questions.json (options that differ in one token or share a long start, a Choice where no option fits, a 32-option Choice, a 10-level Score, plain and region Noul), 140 answers, with the stills as JPEG and as PNG (the RGB values Pillow decodes from the JPEG files). Reference runs: BF16 as released, in float32 with TF32 off (--fp32), and on the first set with the LoRA merged (--merge) and with rows one at a time (MSO_BRANCH_CHUNK=1). Transformers 5.17.0, torch 2.14.0, PEFT 0.19.1, no fla-core.
• Latency on a 1600×900 still: the worker over HTTP with 1, 8 and 16 closed-loop clients, and the reference in one stream, for bench/speed_bench.py's first 1, 3, 6 and 12 questions and Choices of 32 and 255 options.

Test Result

CPU. All checks pass on macOS. PNG decoding and the pixel values equal the reference's bit for bit for all eight PNG images. For the JPEG, the image crate's decoder differs from Pillow's libjpeg-turbo by rounding (4,852 of 18,432 values, by at most 3); given Pillow's values, the pixel values are again bit for bit equal.

GPU tests. All pass. The worker holds 9.4 GB.

Comparison. Input-token counts equal the reference's on every answer. The largest probability difference per answer, against the float32 reference as the common baseline:

Run	Median	p90	Max	Decisions unlike the float32 reference
Reference, BF16	0.019	0.074	0.188	2
Worker, PNG	0.014	0.073	0.209	2
Worker, JPEG	0.015	0.071	0.208	3

Where the difference comes from, on the speed_bench set and one still:

• Merging the LoRA into BF16 moves the reference by at most 0.029, and running its rows one at a time by at most 0.019.
• The vision tower carries most of it. In BF16 the worker's and the reference's image features both differ from float32 by about 14% (relative L2: 14.8% and 13.4%), and from each other by 6.2%.
• Given the reference's image features, the worker's zq and u are within 1.1% and 0.7% of the reference's, and its option-text log-probabilities within 0.018.

The decisions that differ from float32:

• web, one_token (float32 margin 0.19): the BF16 reference and both worker runs.
• phone, choice32 (0.03): the BF16 reference only.
• phone, one_token (0.02) and video, noul_region (0.02): the worker's JPEG run.
• video, speed_bench question 9 (0.18): the worker's PNG run, where it gives continue 0.539 against the float32 reference's stop 0.585 and continue 0.409; in the JPEG run, whose pixels differ by at most 3, it gives stop 0.595.

So the worker is about as far from float32 as the BF16 reference is. That meets the first gate Cua-S1's README sets (at most twice the BF16 reference's difference plus 0.01), but not the 0.01 the RFC proposed, which the BF16 reference doesn't meet either. Cua-S1's second gate (the same top option wherever the float32 margin is at least 0.05) fails once for the BF16 reference and once more for the worker's PNG run. I'd take the comparison against float32 as the gate for OmniJev too, and look at the vision tower's BF16 rounding separately; I'm happy to follow a different gate if you prefer.

Latency. The worker reruns the prefix for every row; the reference reuses it:

Workload	Worker, one client	Worker, requests per second	Reference, one stream
1 question	0.24 s	4.1	0.17 s
3 questions	0.45 s	2.2	0.18 s
6 questions	1.02 s	0.98	0.22 s
12 questions	2.01 s	0.50	0.36 s
32 options	1.90 s	0.53	0.25 s
255 options	14.9 s	0.067	1.10 s

Medians. With 8 and 16 clients the worker's throughput is unchanged and requests wait in turn: 16.0 s and 32.1 s for 12 questions, and 119 s for 255 options with 8 clients, past the frontend's 60-second default. The next PR reuses the prefix within the request.

Demo / evidence

Every answer behind the tables, the GPU test logs, the latency runs, the scripts and the local CI log: omnijev-129-evidence.tgz (SHA-256 55a63ce4282a8acdf430e505df0d8d35f662bc06ba42bc5d91912434bd4433ea).

Self-review

• I have reviewed the full diff and addressed the issues I found.
• I have checked that the change follows the project's architecture and stays focused on the stated purpose.
• I have run the checks appropriate to this change and reported commands, results, and anything I could not verify above.
• I have checked that the PR description, documentation, and any accuracy or performance claims match the implementation and available evidence.

[hsliuustc0106]

Independent review — PR #129 "omnijev: add the native eager worker" (DRAFT)

Reviewer: independent ZCode review agent. Not posted to GitHub; no repo state modified.

0. Provenance and pinned revisions

• Skill: .agents/skills/system1-omni-review/SKILL.md, read from the working tree at
  /Users/hsliu/thinkflowlab/system1-omni/. SHA-256 of bytes read:
  58be3bc60806c4c5af7038a0c6a80fadb94f3360d714556a3e965c8903dde108 (matches the pinned value;
  identical to the copy at main@4a79980). Repository ThinkFlowLab/system1-omni (ID 1386454853).
• References actually read (content hash verified):
  • docs/architecture.md — byte-identical to main@4a79980 (526e0f7a…f091f6).
  • .agents/skills/system1-omni-review/references/repository-contracts.md — working-tree copy,
    byte-identical to main@4a79980 (d6ae9650…f1202).
  • src/runtime/README.md — working-tree copy, byte-identical to main@4a79980 (1bda45b0…a44bf).
  • CONTRIBUTING.md — read at the PR head (git show fc27b75:CONTRIBUTING.md, includes the
    large-code-change section).
• Head re-check (skill requirement): the review request pinned head 063e3b508ac1…, which does
  not exist in the local clone or on the remote. The live PR head is
  fc27b756fcc91f030908c67efbaf67b7e6d71647 (author twu3202, branch omnijev-worker), which is
  what refs/remotes/remote-pr/129 resolves to and what gh pr view reports (headRefOid,
  re-checked at the end of the review). This review covers fc27b75. The PR's actual base on GitHub
  is main@7cbce19f117f… (merge of PR #120), not origin/main@4a79980 (the local origin/main ref
  is stale). PR state: OPEN, DRAFT, mergeable: MERGEABLE.
• Diff scopes (all personally computed):
  • git diff 4a79980..fc27b75 (stale merge-base): 160 files, +14,994/−866. Inflated: it contains
    the already-reviewed #120 content plus other branches merged into the PR branch.
  • git diff 7cbce19..fc27b75 (GitHub "Files changed"): 56 files, +4,675/−1,114. Most of this
    is inherited from #98/#99 (fixed-algorithm GEMMs, run_window, forward_shared), #117
    (shared vision tower, vision.cu, image_preprocess) and #108 (prefix tests) merged into the
    branch; the author discloses exactly this in the PR description.
  • Authored change = commit fc27b75 alone: 33 files, +2,274/−267 (verified with
    git show --shortstat / --numstat). This is the scope reviewed in depth.
• Host constraints honored: no Rust toolchain (no cargo commands run), no GPU. Rust source was read
  directly at the head commit. Python available but the only Python artifact (validate.py) needs
  the pinned reference checkout and a GPU; it was read, not run.

1. Verdict

No P0/P1/P2 findings. 1 P3 finding. 7 notes. The authored change is a faithful, well-bounded
implementation of the eager native worker for OmniJev-4B v1.1 within the repository's contracts.
The evidence base, however, is thin outside author-reported results: CI has produced zero check
runs on this branch, and all GPU/parity numbers are author-reported from one sm_89 GPU. This is
reported as an evidence gap, not a PASS. Draft state is appropriate; the author's own proposed
numerical gate (float32-reference comparison, not Cua-S1's two-gate policy) is the right contract
for a score/noul model and is honestly stated, including one decision that flips between the
worker's PNG and JPEG runs at a 0.18 float32 margin.

2. Findings

P3 — Worker admission slot is released on client disconnect while the request still runs

• File: src/models/omnijev/native/src/main.rs:40-50 (Admission guard) and :62-67, :96-104
  (decide), at head fc27b75.
• Trigger: a client disconnects after its request has been dispatched into
  shared.scheduler.run(...), or while it is preparing. Axum drops the handler future; Admission's
  Drop decrements shared.admitted immediately, but the owned closure (and the SerialScheduler
  permit, and the executor mutex) continue to run to completion on the blocking pool, as the runtime
  contract requires.
• Consequence: the 503 budget (ADMITTED = 16, documented in code as "preparing, waiting or
  running" and in recipe/omnijev/native.md as "holds at most 16, preparing, waiting or running")
  undercounts still-executing work after disconnects, so more requests can be admitted than the
  comment/recipe semantics imply. Execution stays correctly serialized (the scheduler permit is
  retained per src/runtime/README.md), and responses to disconnected clients are discarded — this
  is an accounting/documentation nuance, not a correctness or lifetime violation.
• Suggested fix (optional): move the Admission guard into the scheduler closure (it is
  Arc-based, so it can be cloned in) so the slot is held until execution completes, or reword the
  comment/recipe.

Severity rationale: worker-level overload accounting only; the contract-binding scheduler permit
and resource lifetimes are correct.

Notes (non-blocking)

1. main.rs:73 — acquire_owned().await's Err (semaphore closed) would proceed without a
   preparing permit. The semaphore is never closed, so the path is dead; noting for awareness only.
2. executor.rs:96-104 — per-question latency_s is the request total divided by the question
   count, not a per-question measured time. Disclosed in src/models/omnijev/README.md
   ("which time the request's preparation and its vision and language passes"); informational fields.
3. executor.rs:97 — the question state zq is taken from rows[0]; correct because every row's
   pass shares the identical prefix+head and the fixed-algorithm GEMMs are M-independent (the
   invariance property the PR tests on GPU). Relies on the author-reported GPU invariance result.
4. export.rs:96-116 — every worker start SHA-256s all listed export files (multi-GB shards).
   Deliberate pinned-identity choice; startup cost only.
5. docs/supported-models.md:22 — "for 84 answers" (7 stills × 12 speed_bench questions) vs
   native.md's 140 (adds the 8 gate questions). Both accurate for their subsets; the row cites the
   smaller one.
6. The GitHub diff contains ~3,100 added lines authored in #98/#99/#117/#108 (not yet merged to
   main). Those were not re-reviewed here beyond their interaction with the authored code; they are
   the reason the PR must stay draft until they merge, which the author states.
7. The strict json::parse runs twice per request (once in decide for the 400 mapping, once
   inside contract::compile). Harmless; keeps the error-status contract clean.

3. Contract-compliance walkthrough (authored code)

• Ownership layers (docs/architecture.md): transport is a plain Axum worker
  (main.rs); processing stays model-specific (processing.rs, contract unchanged from #120);
  scheduling reuses omni_runtime::SerialScheduler (serial admission, one complete request per
  permit — the Open-Jev precedent); the model executor (executor.rs) owns vision + language +
  CPU heads; no CUDA/C++ is authored here — the shared crate and src/backends/cuda/qwen3_5/
  changes are inherited from #98/#99/#117. Rust owns host orchestration; device ops stay in the
  backend. Compliant.
• Chain fidelity (serialization → prompt/template → token IDs → prefill → scoring → response):
  prompt() retains the <think> suffix and Jinja-style trailing-whitespace trim;
  layout() computes the questions' common token prefix cut at least one token before any first
  <|opt|> (opens[0] >= 2 enforced, prefix.max(1) safe because every row pass is exactly the
  original single-question sequence truncated at an option close); each row pass
  (executor.rs:120-160) is a full unpadded forward over prefix+row with per-row M-RoPE positions
  from the shared image_positions (verified against Transformers 5.17.0 get_rope_index
  semantics and pinned by positions_sha256 fixtures from the reference); readouts are zq at the
  token before <|opt|>, u at <|/opt|>, option-text log-probabilities with the first token also
  predicted from zq; Score rows skip the LM readout and use the ordinal head — no Cua-S1 26-letter
  or A–Z choice semantics are imposed anywhere. Reconstruction zips answers back by question id
  (duplicate ids impossible: the strict parser rejects duplicate JSON keys), whole-request failure
  preserved (any error → single 500, no partial answers), usage reports the reference's token count
  (prefix once + rows), padding adds nothing. Compliant.
• Pinned identity: export.rs pins format omnijev-merged/1, checkpoint revision
  ffe5f436…, base revision 851bf6e8… (matches the repository-contracts base pin
  851bf6e806ef…), vocabulary 248,079, option-token ids, max_pixels 602,112, head config, and
  calibration (positive finite temperatures, finite noul bias); it hashes every listed file,
  requires the manifest to cover every weight_map shard, rejects path-traversal names and an
  unlisted model.safetensors. Executor::load additionally checks
  cfg.image_token_id == IMAGE_TOKEN and cfg.hidden == heads::HIDDEN, and Processor::load
  pins the three special tokens' ids in tokenizer.json. Strong chain.
• Eager execution path / lifetimes: forward_multimodal_readout (model.rs:971-1074)
  validates the whole MultimodalInput boundary before allocation; scratch is grown by the
  existing prepare_scratch; the vocabulary pass reuses persistent vocab_buffers
  (device picked/logits + host mirror, sized VOCAB_ROWS(64) × row, ld = vocab next_multiple_of(8), all copies/GEMM/downloads bounds-checked against m ≤ 64, rows < t,
  token < vocab); on any error the stream is synchronized before returning ("Work already queued
  may still use the buffers"), and Executor::execute (executor.rs:63-72) synchronizes both
  vision and language even on the error path before releasing admission — exactly the runtime
  contract's "executors must finish queued device work before returning". FFI integer args are
  far inside i32 range; buffer arithmetic uses checked_mul/next_multiple_of; Model::drop
  destroys the fixed GEMM handle only when non-null. No sizes/overflow issues found on the FFI
  boundary touched by the authored code.
• Cancellation / admission: waiting callers are removed without dispatch and dispatched closures
  retain permit+resources until completion (owned by the already-merged omni-runtime, unchanged
  here); the worker adds preparation concurrency 2, a 16-slot worker cap with 503+Retry-After,
  and admission counting that is race-safe (fetch_add/fetch_sub pairing, RAII release). One P3
  accounting nuance above. Compliant with the runtime contract; the execution unit is one complete
  request, consistent with the Open-Jev row of the runtime table.
• Tests: all new tests live in the root tests/ tree with explicit [[test]] registrations
  (CONTRIBUTING rule). CPU tests are fixture-based and reference-pinned (PNG decode bit-exact vs
  Pillow, preprocessing bit-exact given Pillow's pixels, JPEG bounded at ±3 with the processor
  re-verified bit-exact on Pillow's pixels, export manifest negative cases with Drop cleanup,
  token_logprob log-softmax-over-vocabulary-only including padding exclusion). GPU tests
  (readout.rs, worker.rs) are #[ignore]-gated on env-pinned checkpoint/library and check
  bit-equality with forward_fixed, M-independence over 130 targets/two vocabulary chunks, a float64
  CPU log-prob within one BF16 ULP + 1e-3, and answer repeatability/order-independence.

4. Changed-file inventory and 3k-rule readiness

• Authored (fc27b75): 33 files, +2,274/−267 (2,541 changed lines total).
  Code (src + tests + validation scripts, excluding docs/lockfile/static fixtures per
  CONTRIBUTING): my count 2,148 (author reports 2,047 — same order, both below 3,000).
  Excluded from my code count: README.md, docs/supported-models.md, mkdocs.yml,
  recipe/README.md, recipe/omnijev/README.md, recipe/omnijev/native.md (123),
  recipe/omnijev/gate_questions.json (242), tests/omnijev/data/pixels.json (1-line/48 KB
  generated fixture), Cargo.lock.
• Total GitHub diff (7cbce19..fc27b75): 56 files, +4,675/−1,114 (5,789 lines), of which the
  authored commit is ~44% of additions. Total stale merge-base diff (4a79980..fc27b75):
  160 files, +14,994/−866 — not the operative scope.
• 3k rule: not triggered on authored code (≈2.1–2.5k lines). Regardless, the PR description
  carries the large-change preparation anyway: full self-review (all four boxes ticked by the
  author, not touched by this review), split rationale (#120/#98/#99/#117 dependencies, draft until
  merged), a component map (shared crate vs omnijev sections), and per-area validation (CPU checks,
  GPU kernel/readout/worker tests, 140-answer reference comparison, latency table, version/hardware
  pins: RTX 6000 Ada sm_89, CUDA 13.2, driver 595.91.07, Transformers 5.17.0/torch 2.14.0/PEFT
  0.19.1). Interfaces between components (shared Model, VisionConfig, moved image_decode) are
  covered. Readiness: adequate.
• Authored-file list (all at fc27b75): src/models/omnijev/native/src/{main.rs (new, 181), executor.rs (new, 178), export.rs (new, 150), processing.rs (+75/−70), contract.rs (+2/−2), lib.rs (+3/−1), Cargo.toml (+22/−3)}; src/models/qwen3_5/native/src/{model.rs (+143/−2), inputs.rs (+43), image_decode.rs (new, 134, moved from Cua-S1), lib.rs (+3/−2), vision/mod.rs (+2/−1), Cargo.toml (+5)}; src/models/cua_s1/native/src/{image_request.rs (+13/−90), multimodal.rs (+5/−42)} (mechanical moves, limits preserved exactly:
  2048/1,048,576/200/32 MiB); docs/recipes as listed above; tests
  tests/omnijev/{export.rs (new 180), pixels.rs (new 135), worker.rs (new 93), processing.rs (+4/−3), generate_fixtures.py (+57/−2), data/pixels.json (new)},
  tests/qwen3_5/{readout.rs (new 187), model.rs (+34)}.

5. Committed artifact hygiene (complete inventory)

• GitHub-diff inventory (56 files): 26 .rs, 9 docs (.md), 8 .json, 6 .toml/lock, 3 .py,
  plus 4 inherited backend files (.cu/.h) and doc navigation. No binaries, no logs, no
  benchmark output, no editor droppings, no generated data without a committed generator.
• tests/omnijev/data/pixels.json (48 KB, single line): generated by the maintained
  tests/omnijev/generate_fixtures.py (extended in this PR); pattern images are redrawn in test
  code and only encoding-sensitive files (palette, 16-bit gray, JPEG) are stored. This is a
  maintained fixture, not one-off run output — acceptable, keep.
• recipe/omnijev/gate_questions.json (242 lines): maintained validation question set consumed by
  validate.py --questions — maintained corpus, acceptable.
• Stale merge-base-only artifacts (tests/jev_vl/data/replay/image.png, fail_graph_capture.cpp,
  LICENSE.omnijev, etc.) belong to other branches' merges, not to this PR's authored content; no
  hygiene action.

6. Commands run and results

Command	Result	Class
shasum -a 256 .agents/skills/system1-omni-review/SKILL.md	58be3bc6…dde108 (matches pin)	personally run
git log / git rev-parse refs/remotes/remote-pr/129 / git cat-file -t 063e3b50…	head = fc27b75…; pinned 063e3b50… does not exist	personally run
gh pr view 129 (twice)	OPEN, draft, author twu3202, base main@7cbce19, head fc27b75, MERGEABLE; body quoted in this review	observed (live API)
git diff --stat 4a79980..fc27b75; git diff --stat 7cbce19..fc27b75; git show --shortstat fc27b75	160 files +14,994/−866; 56 files +4,675/−1,114; 33 files +2,274/−267	personally run
Full reads at head: main.rs, executor.rs, processing.rs, export.rs, contract.rs (grep), image_decode.rs, inputs.rs, model.rs (readout + hidden_row + run_window signature), vision/mod.rs, Cua-S1 move diffs, tests/omnijev/{worker,export,pixels,processing}.rs, tests/qwen3_5/{readout,model}.rs, generate_fixtures.py diff, validate.py, native.md, README/mkdocs/supported-models diffs, .github/workflows/{ci,docs}.yml	no P0–P2 issues found	source inspected
cargo fmt/clippy/test/build	not run — host has no Rust toolchain (constraint)	not run
mkdocs build --strict	not run (docs changes are present; author reports strict pass)	not run
GPU tests (tests/qwen3_5/readout.rs, tests/omnijev/worker.rs, kernel tests)	not run — no GPU (constraint); author-reported pass on one RTX 6000 Ada	author-reported
validate.py 140-answer comparison + latency table	not run — needs pinned reference checkout + GPU	author-reported

7. CI observed

• gh pr checks 129 --repo ThinkFlowLab/system1-omni →
  "no checks reported on the 'omnijev-worker' branch" (exit 1). Retry produced the same result.
• Corroborated via API on the head SHA: …/commits/fc27b75…/check-runs → {"runs":[],"total_count":0};
  …/status → {"state":"pending","statuses":[]}; gh run list --branch omnijev-worker → empty.
• So there are no CI check states at all to report — including no action_required entries.
  Both workflows (ci.yml, docs.yml) trigger on pull_request (drafts included), so the
  absence means CI never started for this head (likely timing — the commit is dated today — or a
  fork-run approval). Whatever the cause, no CI evidence exists for this PR; the five Rust steps
  and mkdocs --strict are author-reported (macOS) only.

8. Material unverified checks

1. All five CI Rust checks and mkdocs build --strict — author-reported on macOS; no observed
   CI run of any kind (Section 7). The Cargo.toml/Cargo.lock changes (axum/tokio/sha2/image
   additions, [[test]] registrations) are compile-verified only author-side.
2. All GPU behavior — the fixed-GEMM M-independence that the whole eager design leans on
   (rows[0].zq reuse, chunked vocabulary logits, bit-equality with forward_fixed), the kernel
   tests, and the worker repeat/order tests: author-reported on a single sm_89 GPU. Not observed by
   CI or this review; no other GPU arch covered.
3. Numerical parity vs the reference (140 answers, float32 baseline, JPEG/PNG split,
   LoRA-merge and branch-chunk isolations; latency table; 9.4 GB memory): author-reported via
   validate.py; raw outputs are "kept locally" and not attached — the numbers are plausible and
   honestly caveated (one PNG-run decision flips at float32 margin 0.18; the proposed float32 gate
   is a contract decision for maintainers, and the author explicitly invites a different gate), but
   they are not independently reproduced.
4. warmup/readiness over HTTP and the frontend-proxied path — no transport regression test for
   the new worker's HTTP surface (status mapping incl. 413 rejection body, health after warmup,
   503 behavior); the worker tests call the library directly. The recipe's
   OMNI_JEV_BACKEND_URL routing guidance is unverified here.
5. Rebase-on-merge caveat — the tests were run, per the PR body, "on the same tree before the
   rebase onto #120's merge"; the head tree itself has not been revalidated by anyone.
6. Inherited #98/#99/#117/#108 content inside the GitHub diff (~3,100 added lines incl. CUDA) is
   covered by their own PRs, not re-reviewed here.

9. Bottom line

Static review of the authored commit finds the worker faithful to the omnijev contract and the
architecture/runtime/lifetime contracts, with careful pinned-identity, bounds and
error-synchronization handling; one P3 admission-accounting nuance. The blocking gap before this
draft can leave draft state is evidence, not code: no CI run exists for the head, and every GPU and
parity claim is author-reported on one GPU with locally-held raw outputs. Recommended next steps for
the author: get CI to run on the head SHA (or report the exact local commands + full output),
attach the validate.py outputs, and decide the numerical gate question the PR honestly raises.

[twu3202]

Thanks for the review. Pushed 5cf7c4e:

• The admission slot now moves into the preparation and then into the scheduled work, so it's held until the request's work ends, also after the client goes away. Requests still waiting in the scheduler are dropped without running, as before, and free their slot.
• The supported-models row cites the 140 answers.

On the evidence:

• CI: no pull_request workflow has started in the repository since 10-09 14:45Z (the push of 7cbce19 to main didn't start one either), so nothing ran for this branch. The archive below has the full output of the same five steps and mkdocs build --strict on 5cf7c4e, from a clean target directory (Rust 1.99.0, macOS arm64).
• The head tree is the tested one: the rebase onto 7cbce19 only dropped omnijev: add the request contract, preparation, heads and export #120's merge, and fc27b75's tree is the one the GPU runs used. The worker test also passes on 5cf7c4e.
• HTTP, on the GPU host: health after the warmup request, 400 for malformed JSON and for duplicate keys, 413 above 12 MiB, 422 for a refused request, and 24 simultaneous requests giving 16 answers and 8 503s with Retry-After: 1.
• Archive with every answer behind the tables, the GPU test logs, the latency runs, the scripts and the local CI log: omnijev-129-evidence.tgz (SHA-256 55a63ce4282a8acdf430e505df0d8d35f662bc06ba42bc5d91912434bd4433ea).

I'll follow whichever gate you choose and rerun against it.

[hsliuustc0106]

Incremental re-review at new head 5cf7c4ed27: one new commit since my review at fc27b756fc — "omnijev: hold the admission slot until the request's work ends" (2 files, +16/−8).

This fixes the P3 from my review, verified in the diff: the Admission guard is now named and moves through the preparation spawn_blocking closure into the scheduler.run closure, so a client disconnect that drops the handler future no longer releases the slot while preparation or executor work continues — the slot is held until the request's work actually ends, and the struct doc says so. The previously-dead semaphore-close path now returns a typed 500 via let-else. Also addressed from my notes: docs/supported-models.md now cites the full 140-answer comparison instead of the 84-answer subset.

Unverified (unchanged): Rust behavior on this host (no toolchain) and all GPU/parity evidence remain author-reported; no CI has reported on this head. Verdict: COMMENTED (draft; zero actionable findings at this head).

[hsliuustc0106]

Incremental re-check at new head a8f523ab04: the branch absorbed main's advancement (merges of #107, #115, #71, #34, #29, #32/#77 content — all patch-identical in range-diff against my reviewed head 5cf7c4ed27) and adds two small commits, both reviewed:

• d8cd04f: declares abi_version: 7 in qwen3_5.backend.json — verified consistent with ops.h's CS1_ABI_VERSION 7 at this head, and the repo's cuda-contract checker (which validates exactly this) passes in CI at the exact head.
• a8f523a: lists src/models/omnijev/native/ in the manifest's models consumers — the crate exists at that path; this is the #120-promised registration completed on the worker side.

Zero actionable findings at this head; prior reviews' zero-finding status carries (own commits patch-identical). Still a draft, so no approval. Unverified (unchanged): GPU behavior and parity evidence author-reported; no CI-reported GPU execution; no Rust toolchain on this host (fork CI rust/build/benchmarks/cuda-contract green at this exact head).

[hsliuustc0106]

Incremental re-check at new head dac6ad6702 after the rebase wave (prior review: zero findings at a8f523ab04). 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 patch-identical; resolutions clean (Cargo.lock union; the native_vision_graph recipe and model.rs folds take this branch's reviewed versions). Zero actionable findings; still a draft, so no approval. Template state (post-#138 answers/checklist) still to be completed before leaving draft.

[hsliuustc0106]

Incremental re-check at new head b220f476b5 after the rebase/merge wave (prior review: zero findings at dac6ad6702). All commits patch-identical (git range-diff, named refs, cat-file verified); the same three merge resolutions as #134 were checked — manifest abi_version: 7 matches ops.h, omnijev models entry retained (plus main's new decider entry), supported-models table intact. Fork CI rust/build/benchmarks/cuda-contract green at this exact head. Zero actionable findings; still a draft, so no approval. Template state (post-#138 answers/checklist) still to be completed before leaving draft.