bench: add frozen HTTP workload and concurrency matrix

[Levius-Fubuki]

CPU maintenance and merge handoff — 2026-10-15

Current head: 2d25d16b5a3d6842a76438918ede3993526d3f8e.
Actual target: main at f2d3d32897c1fe190fecf853a82a9456d3791462; the PR target was not changed.

Moved the existing Frozen serving matrix README section ahead of the Laya section without changing its contents. This removes the actual README append conflict found when preparing #93 and #116 together; it introduces no serving or validator dependency. A local current-main + #93 + this-head rehearsal merged cleanly and passed smoke validation plus all 65 benchmark tests. #141/#131 remain separate integration work; no other contributor's PR was changed.

Personally ran strict MkDocs and git diff --check on this documentation-only head; smoke validation and all 58 benchmark tests passed from its complete source archive.

This pass follows the contributor's 2026-10-15 instruction to maintain these existing PRs, complete CPU-only fixes and merge preparation, and keep #117 unmerged. Existing contributor declarations are preserved; this update adds no new human attestation or maintainer approval. No PR was merged and no GPU/model/performance run was performed. Source-pinned historical results remain historical. Current-head hosted workflows: CI run 38667549804, Docs run 38667552587. Read their live results separately from the personally executed checks above; these CPU workflows do not execute the skipped GPU tests.

Historical records below

Earlier maintenance/review/test sections retain their named revisions and environments. Their old ABI or build commands are historical reproduction instructions, not commands for deploying this current head. Use the current branch recipes for builds; the CPU checks above do not qualify skipped device tests.

Required human contributor answers

These answers reuse the contributor's explicit 2026-10-12 maintenance instruction:
keep the existing PRs mergeable and CI passing; eliminate conflicts and review
issues; serve maintainers who need clear, verified changes that are easy to merge;
repair, validate and push to the existing PRs. The source is the contributor's
local maintenance conversation on that date; private conversation details are
not published.

1. What outcome are you trying to achieve?

   Keep the existing PRs mergeable and CI passing.

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

   Eliminate merge conflicts and resolve review issues.

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

   Repair the identified conflicts and review issues, validate the affected
   integration, and push the verified changes to the existing PR branches.

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

   Repository maintainers, who need clear, verified changes that are easy to merge.

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

The contributor explicitly directed updates to the existing PRs. Maintain these
PRs; no new PR is needed for the maintenance work.

Observed bug-fix evidence: before the deadline correction, validation or connection close could finish after the deadline while the exchange still reported success. The deterministic regression reproduced that result; after the fix it records timeout, keeps raw response and credits zero decisions. See the correction and regression and test_validation_or_close_over_deadline_cannot_succeed. The pre-fix failure and corrected local records remain in the maintenance evidence archive. Blocking stages still do not guarantee a hard wall-clock return bound.

Purpose

Add a frozen-plan HTTP matrix runner for workload variants and closed-loop concurrency against an existing worker/frontend. Responses must match a predeclared exact JSON or raw-byte oracle. Preserve plan/runner hashes, raw responses, failures and planned/attempted/successful denominators. Stop dispatch after failure and drain admitted work.

Maintained integration — 2026-10-13

Head 4f2e34d78499e945c867e4a2dea3a100e8236bde, normal merge of main 3e9cc33a61ddf110d97512fb6f296cf540377c61. Independent review found a deadline defect: response validation or close could finish after the deadline while the exchange still reported success. A deterministic regression reproduced it; the post-validation/close deadline check now records timeout, retains raw response and credits zero decisions.

DNS, connect, validation and close are non-preemptible; the deadline is a completion classification, not a hard wall-clock return guarantee. Documentation now states this explicitly. Frozen budgets, failure drain, timer exclusions and interior completion-window limitations are preserved.

Test Plan

System1-Omni Version / Commit: 4f2e34d78499e945c867e4a2dea3a100e8236bde.

Reproduction commands from the repository root; recorded local feature switches and device prerequisites remain scoped to the original results above. This PR-body update runs no additional source or device checks.

