[MrDongsls]

Stacked on #151 + #174 — incremental review: e7643f3a plus the infermatrix/main merge 9576f78d.

Required human contributor answers

1. What outcome are you trying to achieve?

   Start the native Rust path for the 0.8B: export the checkpoint into a verifiable bundle, implement the wire admission, and mirror the token-level processing — checked against frozen token-plan fixtures recorded from the pinned runtime. (Source: issue [New Model]: Support the Vela 2.0 series #137.)

2. What is the highest-leverage variable?

   Exact token-level parity with the pinned runtime is the foundation every later stage validates against.

3. How will this work move that variable?

   Five frozen fixtures (including a windowed long-span and a CJK case) lock the predict mapping and rendered rows field-for-field; the opt-in render test replays them through the Rust code.

4. Who is the customer or user?

   Contributors building the native worker; ultimately users needing GPU-accelerated Vela decisions.

Is a PR needed? Yes — prerequisite for the executor PR.

For a bug fix: N/A.

Purpose

Export script (export_native.py: gate-verified, per-tensor dtypes preserved, manifest SHA-256), Rust bundle gate + wire admission (byte-equal 422/400), token-level processing (state/questions/words/render/processing), five frozen token-plan fixtures, parity example binary.

Test Plan

System1-Omni Version / Commit: model/vela2-native-processing @ 9576f78d

Check	Command	Result
CPU tests	cargo test -p omni-vela-native	31 passed; 1 ignored
clippy	cargo clippy -p omni-vela-native --all-targets -- -D warnings	clean
Docs	mkdocs build --strict	exit 0
Opt-in render parity	VELA_NATIVE_BUNDLE=… VELA_NATIVE_MANIFEST_SHA256=… cargo test --test vela_native_processing -- --ignored	5/5 field-for-field

Test Result

All passed. Render parity is opt-in (needs the exported bundle's tokenizer); CI covers CPU tests only.

• Current head 9576f78d; infermatrix/main merged. Every check in the table plus PYTHONPATH=src pytest tests/vela (137 passed), cargo fmt --all --check and strict mkdocs rerun and green at this head.

Demo / evidence

Author-run, x86 WSL2, CPU FP32:

• Opt-in render parity: all five token plans reproduce field-for-field (92s), including CJK and the windowed long-span (7,713 tokens).
• example-request totals 1,041 tokens matching the oracle's usage.input_tokens.
• Manifest file digests cross-checked against the package's own SHA256SUMS.
• All test bodies at repository root; private-item test uses #[path] wiring.

Not measured: GPU, performance.

Self-review

• Full diff reviewed.
• Architecture and scope checked.
• Checks run and reported above.
• Description and claims match implementation.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-25T06:03:12.359580Z We couldn’t complete this review. Comment @codex review to try again.	7391de2	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[hsliuustc0106]

Draft review — PR #175 (first-pass review; not posted)

PR: #175 "vela: add the 0.8B native bundle export, wire contract and token-level processing"
Reviewed head: 0f1e4da59101739a354a54fb3131e4972748daa6 (matches the PR's head.sha on the API; non-draft, open, base main)
Own footprint: 30 files, +3352/−91 vs shared merge-base 78fe14378 (10 first-parent commits; main itself has moved to e573260c, so the vs-main diff's negative deletions are main's newer content, not this PR's).

Stack structure

Verified: git merge-base --is-ancestor confirms a strict linear stack — 78fe14378 ⊂ #175 (0f1e4da5) ⊂ #176 (c2d704d7) ⊂ #177 (064244db) ⊂ #195 (71c18940). This PR is the base slice; deep-dive target.

• First-parent chain: 7 regular commits + 3 merges (57994cba upstream/main, ddaecd8f hybrid-reference-serving, 1685b5bb the re-merge keeping "Vela native rows alongside the Clef/pplx-decider additions").
• Merge hygiene: re-ran every merge with git merge-tree --write-tree and diffed against the committed tree. All three merges carry real conflicts (workspace Cargo.toml members, README model rows, Cargo.lock); the committed resolutions are proper unions (e.g. 1685b5bb keeps both the Vela native rows and upstream's Clef/pplx-decider rows; member list keeps vela/native and startlux/native). No evil-merge content found.
• This branch carries its own duplicate of the token-level work built in parallel (9812853b on the heads line has the same title as 7391de2a here); the final tree keeps this slice's canonical files (the later slices' diffs show no processing.rs/render.rs changes between heads), so the duplication was resolved to a single source. Watch-item for the rest of the stack: satisfied here.

Export contract

• recipe/vela/export_native.py:70 — verify_package(package) runs the worker's own gate (10-file SHA-256 pin table in src/models/vela/hybrid.py:54, exact shard-size check, config identity) before any parse. Inputs are safetensors (load_file) + json — no pickle surface, so weights_only is moot. Good.
• Manifest: per-output files_sha256 for all six bundle files plus source pin table, revision, tensor inventory (320/204/116 backbone, 37 heads). Good.
• Rust side (src/models/vela/native/src/bundle.rs): re-hashes every manifest-listed file, pins format/model_id/revision/inventory, rejects an empty pin table, and fails closed on unpinned directory entries (9f13566b). Synthetic (weight-free) gate tests in tests/vela/native_bundle.rs run in CI via cargo test --workspace.
• CI-wiring of the parity corpus: the 5 native-plan-*.json fixtures are recorded from the pinned runtime itself (export_token_plan.py drives the pinned vela2_inference.Vela2); the mapping tests are always-run, the render replay is #[ignore] opt-in behind VELA_NATIVE_BUNDLE (declared policy, matches the Valen-native #158 pattern).

Wire contract

src/models/vela/native/src/contract.rs mirrors models.vela.protocol: 8 MiB body cap, upstream's {"detail":[{"loc","msg","type"}]} triples, missing/extra/blank-model/wrong-member order matches the reference preamble, duplicate-key and non-object 400 messages verbatim (2de81d56). The rejection tests (tests/vela/native_contract.rs) are hand-mirrored from the reference worker's protocol tests — they are not generated from the pinned reference at test time; the one residual divergence (non-finite JSON number answers the generic 400 instead of naming the value) is documented in the module doc. Acceptable, but note the corpus is a transcription, not a recorded replay like the token-plan fixtures.

Findings

1. P2 — no atomic staging in the exporter. recipe/vela/export_native.py:102-106,144 writes the bundle directly into the final --out directory (mkdir + save_file/write_text, manifest written last). Trigger: a crash/disk-full mid-export leaves a partial bundle in place. Consequence: contained — the Rust gate fails closed on the hash/inventory mismatch — but this deviates from the Kev export precedent (#170: mkdtemp + atomic rename) that this series is supposed to follow. Suggest staging into a temp dir and renaming the finished bundle.
2. P3 — fixture recorders do not hash-pin their input. recipe/vela/export_token_plan.py:129 (export_head_inputs.py likewise) loads the package via the pinned runtime's own Vela2.from_pretrained without calling verify_package first, unlike the bundle exporter. Trigger: recording fixtures against a wrong/modified package silently freezes wrong parity targets. (Fixed in the stack: #195's exporters call member = verify_package(package).)
3. P3 — bundle-directory walk is single-level while the comment claims "at any depth". src/models/vela/native/src/bundle.rs:73 vs :79 (std::fs::read_dir, non-recursive). Any subdirectory is caught by name today, so enforcement holds in practice; #195 later makes the walk genuinely recursive. Also bundle.rs:83 entry.expect("a readable bundle entry") panics instead of erroring on an unreadable entry.

Notes (informational)

• The bf16-out branch re-saves every backbone tensor to bf16 "once, at export" — documented, and the manifest inventories count both dtypes.
• 27 always-run tests, 1 ignored at this head — matches the body's claim; Cargo.toml workspace membership means CI runs them (cargo test --workspace --locked).
• Registration consistent at this head: README row "bundle export, wire contract, token-level processing", supported-models row, mkdocs.yml nav (native.md), recipe/README pointers all present and mutually consistent.

Body / template / evidence

• Template answers: present (4 questions + is-a-PR-needed + N/A bug-fix). Checklist: all four boxes checked.
• Head pin: model/vela2-native-processing @ 0f1e4da — correct, pins its own head.
• Evidence: author-run CPU FP32 render parity 5/5 (92 s), token totals cross-checked against the oracle's usage.input_tokens, manifest digests cross-checked against the package's SHA256SUMS; "Not measured: GPU, performance" honestly stated. "Review the last two commits only" is a review-state note; the full 10-commit chain was inspected here.
• CI observed at the exact head 0f1e4da5 (Actions API, ThinkFlowLab/system1-omni): workflow runs CI success (id 40277592873) and Docs success (id 40277592845); check-runs benchmarks/build/cuda-compile/cuda-contract/rust all success, deploy skipped. (Companion repo ThinkFlowLab/system1-agents has no runs for these SHAs — nothing expected there.) The combined commit status reads "pending" with zero status contexts — a stale-commit-status quirk, not a failing check.
• Local (this review, python 3.x miniconda — only applicable pieces): n/a here; run at #195 head, see that review.

Verdict: COMMENTED

Non-draft, template complete, CI green — but two process-contract findings (P2 atomic staging, P3 recorder pinning) against the precedents bar, plus the depth-of-walk nit. None block the stack's direction; all are addressed or addressable in the follow-up slices (the recorder pinning and walk depth are in fact fixed by #195).

[MrDongsls]

Addressed in b9a60c0. The exporter now stages the bundle set and the optional bf16 backbone in sibling directories and moves each into place with one rename, following the valen export precedent. A non-empty target is refused and a stale staging directory from a killed run is cleared by the next run. native.md records the behavior next to the export command. The recorder pinning and walk depth items were already fixed by the later slices as the review notes.

[hsliuustc0106]

Pass-107 fix-delta review — #175 (vela: 0.8B native bundle export, wire contract, token-level processing)

Head reviewed: b9a60c0e4 (model/vela2-native-processing) — pushed after pass-102's review of 0f1e4da59. OPEN, non-draft. Base: main.

Delta reviewed this pass

One authored commit, exactly as declared:

• b9a60c0e "vela: stage the native export atomically" — 2 files, +52/−21 (recipe/vela/export_native.py, recipe/vela/native.md), on top of the pass-102 head 0f1e4da5. First-parent chain verified: no other commits, no merges.

Stack integrity

git merge-base --is-ancestor confirms b9a60c0e ⊂ #176's head 86f689850 ⊂ #177's head 65766b834 ⊆ #195's head 7b60e4ec1. The series is still a clean linear stack at the new heads.

Prior findings — status at this head

Pass-102 finding	Status at b9a60c0e4
P2 — export_native.py wrote the bundle in place (no mkdtemp+rename atomic staging)	FIXED. New stage_into_place (recipe/vela/export_native.py:56-77): stages in a sibling <target>.export-partial directory, refuses a non-empty target, clears a stale staging remnant from a killed run, then moves the complete set into place with one os.replace; except BaseException: shutil.rmtree(staging); raise removes the partial on any failure including Ctrl-C. Applied to both the main bundle and the optional --bf16-out set. The manifest's files_sha256 hashing moved inside the staged write, so the pins hash exactly the staged bytes; written bytes unchanged. native.md:30-34 documents the behavior. This matches the one-rename discipline of the earlier valen-export precedent.
P3 — fixture recorders didn't run verify_package	STILL OPEN at this head. recipe/vela/export_token_plan.py still has no verify_package call (the only recipe-side callers at this head are export_native.py:89-94). The fix exists only at the top of the stack: #195's 71c18940 adds verify_package to both recorders (recipe/vela/export_token_plan.py:127-133, recipe/vela/export_head_inputs.py:169-175), which are not in this PR's lineage.
P3 — single-level bundle walk despite the "at any depth" gate comment	STILL OPEN at this head. src/models/vela/native/src/bundle.rs:79-91 still walks only the top level (and still .expect("a readable bundle entry") on unreadable entries). Fixed in #177 (65766b83, one level inside admitted dirs) and fully recursive in #195 (8d994e1d's walk_unpinned); neither is in this PR's lineage.

New findings

P3 — the PR body was not updated for the fix head; the template's head pin is stale. Test Plan still reads "System1-Omni Version / Commit: model/vela2-native-processing @ 0f1e4da", and the check table (e.g. cargo test -p omni-vela-native "27 passed; 1 ignored") describes the pass-102 head. Nothing in the body mentions b9a60c0e, the atomic staging, or the author-reported checks at this head (PYTHONPATH=src pytest tests/vela 135 passed) — those live only in the commit message. Trigger: a reviewer reads the body and reviews 0f1e4da evidence for a b9a60c0e head. Consequence: the template's evidence field does not cover the commit under review; the same round added "Head updated to 7b60e4e …" to the #195 body, so the convention exists and was applied inconsistently. Fix: add a head-update line pinning b9a60c0e with the commit's reported checks.

Verification (sources kept separate)

• Source inspected: full git show b9a60c0e4; bundle.rs walk and export_token_plan.py at this head; 71c18940's recorder fix and 65766b83/8d994e1d walk fixes on the upper branches (to locate where the open P3s are actually resolved).
• Commands run: git fetch + rev-parse (head matches b9a60c0e4cb…); git merge-base --is-ancestor ×3; git log --first-parent; git grep for verify_package / stage_into_place at this head (3 stage_into_place occurrences, zero in-place mkdir(exist_ok=True) writes remain).
• Observed CI: no Actions runs exist for b9a60c0e4 on either ThinkFlowLab/system1-omni or InferMatrixLab/system1-omni (both queried by head_sha). Latest green runs on this branch are at the prior head 0f1e4da59 (CI 40277592873, Docs 40277592845, 2026-10-26, success). Mitigation: this head is an ancestor of #195's 7b60e4ec1, where CI (41173895523) and Docs (41173895796) are green at 2026-10-31T05:46Z — the follow-up commit's content is covered there, but no run ran this commit itself.
• Author-reported (not independently verified): "stage_into_place behavior checks and PYTHONPATH=src pytest tests/vela (135 passed) run at this head" (commit message only).

Template

All four required contributor answers, PR-needed decision, Test Plan, Test Result, Demo/evidence and Self-review checklist are present — but the head pin is stale (finding above).

Verdict: COMMENTED

The P2 is properly fixed — stage_into_place is the right shape and the cleanup paths are correct. COMMENTED because two pass-102 P3s remain open at this head (both are fixed further up the stack, in #177/#195, so merging the series resolves them) and the body evidence was not refreshed to the new head.

[MrDongsls]

Addressed: added a head-update note in Test Result pinning b9a60c0e with the checks recorded in the commit message. No code change.

[psv666]

[P2] Keep the verified package reusable after generating token-plan fixtures.

At export_token_plan.py:126–129, importing vela2_inference and initializing the runtime leaves Python bytecode writes enabled. For a writable package, this creates __pycache__, which the package's verify_files() gate then rejects. The documented export workflow can therefore make a previously accepted package unusable at its next verification/worker startup.

I reproduced this through the actual main() using a tiny correctly hashed Python module, a stub loader and an empty case list (no model or Torch execution): verification succeeds before export; export returns 0 and creates vela2_inference.cpython-312.pyc; subsequent verification raises existing bytecode cache __pycache__/ ... is not verifiable against the pin.

Please disable bytecode writes across both import and initialization, including lazy imports, and add the reuse regression. #195 already carries the corresponding fix; it also needs to land in this prerequisite slice.

The new externally anchored manifest and unsafe-key regressions are present. Personally run: 31 Rust tests passed / 1 ignored, 135 Python tests passed, strict MkDocs passed. The tokenizer/bundle replay was not run. Current GitHub mergeability reports conflicts with main; refresh the cumulative diff/review map and integration checks after resolving them.

Review scope and provenance

Head eec1fe6c3133d44f93efb72510ed2f6bd63419e6; PR API base 05075d0f36e399e3a001ffedea400c666990cda9; diff merge-base 05075d0f36e399e3a001ffedea400c666990cda9. Inspected scope: processing/export and current manifest-gate follow-up, with dependency inventory inspected; this is a focused follow-up rather than a new full review of every inherited line. Complete changed-file inventory: 45 files, 4,160 changed authored-code lines / 5,331 total text lines (binary fixtures counted as files, not text lines).

Loaded trusted InferMatrixLab/system1-omni@5fa5179d57f9267bb0d20162004d8ce26352aa9d:.agents/skills/system1-omni-review/SKILL.md, SHA256 3a4b876162da14eee8db56af86664576aa5755b8f3e80b3072e944018647c3bc, plus architecture, repository contracts, CONTRIBUTING, self-review artifact requirements and CI/Docs workflows. Personally executed checks are distinguished above from author-reported GPU evidence. No GPU inference was run in this review; no approval or readiness transition is implied.

[MrDongsls]

Fixed in e7643f3. The exporter disables bytecode writes across import and initialization, including lazy imports, and restores the previous setting on exit. Added a repeat use regression that drives the real main against a stub package and fails before the fix with the reported cache error. Real main merged. 137 Python and 31 Rust tests pass, strict mkdocs passes.

[Levius-Fubuki]

Independent review and package replay at 9576f78dcd3644f3b9738bd116e40c30ab93c463. Changes required before advancing this slice.

[P2] Preserve Python's numeric canonical JSON in structured state

state.rs:34 serializes scalar values with serde_json::to_string, which differs from the pinned runtime's json.dumps(ensure_ascii=False, sort_keys=True, separators=(',', ':')).

Actual compiled public-API probe:

let value: serde_json::Value =
    serde_json::from_str(r#"{"small":1e-7,"large":1e20}"#).unwrap();
let state = serde_json::json!({"request": value});
let parts = omni_vela_native::state::system_one_state(&state).unwrap();
println!("{}", parts.parts[0].text);

pinned Python: {"large":1e+20,"small":1e-07}
native Rust:   {"large":1e+20,"small":1e-7}

The pinned Vela tokenizer produces 17 versus 16 token IDs for these strings. This changes the model input for an accepted structured state. Please match Python's numeric representation, add a regression covering the actual state-to-parts/tokenization path, and propagate the fix through #176/#177/#195.

Personally run

• PYTHONPATH=src pytest tests/vela: 137 passed.
• cargo test --release --locked --offline -p omni-vela-native: 31 passed, 0 failed, 1 ignored.
• Real pinned-package export and the ignored processing replay: all 5 token-plan cases match the frozen JSON field-for-field, including CJK and the 7,713-token long-span case.
• cargo fmt --all -- --check, strict crate clippy (--all-targets -- -D warnings), and strict MkDocs: passed.

Package: vllm-sr/Vela-2.0-0.8B@326b01d81f61fff7fd4ae570e54b87a49119e490; downloaded files checked against authoritative revision/blob/LFS hashes. The bytecode-cache fix from the previous review is present; I am not reopening it. This slice's checks require no GPU and do not establish native GPU parity.

Scope: base/merge-base 5fa5179d57f9267bb0d20162004d8ce26352aa9d; 31 changed files, 3,335 authored-code lines / 3,700 total changed lines (binary sizes audited separately). CI and Docs were observed successful at this same head.

Advance condition: fix the canonicalization regression and rerun the package/token-plan regressions before moving the stack forward.

Review method: the repository's system1-omni-review skill at trusted main 5fa5179d, SHA-256 3a4b876162da14eee8db56af86664576aa5755b8f3e80b3072e944018647c3bc; contributor self-review/split rationale, component interfaces and complete changed-file inventory checked. Results below are reviewer-run; previous author measurements and green weight-free CI are not substituted for model/GPU replay.

[Levius-Fubuki]

[P2] Match Python numeric canonicalization before rendering state. For {"request":{"small":1e-7,"large":1e20}}, the compiled public state API emits {"large":1e+20,"small":1e-7}, while the pinned Python runtime emits {"large":1e+20,"small":1e-07}. The pinned tokenizer gives 16 vs 17 IDs. Please preserve Python's float representation and cover state→parts→tokens, then propagate the fix down the stack.