Repository navigation
Conversation
Second member of the Vela 2.0 family: a pinned vela2-decoder executor (hybrid.py) with a 10-file gate (shard index and the side-loaded broad head hashed; the 2.02 GB shard by exact size), member-aware response finishing (span_heads with spans, keyed by span question ids), worker member detection from config.json model_type, and the per-member /v1/models entry. compare_reference.py serves either member; the hybrid oracle fixtures export under oracle-hybrid-. Evidence (author-run, x86 WSL2, CPU FP32, no fla — the runtime's CPU path never dispatches to it): nine-case parity vs the pinned upstream server exact-match (four 200s incl. key order, five agreed rejections incl. the 16,384-window 413); 0.3B nine-case regression; live worker smoke with span_heads and per-member thresholds; byte-identical Rust frontend forwarding; 133 CPU contract tests.
- recipe/vela/export_native.py: gate-verified export into a native bundle — 320 backbone tensors re-keyed backbone. -> model. with per-tensor dtypes preserved (204 BF16 Linear/conv1d + 116 F32), the 37 F32 heads incl. the side-loaded broad span head in one file, backbone_config at the config root (the family Config::load shape), and a manifest pinning the source revision, the gate table, the tensor inventory and every bundle file's SHA-256. - src/models/vela/native (omni-vela-native): the bundle manifest gate and the wire admission mirroring protocol.py and the pinned runtime's preamble with byte-equal 422/400 bodies. - recipe/vela/export_token_plan.py and five frozen token plans under tests/vela/fixtures/: the predict mapping, the row decomposition and the rendered records (p_ids, blocks, readout positions, truncation, a windowed long-span case); example-request totals 1,041 tokens, matching the oracle's usage. Checks: cargo test (15 tests, no weights, no GPU), fmt/clippy clean, mkdocs --strict.
The Rust processing of the native path, mirrored from the pinned runtime and checked field-for-field against the five frozen token plans (opt-in render parity with the exported bundle's tokenizer): - state.rs: the state -> typed-parts mapping (role grouping, field ranges, canonical DEV2.0 JSON) with the pinned rejection triples; - questions.rs: the SystemOne -> predict mapping — the validation walk and order, Noul folded into a two-option choice (choice renders [ABS], the reference's abstain default), over resolution, threshold/head carried, the set-label collision check; - words.rs: the GLiNER2/CJK _WORD splitter and the first-token word mapping; - render.rs: the encoder (char-offset tokens, memoized), the six-rule span-head dispatch with the hallucination-alias label replacement, the truncation budget, the DEV2.0 rendering (parts pass, question blocks with option ends and query positions, span layout a/c with word positions) and the window rows (head only, as the reference); - processing.rs: the Processor over the exported bundle. Also: the envelope error locs are flat arrays, matching the reference triples exactly. Checks: cargo test (26 tests) + fmt/clippy clean + opt-in render parity (5/5 fixtures).
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…processing # Conflicts: # Cargo.toml # README.md # docs/supported-models.md # recipe/README.md
…reference-serving # Conflicts: # README.md # docs/supported-models.md # recipe/README.md
…ember docs Keep the Clef and pplx-decider rows from main alongside the Vela 0.8B hybrid additions; the package-gate descriptions now carry the directory-symlink refusal from main.
…ows alongside the Clef/pplx-decider additions
…native-processing # Conflicts: # recipe/README.md
hsliuustc0106
left a comment
There was a problem hiding this comment.
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 (
57994cbaupstream/main,ddaecd8fhybrid-reference-serving,1685b5bbthe re-merge keeping "Vela native rows alongside the Clef/pplx-decider additions"). - Merge hygiene: re-ran every merge with
git merge-tree --write-treeand diffed against the committed tree. All three merges carry real conflicts (workspaceCargo.tomlmembers, README model rows,Cargo.lock); the committed resolutions are proper unions (e.g.1685b5bbkeeps both the Vela native rows and upstream's Clef/pplx-decider rows; member list keepsvela/nativeandstartlux/native). No evil-merge content found. - This branch carries its own duplicate of the token-level work built in parallel (
9812853bon the heads line has the same title as7391de2ahere); the final tree keeps this slice's canonical files (the later slices' diffs show noprocessing.rs/render.rschanges 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 insrc/models/vela/hybrid.py:54, exact shard-size check, config identity) before any parse. Inputs are safetensors (load_file) +json— no pickle surface, soweights_onlyis moot. Good.- Manifest: per-output
files_sha256for 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 intests/vela/native_bundle.rsrun in CI viacargo test --workspace. - CI-wiring of the parity corpus: the 5
native-plan-*.jsonfixtures are recorded from the pinned runtime itself (export_token_plan.pydrives the pinnedvela2_inference.Vela2); the mapping tests are always-run, the render replay is#[ignore]opt-in behindVELA_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
- P2 — no atomic staging in the exporter.
recipe/vela/export_native.py:102-106,144writes the bundle directly into the final--outdirectory (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. - P3 — fixture recorders do not hash-pin their input.
recipe/vela/export_token_plan.py:129(export_head_inputs.pylikewise) loads the package via the pinned runtime's ownVela2.from_pretrainedwithout callingverify_packagefirst, unlike the bundle exporter. Trigger: recording fixtures against a wrong/modified package silently freezes wrong parity targets. (Fixed in the stack: #195's exporters callmember = verify_package(package).) - P3 — bundle-directory walk is single-level while the comment claims "at any depth".
src/models/vela/native/src/bundle.rs:73vs: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. Alsobundle.rs:83entry.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.tomlworkspace 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.ymlnav (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 CIsuccess(id 40277592873) and Docssuccess(id 40277592845); check-runs benchmarks/build/cuda-compile/cuda-contract/rust allsuccess, 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).
Review follow-up on the export recipe, kept minimal, mirroring the valen export precedent: - export_native.py stages the bundle set in a sibling directory and moves it into place with one rename, so a crash or a full disk mid-export can no longer strand a partial bundle under --out. The same staging covers the optional --bf16-out backbone directory. A non-empty target is refused and a stale staging directory from a killed run is cleared by the next run with the same target. - native.md records the staging behavior next to the export command. The written bytes are unchanged; only the path they take to the final directory differs. stage_into_place behavior checks and PYTHONPATH=src pytest tests/vela (135 passed) run at this head.
|
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. |
Stacked on #151 + #174 — review the last two commits only.
Required human contributor answers
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.)
What is the highest-leverage variable?
Exact token-level parity with the pinned runtime is the foundation every later stage validates against.
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.
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@0f1e4dacargo test -p omni-vela-nativecargo clippy -p omni-vela-native --all-targets -- -D warningsmkdocs build --strictVELA_NATIVE_BUNDLE=… cargo test --test vela_native_processing -- --ignoredTest Result
All passed. Render parity is opt-in (needs the exported bundle's tokenizer); CI covers CPU tests only.
Demo / evidence
Author-run, x86 WSL2, CPU FP32:
usage.input_tokens.#[path]wiring.Not measured: GPU, performance.
Self-review