cargo fmt --all --check
cargo clippy --workspace --locked --all-targets --features omni-laya/serve -- -D warnings
cargo test --workspace --locked --features omni-laya/serve
cargo build --workspace --release --locked --features omni-laya/serve
python3 -m mkdocs build --strict

python3 -m unittest discover -s tests/benchmarks -p 'test_*.py' -v

Run the frozen synthetic matrix example as documented in benchmarks/serving_matrix.md; retain the predeclared plan, full raw responses, failure classification and denominators. The client targets an existing server; synthetic checks are not model throughput evidence.

Test Result

43 focused / 58 complete Python tests pass; the 22-request, four-round synthetic example, Python compilation, Ruff and strict MkDocs pass. The reviewer reran the corrective tests. Current-head GitHub Rust, benchmarks, CUDA-contract and docs checks pass. Synthetic transport checks do not establish inference throughput or a performance gain.

Demo / evidence

The committed synthetic plan and deadline regressions are linked in the observed-evidence section. Exact-head CI and Docs passed; this is transport/validator evidence, not model inference or throughput evidence.

Video: N/A for this nonvisual API, validator, shared executor or documentation change. Source-pinned tests and the linked validation records provide the relevant evidence; existing hardware and unverified-measurement limits remain unchanged.

Self-review

The contributor explicitly confirmed completion of self-review and authorized this checklist/template update on 2026-10-13 in the maintenance conversation. This records the human confirmation; it does not turn unverified measurements into passes or replace maintainer review.

• 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.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[hsliuustc0106]

Reviewed at head fc96d38a0 (base 4a79980d; 2 files, +641). Solid frozen-workload client: every case/variant pins one exact oracle (type-strict JSON or exact bytes), the plan is snapshotted byte-exact with plan_sha256/runner_sha256, concurrency budget is pinned (2 warmups / 2 repetitions), denominators are kept planned/attempted/successful with failure kinds and raw bytes retained, errors stop dispatch and drain in-flight, and output refuses to overwrite. I verified the body's claims (26 tests / 18 new; +641/2 files) and the evidence tarball's SHA-256 matches the body exactly.

Personally run at this head: python -m unittest discover -s tests/benchmarks -p 'test_*.py' -v → 26 tests OK; python benchmarks/bench.py validate benchmarks/smoke.jsonl → 4 requests validated (base runs 8 → confirms 18 new). Observed CI: benchmarks/build/docs/rust all pass.

Three P3 findings:

• The tool ships undocumented: commit cd530dcc ("chore: retain only core implementation in PR diff") deleted benchmarks/serving_matrix.md (+123 lines of CLI/schema/oracle/limitations docs, including the "no token injection" caveat) that earlier commits in this same PR had built; benchmarks/README.md was not updated and no example plan is committed, so the plan schema is discoverable only from test code. Documentation is not diff bloat in a tooling PR — restore it.
• benchmarks/serving_matrix.py:90-91 — metadata is only checked to be a dict; "metadata": {} yields a complete: true run with zero hardware/model/runtime identity, diverging from bench.py's ten enforced fields and the benchmarks measurement rules.
• benchmarks/serving_matrix.py:136-138 — no auth/header support; a frozen matrix against a token-protected deployment (which bench.py supports via OMNI_JEV_TEST_TOKEN) fails every request with no workaround.

Notes (non-actionable): missing top-level plan keys raise KeyError tracebacks (still fails safely); successful_decisions counts list elements for list-shaped answers.

No GPU campaign needed and none claimed; no performance claims made. Rust suite covered by observed CI (no toolchain on this host).

Provenance: canonical .agents/skills/system1-omni-review/SKILL.md (SHA-256 58be3bc6…dde108) and its repository map at trusted base 4a79980d8a75; remote head rechecked before posting.

[Levius-Fubuki]

@hsliuustc0106 already fixed 👀

[MinhaoLi0318]

Reviewed at 9778a75a6d (base 4a79980d), focusing on benchmark methodology. The three earlier findings are resolved:

• the ten metadata fields are enforced (serving_matrix.py:85-93);
• Bearer auth now covers health and every phase (:212-216, :231, :245);
• the guide and the example plan are back.

On CPU (Python 3.12.14, httpx 0.28.1), the full benchmarks suite passes 29/29, bench.py validate benchmarks/smoke.jsonl validates 4 requests, and the example snippet in serving_matrix.md:81-94 completes. Several parts are good as they are: the byte-exact plan snapshot with plan/runner SHA-256, separate planned/attempted/successful counts, retained failure latencies and raw bytes, and clearly documented closed-loop semantics (no arrival-rate model). The findings below are about whether the numbers the tool produces can be used and compared.

1. blocking: the exact whole-body oracle cannot match responses that include per-request timing. exact_json compares the entire document, including every key (serving_matrix.py:96-103, used at :157-160). The merged Open-Jev native worker returns metadata.inference_seconds measured per request (src/models/open_jev/native/src/processing.rs:102-106). The JEMM worker in #118 (head a3a861cd) returns usage.latency_ms (src/models/jemm/native/src/processing.rs:323). I reproduced this locally with an Open-Jev-shaped response and an oracle captured from one response, as serving_matrix.md:43-44 instructs. The first readiness request fails, and the run stops with complete: false, failures: {"correctness": 1}, one inference request and zero rounds. So the matrix cannot measure the repo's main native worker. Suggestions:

   • Add an optional ignore_paths list (JSON Pointers, for example ["/metadata/inference_seconds"]) to the plan, validate it at load time, and drop those paths before comparing. Because it lives in the plan, it is covered by plan_sha256. Keep storing the raw bytes as now.
   • Alternatively, let a case declare an oracle over answers (plus model and usage.input_tokens) only.
   • Either way, add a test with an Open-Jev-shaped response.

2. should-fix: the run budget is fixed at n=2, and no spread is reported. repetitions and warmup_per_case must be exactly 2 (serving_matrix.py:78-81). benchmarks/README.md:134-136 asks for two measured repetitions "unless a different budget was predeclared", and a hashed frozen plan is exactly that predeclaration. summary.json lists each round separately but gives no spread across rounds (:189-205, :306-310). Two back-to-back rounds can agree closely and still not show run-to-run variation. In my local check (c=64, n=64), the two rounds of one invocation differed by about 2% (1698 vs 1724 req/s), but separate invocations ranged from 796 to 1775 req/s. In vllm-omni's nightly perf runs, n=10 results moved by about 15% between nodes (vllm-project/vllm-omni#8614). Suggestions:

   • Accept a predeclared repetitions >= 2 and warmup_per_case >= 2, with 2 as the documented default.
   • For each (case, concurrency), add min/median/max and the relative spread across rounds to summary.json.

3. should-fix: requests/s includes ramp-up and drain, and a round can be as small as the concurrency. validate_plan only requires concurrency <= requests_per_case (:76). Throughput is successful requests divided by the whole wave time (:199-200, :287-292), so with requests_per_case == concurrency each worker sends exactly one request, and req/s is roughly c divided by the slowest latency. Locally I ran c=64 against a fixed 20 ms handler (four invocations) and compared req/s with c/p50:

   requests_per_case	req/s ÷ (c/p50)
   64	0.45–0.78
   256	0.58–0.83
   2048	0.92–0.96

   Suggestions:

   • Require requests_per_case >= k × max(concurrency) (for example k = 16), or report throughput over a steady-state window that excludes the first and last c completions.
   • Note in serving_matrix.md:66-67 that nearest-rank p95 is the maximum when n < 20. The example plan's p95 is the maximum of 4 samples.

4. should-fix: no run time or host identity is recorded. config.json holds only the hashes, the plan and the client settings (serving_matrix.py:223-228). There is no UTC start/end time, client hostname, OS/platform or Python version. bench.py at least records Python and httpx versions (bench.py:251-252). The required metadata has no server host/node field (:87-90). Without these, rounds from different nodes or days cannot be told apart afterwards. The only absolute time in the output is whatever Date header the server sends. Suggestions:

   • Record automatically: started_at/finished_at (UTC), socket.gethostname(), platform.platform(), platform.python_version() and os.cpu_count().
   • Add a required host (server node) field to the metadata, or state in the guide that reservation must name the node.

5. nit: a new TCP connection per request, unlike bench.py. exchange opens and closes a connection for every request (serving_matrix.py:106-112, :179). bench.py keeps a keep-alive pool sized to the concurrency (bench.py:259-265), so latencies from the two tools in benchmarks/ are not directly comparable. Over HTTPS, every request also pays a TLS handshake. On loopback with the same 20 ms handler, the new-connection p50 was 0.5–0.6 ms higher than keep-alive (24.61 vs 24.07 ms, 24.93 vs 24.34 ms). Suggestions:

   • Reuse one connection per closed-loop worker, or record connect_seconds separately.
   • State the chosen definition next to serving_matrix.md:63.

Verdict: Comment. Item 1 blocks using the tool on the main native worker. Items 2–4 determine whether the matrix's numbers can be compared across runs and nodes.

AI assistance: Claude Code ran these checks and drafted this review.

[hsliuustc0106]

Incremental re-review at head a9da233f (base 4a79980d), following my review of fc96d38a0. git range-diff confirms commits 1–6 are patch-identical; the two new commits are 9778a75 "fix(bench): require runtime provenance and authenticate matrix requests" and a9da233 "fix(bench): compare native timers and report matrix measurement scope". Incremental diff: 5 files, +616/−20 — benchmarks/serving_matrix.md (new, +158), benchmarks/serving_matrix.example.json (new, +43), benchmarks/README.md (+6), benchmarks/serving_matrix.py (+170), tests/benchmarks/test_serving_matrix.py (+259). Full PR stays 5 files, +1237, benchmarks/tests/docs only; no serving or runtime code touched. Remote head rechecked via the GitHub API before this review: a9da233f, OPEN, non-draft.

Prior findings — fix status

Prior finding (at fc96d38a0)	Status	Evidence at head
P3: tool ships undocumented (cd530dcc deleted serving_matrix.md; no example plan; README silent)	Fixed	serving_matrix.md restored and expanded (schema, oracle rules, ignore_paths, auth, phases/metrics, measurement scope, limitations, reproduction script); committed example plan; benchmarks/README.md:179 links both.
P3: "metadata": {} yields complete: true with zero identity (serving_matrix.py:90-91)	Fixed	serving_matrix.py:130-136 now requires 11 nonempty, non-blank fields — gpu, gpu_ids, driver, cuda, precision, model_revision, runtime_revision, cache_policy, cuda_evidence, reservation (bench.py's ten) plus host — enforced before output/network. test_metadata_requires_each_pinned_identity_before_creating_output loops 11 fields × 5 falsy values and asserts no output dir. The example plan records explicit N/A-with-reason for a CPU fixture.
P3: no auth support for token-protected deployments	Fixed	serving_matrix.py:326-330 reads OMNI_JEV_TEST_TOKEN (same env var as bench.py:257), requires printable ASCII (rejects CR/LF header-injection and non-latin-1, fail-closed before network/output), and sends Authorization: Bearer … on health and every phase (:204, :352, :366). The auth test asserts all 23 requests carried the header and that the token appears in no output file. Docs direct credentials out of the plan and recommend HTTPS.

Both prior non-actionable notes are unchanged: missing top-level plan keys still raise KeyError tracebacks (still fails safely before network/output), and successful_decisions still counts elements for list-shaped answers.

New commits

Provenance (9778a75) — enforced for presence, honest about being advisory in fact. The client can refuse a plan that omits identity; it cannot verify the declarations, and the restored docs say exactly that: "The client checks presence; it does not independently verify these declarations or prove CUDA execution." Runner identity is pinned via runner_sha256 and client hostname/platform/Python/CPU count in config.json. The host field is a documented superset of bench.py's ten fields (matrix runs target a server node). No leak paths found: the token is read once from the environment, never written to plan.json/config.json/responses.jsonl (request headers are not saved; only response headers are retained, as before).

Native timers and scope (a9da233). "Native timers" covers two things, both sound:

• Server-native timing fields. ignore_paths (RFC 6901) excludes only volatile primitive leaves such as Open-Jev's /metadata/inference_seconds or JEMM's /usage/latency_ms. Exclusions are type-checked, must resolve in the actual response, cannot hide changed answers/missing fields/type flips (tested), cannot launder non-finite lexemes (1e309 in an excluded field still fails, tested), are rejected for overlapping prefixes (a prefix must itself be a primitive leaf, so /a + /a/b cannot validate), are incompatible with exact-byte oracles, and all validation happens before output creation or network access. Raw bytes and the byte-exact plan snapshot are retained, so excluded values stay auditable.
• Client clock. Completion timestamps use time.perf_counter — monotonic, process-wide, cross-thread comparable, which is what the interior-window estimator requires — recorded as completed_at_monotonic_seconds; connect_seconds splits out DNS/TCP/TLS. Latency semantics are stated precisely: fresh connection per request, including connection establishment, serialization, full read, oracle validation and close — and the docs explicitly warn that bench.py's keep-alive pool makes the two tools' figures non-comparable.

Measurement-scope reporting is honest: headline round rates are labelled throughput_scope: "complete_wave_including_ramp_and_drain"; steady_state excludes c completions at each end, reports excluded_each_end, and is available: false with a reason for n <= 2c, failed or zero-duration windows; excluded_phases lists health/readiness/warmup/feasibility; failed rounds are counted and excluded from aggregates with planned/attempted/complete/incomplete denominators preserved; p95 = max under 20 samples is now documented; the docs state the interior estimator "does not prove stationary traffic or isolate server-only throughput". No tolerance was relaxed and no failures discarded.

One deliberate scope change to be aware of: repetitions/warmup_per_case moved from "exactly 2" to ">= 2, default 2" (serving_matrix.py:121-124). The budget stays plan-predeclared and bound by plan_sha256, and effective_budget is recorded in config.json with docs instructing not to extend it after seeing results — acceptable, but anyone comparing runs across PR revisions should read effective_budget, since the earlier reviewed pin was 2/2.

One new P3:

• benchmarks/serving_matrix.md:112 — "The four-request/two-worker example deliberately has no interior-window estimate" is only true for the example's concurrency=2 rounds. The committed example plan uses concurrency: [1, 2] with requests_per_case: 4, so its concurrency=1 rounds satisfy 4 > 2·1 and do report steady_state.available: true over n−2c = 2 interior requests. I observed this by running the committed example against the synthetic server (c=1 rounds: available=true, requests=2; c=2 rounds: available=false). Trigger: a reader quotes the doc and misses that the example emits a thin 2-sample interior estimate, or cites it as steady-state throughput. Suggest rewording, e.g. "has no interior-window estimate at concurrency 2".

Evidence

• Personally run at a9da233f (extracted with git archive to a scratch dir; SHA-256 of benchmarks/serving_matrix.py matched git show byte-for-byte — note the local checkout was on an unrelated branch, so the suite was not run from the working tree): python -m unittest discover -s tests/benchmarks -p 'test_*.py' -v → 41 tests OK (33 in test_serving_matrix.py: 31 MatrixTests + 2 MetricTests; 8 pre-existing in test_bench.py), Python 3.12.8.
• Ran the docs' example reproduction script: the committed example plan validates and completes end-to-end (4 rounds, 2 aggregate groups), confirming the restored docs are executable.
• Inspected the new tests: auth coverage (23/23 header assertion + token-absent-from-every-output-file), provenance matrix (11×5 falsy values, no output), pointer escaping/array-index canons, non-finite-in-excluded-field rejection, interior-window math on shuffled completion orders (4 interior requests, 18 decisions, 4.0 s window), short/failed round exclusion.
• Observed CI at this head: benchmarks pass (26 s), build pass (18 s), rust pass (1 m02 s); deploy skipped.
• No performance or numerical claims are made by the PR and none were evaluated; no GPU or model execution was needed or performed.

Provenance: canonical .agents/skills/system1-omni-review/SKILL.md at trusted base 4a79980d8a75, SHA-256 of bytes read 58be3bc60806c4c5af7038a0c6a80fadb94f3360d714556a3e965c8903dde108; diff pinned fc96d38a0…a9da233f; remote head rechecked before posting.

Material unverified checks

• metadata provenance is presence-only by design; correctness of any real deployment's cuda_evidence/model_revision claims remains with the contributor's evidence, which this PR does not provide.
• Timer/latency behavior exercised only on macOS arm64 + Python 3.12.8 locally; Linux CI runner behavior of perf_counter/threading.Timer is assumed, not personally observed.
• Auth was verified against a loopback fixture, not a real token-protected deployment.
• Rust/docs checks are covered by observed CI only (no Rust toolchain on this host; no docs/ changes in the PR).

[hsliuustc0106]

Incremental re-check at new head cbdd473d43 — rebased after my review at a9da233f2a (range-diff: all commits patch-identical, none added or dropped; no content change). Prior status unchanged: the three findings from the 2026-10-08 review remain fixed; the one open P3 from that review (the serving_matrix.md example's c=1 rounds do emit a thin 2-sample steady_state estimate despite the "deliberately has no interior-window estimate" claim) is still open. No CI on this head yet (identical benchmarks/rust/build series passed at the reviewed head). Verdict: COMMENTED (one open P3).

[hsliuustc0106]

Correction at head cbdd473d43 (unchanged since my 2026-10-10 re-check; found in the 2026-10-11 audit). That re-check claimed "rebased … all commits patch-identical, none added or dropped; no content change" and reported the interior-window P3 open. Both statements were wrong: the head commit cbdd473 itself is a new commit editing exactly the sentence the P3 named — benchmarks/serving_matrix.md now says the concurrency-2 example has no interior-window estimate and that concurrency 1 can report a two-request window, "too short to establish stationarity". That is the fix the finding asked for, stated honestly. The sweep had compared against unfetched refs (tooling since fixed). Verdict: APPROVED — zero actionable findings at this head. Unverified: benchmark timings remain author-reported (no GPU on this host).

Provenance: review skill at ca5f69b (SHA-256 58be3bc6…dde108); fork CI green at this exact head.

[hsliuustc0106]

Incremental re-check at new head 4f2e34d784 after the rebase wave (prior review: APPROVED at cbdd473d4). 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.

New since that review: one commit 4f2e34d "fix(bench): enforce matrix deadline through validation and close" — reviewed: conservative deadline classification with preserved raw evidence and zeroed decisions, docs updated, and the full 35-test CPU suite passes locally at this head (personally run via archive extract). CI at this exact head: benchmarks/build (and docs) pass. All prior findings remain fixed.

APPROVED (unchanged verdict). Transition note: the body predates the post-#138 template — the four human contributor answers are not yet recorded; add them when the PR is next touched. Unverified: GPU behavior author-reported (unchanged); Rust suite covered by exact-head CI.

[MrDongsls]

Re-checked PR #116 at head 2d25d16b against the previously approved 4f2e34d. The incremental change is documentation-only: it reorders the Frozen serving matrix section in benchmarks/README.md to avoid the known #93/#116 README append conflict. No benchmark client, validator, test, serving, or runtime code changed.

The previously reported oracle normalization, metadata provenance, authentication, measurement-scope, and deadline-classification findings remain fixed. Current CI and Docs workflows pass, and the PR makes no model throughput or production-performance claim.