runtime: optional bounded admission

[xiaoyu-xyz]

Stacked on #34, which is still open. The diff below therefore contains #34's files as well; this change is src/runtime/src/admission.rs (160 lines), tests/runtime/admission.rs (110 lines), one line in src/runtime/src/lib.rs, and the [[test]] registration in src/runtime/Cargo.toml — git diff --stat 1363d47 5c37b54 is 4 files, +275. Once #34 lands this rebases onto main and the diff is only its own.

Purpose

The first step review pointed at on #35: optional bounded admission in the existing runtime, with capacity units explicit, and the worker lifecycle left out.

SerialScheduler admits one unit at a time and never refuses — a hundred requests queue behind each other and the only bound is the caller's patience. That stays the default. This is what a worker opts into when it wants a bound instead, next to the scheduler rather than in place of it.

Units are the caller's, and they are explicit

Admission counts two things: how many requests are outstanding, and what each one costs.

let admission = BoundedAdmission::new(3, Some(100));   // 3 requests, or 100 units
let permit = admission.try_admit(60)?;                  // this request costs 60

A request costs one; the cost is whatever the caller passes. A prefill worker can admit by token count, a worker with large payloads by bytes, a worker that wants neither by 1. Nothing here guesses which of those a model means — which is why the cost is a parameter rather than a documented convention, and why the type does not pick a unit for anyone.

The permit is the reservation

Capacity is held until the permit drops, so "outstanding" means accepted and not yet finished with rather than "accepted". A worker that hands work to a thread and forgets to release would be bounded forever, so release cannot be something a caller has to remember.

The permit is deliberately neither Clone nor Copy: two permits for one admission would release capacity twice, which is exactly the failure this type exists to prevent. Releasing more than was held panics rather than saturating, so a double release is visible instead of quietly drifting the counter.

Two cases a plain depth < capacity check gets wrong:

• A request costing more than the whole unit ceiling is refused, rather than admitted and overrunning it.
• A zero bound panics at construction. It would admit nothing at all, which is a configuration mistake; failing where it is written beats every request being turned away with no explanation.

The fit check and the reserve use the same saturating arithmetic, so a unit cost near usize::MAX cannot pass one and overflow the other.

report() returns Report { depth, capacity, rejected } — the same shape #34 publishes from /health, so a worker that opts in gets queue state on the health route without new plumbing. rejected here counts refusals from try_admit only; #34's Report documents a second term, work accepted and then dropped because its caller had gone, which an admission bound cannot observe.

Not in this PR

Per the same guidance: worker-thread ownership, readiness transitions, drain on shutdown, and any change to SerialScheduler or to the native workers. This changes no existing behaviour — nothing is bounded until something asks for a bound, and no worker asks today.

Test Plan

cargo fmt --all --check
cargo clippy --workspace --locked --all-targets -- -D warnings
cargo test --workspace --locked

tests/runtime/admission.rs, under the repository tests/ tree per CONTRIBUTING with an explicit [[test]] path:

• a request bound refuses once full, counts the refusal, and admits again once a slot frees
• capacity is held until the permit drops — the property that makes the bound mean anything
• a unit bound admits by cost rather than by count, including that a large request's units come back when it finishes
• one request costing more than the ceiling is refused rather than overrunning it, and one costing exactly the ceiling is admitted
• the request count is reported as capacity
• a zero request bound and a zero unit bound each panic at construction
• clones share one bound, so a queue and its handlers cannot disagree about how much room is left

Test Result

System1-Omni Version / Commit: rebased onto main 3d4ed19; head 6ce1882.

Run both locally and in CI on this head. Locally (macOS arm64, Rust 1.99.0, portable toolchain): cargo fmt --all --check clean, cargo clippy --workspace --locked --all-targets -- -D warnings clean, cargo test --workspace --locked 180 passed, 0 failed, 28 ignored. In CI on 6ce1882: rust completed/success (that job runs exactly those three commands), build completed/success, benchmarks completed/success, deploy skipped.

The count moved from 107 (at 19c4d80) to 180 because main advanced a long way while this was stacked, not because this change adds tests beyond its own: this PR contributes the 8 in tests/runtime/admission.rs.

No new dependency, and Cargo.lock is unchanged. src/runtime/Cargo.toml gains only the [[test]] target that points at the new test file, which is how this repository registers tests.

Rebased onto main 3d4ed19

The stack has collapsed: #34 is merged, so this branch no longer carries #34's commits. merge-base(main, head) is main itself, and the diff here is now only the four files this change owns (+275/-0). The reviewer's "rebase twice" note is therefore complete, and the rejected/413 changes it referenced are on main now rather than arriving with a rebase.

Changes since the review of 19c4d80

• try_admit now computes the sum once with saturating_add and uses it for both the fit check and the reserve. Previously the check saturated while the reserve added directly, so the two could disagree for a unit cost near usize::MAX. This is the reviewer's src/runtime/src/admission.rs:97 finding.
• The rejected doc above now states the term this type can count. The finding it answers (src/runtime/src/engine.rs:41-43) is in a file this PR does not touch, so the doc change itself lives on runtime: add the worker-side engine contract #34 at 095d831; what this PR does is count exactly that term — BoundedAdmission increments rejected only in the refusal branch of try_admit, and never for work it accepted. At this head the rejected field's wording in Report is still the broader one, because the narrowed version arrives with the rebase.
• Not addressed: the src/frontend/src/engine.rs:128 413-vs-malformed-body finding and the serve()/shutdown/Unavailable test gaps. Both are in runtime: add the worker-side engine contract #34's files rather than this change, and runtime: add the worker-side engine contract #34 is frozen at a reviewed head; they are follow-ups once the stack lands.

Demo / evidence

N/A — no runtime behavior applies to this change. Nothing constructs a BoundedAdmission outside its tests, so there is no measurable workload, latency, throughput or memory result for this diff: the library is added and unwired, existing deployments are unchanged, and there is no model, hardware or precision configuration to record. The evidence for this change is the test suite in the Test Plan, and the commands and outcomes are in Test Result above. A serving-level measurement becomes meaningful only once a worker opts in, which is a later step.

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]

Reviewed at head 19c4d8063 (merge base 99865743d; note the PR is behind current main 4a79980d — PR #92 — and will need a rebase). One structural note for reviewers: the GitHub files-changed diff (11 files, +1178) is the stack on open #34 — this head's parent is exactly #34's head 1363d4776. #109's own change is ~+273 in 4 files: src/runtime/src/admission.rs (+158), tests/runtime/admission.rs (+110), and five lines of registration. The src/runtime crate itself (with SerialScheduler) already exists at base, and the runtime/frontend engine.rs modules come from #34.

The admission type itself is clean, well-scoped, and genuinely optional: BoundedAdmission counts 1 outstanding request plus a caller-defined unit cost with dual bounds; Permit is RAII, releases exactly once, and double-release fails loudly via checked_sub(...).expect; poisoned locks are recovered with counters intact; oversize and zero-bound cases are tested. Nothing consumes the new APIs yet — SerialScheduler, the workers, and the frontend forwarding path are untouched, so existing deployments see zero behavior change. /health is correctly not admission-gated; queue-full maps to an immediate 503 and admitted-but-slow to 504, both tested.

P3 findings:

• src/frontend/src/engine.rs:128 — every to_bytes error becomes 413 "request body too large"; axum 0.8 surfaces non-size body failures the same way, so a malformed/aborted body can be mislabeled. Consider separating the length-limit case.
• src/runtime/src/admission.rs:97 — the fit check uses saturating_add (:89) but the reserve uses held.units += units; a caller passing pathological unit values can panic/wrap where the check saturates. Use the same arithmetic in both places.
• src/runtime/src/engine.rs:41-43 — Report.rejected is documented as also counting "accepted and then dropped because its caller had gone", but BoundedAdmission::report can only count try_admit refusals; narrow the doc or mark that term as engine-owned.
• Test gaps (coverage, not defects): serve()/graceful shutdown drain, the Unavailable and "engine stopped" 503 paths, any BoundedAdmission + Engine + transport integration case, and concurrent admit/release are unexercised.

Readiness: draft is appropriate — the body still needs the template's Demo/evidence section and the four self-review checkboxes before leaving draft, and the drain-on-shutdown behavior the docs defer remains unimplemented. CI (rust/build/benchmarks) passes on this head (observed); the author's local macOS counts (107 passed / 10 ignored) were not independently reproduced — no Rust toolchain on this host — and the axum error-conflation nit is asserted from the API shape, not demonstrated.

Provenance: canonical .agents/skills/system1-omni-review/SKILL.md (SHA-256 58be3bc6…dde108) and its repository map read at trusted base 4a79980d8a75, plus src/runtime/README.md, docs/architecture.md, CONTRIBUTING.md. Remote head rechecked before posting.

[xiaoyu-xyz]

Thanks — detailed and useful. Acting and non-acting, per finding.

Fixed, src/runtime/src/admission.rs:97 (head 5c37b54). The mismatch was real. The fit check saturated while the reserve added directly, so a request cost near usize::MAX could pass the check and then overflow on held.units += units — panicking in a debug build. The sum is now computed once and used for both sides:

let sum = held.units.saturating_add(units);
let fits_units = match self.units {
    Some(ceiling) => sum <= ceiling,
    None => true,
};
...
held.units = sum;

I did not add a test for it: reaching the disagreement needs a request cost near usize::MAX, and BoundedAdmission::new panics on a zero bound but accepts a usize::MAX ceiling, so the failing case is constructible but not cheaply assertable. The change is arithmetic consistency rather than a new behaviour, so it is covered by the existing suite rather than by a new case. Say the word if you would rather see the pathological-value test anyway.

Corrected in this PR's description, src/runtime/src/engine.rs:41-43. Report.rejected in #34 keeps its broader wording; the body here now states which term this type can actually count, and that "accepted and then dropped because its caller had gone" is not observable from an admission bound. The doc text itself is #34's file, so I have not edited it under this PR.

Now folded into #34, on your call being overridden by the maintainer's. The src/frontend/src/engine.rs:128 finding is fixed there instead of deferred: to_bytes reports both a body over the limit and a body that could not be read through one error type, so the two are now told apart by the source it wraps (LengthLimitError), which is the check axum's own documentation recommends — 413 stays the oversized answer and a malformed or truncated body becomes 400. Report.rejected's doc is narrowed the same way. Because that moves #34's reviewed head, its recheck at 1363d47 no longer covers the new commit and CI runs again there; I will not claim the new head is verified until those checks report.

The serve()/shutdown/Unavailable/concurrent-admit test gaps are still not addressed. They are coverage rather than defects, and I would rather add them deliberately than pad this change.

On readiness:

• The template's missing sections are now there: ## Demo / evidence (N/A with the reason — nothing constructs BoundedAdmission outside its tests, so there is no workload to measure) and the four self-review checkboxes. Test Result records the head, the commands, and what I could not verify.
• Drain-on-shutdown stays unimplemented and out of scope; the body says so under "Not in this PR", and it will need the transport-side work in runtime: queue-length limits for the native runtime — scope question #35's thread rather than here.
• Rebase is still not possible: runtime: add the worker-side engine contract #34 is still open (merged=false), so replaying onto current main 4a79980 would conflict rather than clean up. I will rebase as soon as runtime: add the worker-side engine contract #34 lands and will re-report then.

One correction to my own earlier note on #34, which this review's CI observation prompted me to check: CI does run on these fork PRs. .github/workflows/ci.yml triggers on pull_request: branches: [main], and this head has 4 check runs — rust, build, benchmarks success, deploy skipped. I had claimed CI never ran here, and that was wrong. On 5c37b54 the rust job — which is exactly cargo fmt --all --check, cargo clippy --workspace --locked --all-targets -- -D warnings, cargo test --workspace --locked — is completed/success, so this head is verified by CI rather than only by my local runs.

Correction, later the same day: I was wrong about the ready-for-review step, and have now marked this ready. I had said I could not, on the grounds that I have only pull on the upstream repository — but the PR author is the fork owner, and GitHub lets the author mark their own PR ready. The actual failure was my tooling (gh pr ready errored), not GitHub's permissions; the GraphQL markPullRequestReadyForReview mutation succeeds. My original sentence is left visible above rather than quietly rewritten.

It still is not rebased: #34 remains open, so replaying onto main 4a79980 conflicts rather than tidying the diff. That is the one remaining step, and it happens when #34 lands.

[hsliuustc0106]

Delta review at head 5c37b544a (one commit since my 2026-10-08 review: 5c37b54, "use the fit check's arithmetic for the reserve"). The in-scope P3 is fixed minimally and correctly: sum = held.units.saturating_add(units) is computed once and used in both the fit check and the reserve, with the doc updated; outstanding += 1 remains bounded by the request ceiling. Of the other three prior P3s, two (the 413-on-any-body-error split and the Report.rejected narrowing) are now fixed on the new #34 head 095d83107 and arrive here with the declared rebase — at this head those files still carry the old code, which is fine given the declared stacking. The body is now complete: Demo/evidence justified N/A and all four self-review boxes filled by the author with honest evidence discipline.

Still outstanding before this leaves draft:

• Rebase twice: the stack parent moved (1363d4776 → 095d83107) and the branch is behind main (4a79980d); this head still builds on the old #34.
• Test coverage (acknowledged by the author as follow-ups): no concurrent admit/release case (Mutex-poisoning recovery also unexercised), no BoundedAdmission + Engine + transport integration case, and the Unavailable/"engine stopped" 503 paths remain untested at this head.
• One wording nit in the body: the "changes since" bullet for the rejected doc reads as if committed in this PR; at this head it isn't — it lives on #34@095d83107.

Observed CI at this head: rust/build/benchmarks all SUCCESS. Not reproduced locally (no Rust toolchain on this host); the prior head's author-reported macOS counts are now stale relative to the arithmetic change.

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.

[hsliuustc0106]

PR #109 incremental re-review — "runtime: optional bounded admission" (head 6ce1882)

Independent incremental re-review following the 2026-10-08 review of head 19c4d8063/5c37b544a (/tmp/pr-review/body-109.md). Not posted anywhere; local draft only.

1. Scope, stacking and provenance

Pinned refs (both verified in the local repo; p6old/109 fetched this session per the missing-ref fallback):

• old head refs/remotes/p6old/109 = 5c37b544afb5e915bd18828980a7a05d2f701b4b (the 2026-10-08 review head)
• new head refs/remotes/origin/pr/109 = 6ce188297c7baeaf8c42d3c88e610e6a3a492f58
• remote recheck before reporting: git ls-remote origin refs/pull/109/head = 6ce188297c… — matches.
• PR state observed via gh pr view: OPEN, not draft (the task brief said DRAFT; it has since been marked ready for review), mergeable: MERGEABLE, base main@3d4ed19d9, head branch runtime-bounded-admission.

Skill load record: canonical .agents/skills/system1-omni-review/SKILL.md, SHA-256 of bytes read 58be3bc60806c4c5af7038a0c6a80fadb94f3360d714556a3e965c8903dde108 (matches the pinned value), plus references/repository-contracts.md, docs/architecture.md (delta inspected — it is main's Open-Jev-27B batching / JEV-VL documentation, untouched by this PR's own diff), src/runtime/README.md contract as inlined in src/runtime/src/{admission,engine}.rs docs at head, and CONTRIBUTING.md at head. One repo-level drift note: the review skill and the self-review skill both link CONTRIBUTING.md#large-code-changes, and that section/anchor does not exist in CONTRIBUTING.md at this head — recorded as repository doc drift, not a finding against this PR.

What the incremental delta actually is

git diff --stat p6old/109 origin/pr/109: 219 files, +21888/−416. That number is misleading without the merge structure:

• git merge-base(p6old/109, origin/main) = 1363d47 ("runtime: add the worker-side engine contract") — i.e. the old head was 1363d47 (#34's head) plus exactly the two admission commits.
• Since then main advanced from 1363d47 to 3d4ed19 (merges of #92, #105, #96, #108, #120, #25, #28, #64, #29 CLM, #34, #71), and the two admission commits were rewritten on top of the new main tip.
• git range-diff 1363d47..5c37b544a 3d4ed19..6ce1882 shows 19c4d80 = 6c2a060 and 5c37b54 = 6ce1882 (= = patch-identical). The bounded-admission change I reviewed is byte-for-byte what ships, plus its fix commit.
• Effective PR diff vs merge-base (git diff origin/main origin/pr/109): 4 files, +275/−0 — src/runtime/src/admission.rs (+160), tests/runtime/admission.rs (+110), src/runtime/src/lib.rs (+1), src/runtime/Cargo.toml (+4). Nothing else.
• So essentially the entire +21888 branch delta is merged main content authored by others (twu3202, linear3735, Levius-Fubuki) and by this author in other merged PRs (#29 CLM, #71 Laya) — not authored for #109.

Relationship to PRs #71–#74

• #71 "laya: pre-resolve native execution resources and kernel handles" (codex/laya-execution-plan, head b5550fd) is MERGED into main at 3d4ed19, merged 2026-10-11T05:05:38Z — i.e. after my 2026-10-08 review. Its commits (ebf4e8a "Add Laya CUDA resource ownership" through d235783 "perf(laya): resolve encoder resources and kernel handles at load", authored by linear3735, plus xiaoyu's integration merges by levius) are exactly the "Laya native CUDA series" visible in this delta. #109 does not supersede or subsume #71 — #109 is now stacked on top of #71's merged content.
• #72 (laya: reuse native input staging and group CUDA uploads, df9f4c6069), #73 (cuda: clean up failed and unwound Laya graph captures, 23f2680aa1) and #74 (laya: bound native workspace cache before allocation, f8fb7996de) remain OPEN, and their content is not in #109's branch. They are independent follow-ups hardening the same Laya layer #109 sits on (staging reuse, capture-unwind cleanup, workspace-cache bounds). Their open state is consistent with the merged layer being a first step; nothing in #109 conflicts with them.

2. Prior-finding status — every item re-verified in the tree at 6ce1882

#	Prior finding (2026-10-08)	Status at 6ce1882	Where fixed
1	P3 src/frontend/src/engine.rs:128 — every to_bytes error answered 413	FIXED and verified. Ok(Err(error)) now distinguishes the length limit via StdError::source(&error).is_some_and(|s| s.is::<LengthLimitError>()) → 413; anything else → 400 "request body could not be read". Regression test drives a malformed chunked body over a raw socket and asserts 400 + worker counter 0.	main, commit 095d831 (message credits this review by file:line)
2	P3 src/runtime/src/admission.rs — fit check saturating_add vs reserve held.units += units	FIXED in this PR. try_admit computes let sum = held.units.saturating_add(units); once and uses sum for both the fit check and the reserve (held.units = sum); doc comment states the invariant.	6ce1882 (in #109's diff)
3	P3 src/runtime/src/engine.rs:41-43 — Report.rejected doc overcounts	FIXED and verified. Doc now says the field counts refusals ("a bound counts the requests it turned away…") and explicitly excludes accepted-then-dropped work.	main, same commit 095d831
4	Test gaps: drain/shutdown, 503 paths, integration, concurrency	Partially improved on main; rest explicitly deferred. A a_busy_worker_produces_503_and_is_not_retried transport test exists (tests/frontend/in_process.rs, via #34's merge). Still unexercised at head: concurrent admit/release on BoundedAdmission, serve()/graceful-shutdown drain, admission+Engine+transport integration. The PR body now scopes this out deliberately ("worker-thread ownership, readiness transitions, drain on shutdown… changes no existing behaviour").	partial (main); deferral documented in #109

Residual nit on (2): the saturating-arithmetic unification has no regression test at the actual boundary — the tests cover over-ceiling (101>100) and exact-ceiling (100==100) unit costs, but no unit cost near usize::MAX that would have caught the original mismatch. P3/info; the unified saturating_add makes the panic unreachable, so this is coverage only.

3. #109's own change (+275, 4 files)

Patch-identical to the reviewed stack (range-diff =), with finding (2) fixed as above. Re-affirmed from the tree: BoundedAdmission is opt-in and unwired (nothing constructs it outside its tests; SerialScheduler and all workers untouched), /health is not admission-gated, the permit is RAII/non-Clone with loud double-release (checked_sub(...).expect), poisoned-lock recovery keeps counters, zero bounds panic at construction, and report() feeds the same Report shape #34's health route publishes. The body's claim that rejected counts only try_admit refusals matches the code (the counter increments in exactly one branch).

4. Laya native CUDA series review (merged main content via #71 — findings belong there, not to #109's diff)

Per the task, the series is reviewed against the skill's native/FFI/CUDA contract. Files inspected at head: src/models/laya/src/model.rs (executor, plan, workspace cache), src/backends/cuda/src/lib.rs (Rust bindings), src/backends/cuda/kernels/runtime.cu (device runtime), tests/cuda/laya/{fixture.c,fixture.rs,resolved.rs,README.md}, tests/cuda/laya_runtime.rs, tests/laya/{model,model_dispatch,model_fixture,original_encode,batch}.rs, src/models/laya/src/{artifacts,weights}.rs, recipe/laya/native/VALIDATION.md, src/models/laya/README.md. Note: at head the series' intermediate resident.rs/workspace.rs/encoder.rs modules no longer exist — merge 8f58f15 ("port prepared execution plan onto native worker") folded them into model.rs; the net delta vs p6old is model.rs (+460/−140), cuda lib.rs (+43), and the new tests/docs.

What holds up (contract-by-contract):

• Rust/C ABI lockstep: the Launch type unsafe extern "C" fn(*mut Ptr, i32, i32, i32, Ptr) -> i32 matches the fixture's KERNEL(name, count) macro and runtime.cu's wrappers exactly (args, b, l, rows, stream); laya_linear's 10-parameter signature matches Blas::linear's Linear type parameter-for-parameter (rows, columns, width, gelu). Kernel entry points are resolved once at load (Cuda::resolve, commit d235783) instead of per launch.
• Bounded workspaces: Workspace::new allocates exact-size buffers for a fixed (b, l); infer validates b ≤ 16, l ≤ 512 and l % 16 == 0, b == markers.len().next_power_of_two(), per-row lens/markers bounds, dummy rows zero-length, and rows ≤ 2048 before any launch. The reuse cache is bounded twice over: at most 4 workspaces and at most 512 MiB total (while self.cache.len() >= 4 || sum + s.bytes > 512 MiB), FIFO eviction, and a workspace that fails mid-infer is dropped (freed), never cached. Workspace field order destroys the graph before its referenced allocations, and Model::drop syncs then clears the cache first.
• Residency: all checkpoint tensors are uploaded once at load into HashMap<String, Buffer>; the dtype policy (f16 embeddings, f32 norms/biases/scorer.0, bf16 rest) is pinned by resident_precision_matches_cuda_consumers (206 tensors, exactly 1 f16); LAYA_VERIFY_WEIGHTS=1 does a read-back byte comparison per buffer; validate_names enforces an exact expected/actual tensor-set match before the plan is built, so the later weights[name] indexing cannot panic on a bad checkpoint. Rotary tables are size-checked exactly (data.len() == 512*32*4, "invalid rotary table size") — the 0a349f3 validation is present.
• Stream/device ownership: one non-blocking stream created in laya_init; Cuda/Buffer/Kernel/Graph all hold Rc<Context> (deliberately single-threaded, !Send world, matching "buffers and graphs remain on the owning worker thread"); capture uses cudaStreamCaptureModeThreadLocal; every Drop (Buffer, Graph, Context, Blas, Model) syncs before freeing. resolved.rs proves a resolved Kernel keeps the context (library+stream) alive after the Cuda handle is dropped, and that the stream is freed only with the last owner.
• Capture error recovery: capture() syncs, begins, runs the work, ends; on work error or end error it frees a non-null graph before propagating. The fixture injects exactly the two nasty cases — partial ownership with an error (graph_mode=1: handle set, error returned → Rust frees it) and null-success (graph_mode=2: Ok with null → later replay fails cleanly). Warmup discipline (two eager encodes + sync) precedes capture; dumps are disabled in graphs mode so no readback/sync can occur inside capture; inputs are written before replay.
• Sparse-bundle compatibility (ecfdeaf): specialized attention wrappers (attn_{full,local}_b{1,4}_l512) are optional at resolve; generic shapes fall back to the eager kernel; a missing specialized shape fails with a specific error. missing_specialized_attention_only_rejects_its_shape tests precisely that with an OMIT_SPECIALIZED_ATTN fixture build.
• Dispatch parity: prepared_plan_preserves_original_launch_trace_for_all_shape_selectors freezes the pre-plan eager launch sequence (from 7f39ac4) and requires the prepared plan to reproduce it — kernel names, (b, l, rows) and pointer values — byte-identically, for both rope variants × b∈{1,2,4,8,16} × l∈{16,512}, 242 launches per encode. This is a strong host-side regression oracle for the d235783 "resolve at load" rework.

Findings in the series (all attributable to merged main / #71's integration merges, since #109's diff does not touch these files):

• L1 (P3, merged-main content): ownership/ABI host-test coverage added by the series was then dropped in the merge resolution. ebf4e8a's src/backends/cuda/tests/runtime.rs (254 lines) contained seven tests: incompatible-library load rejection, copy round-trip + bounds, copy errors still synchronize (copy_errors_still_synchronize_and_async_errors_are_reported — the "submission may report failure after copying; Rust still must drain it" case), failed allocations/uploads releasing partial resources, buffers keeping the library/stream alive, operations/drops restoring the device, and a GPU round-trip. At head only the context-retention test survives (as tests/cuda/laya/resolved.rs); the other host-side tests exist nowhere in the tree (name search across the whole head). fixture.c still carries the now-dead injection hooks (copy_mode/test_copy_mode, sync_count/test_syncs, test_allocations, test_live_bytes/test_peak_bytes/test_reset_peak) with no Rust consumer. The dropped drain-after-failed-copy behavior is now asserted nowhere.
• L2 (P3, merged-main content): the series' GPU numerical and reuse tests have no in-tree successor. 5b5518b's encoder reuse check ("Reusing the same allocations must not accumulate residuals across requests" — re-run equality without intermediate synchronization, the numerical invariant this task specifically asks about) and 684470a's distinct-mixed-rows-at-maximum-batch assertion lived in src/models/laya/src/encoder/tests.rs, which was deleted in the 8f58f15 fold. Numerical claims now rest solely on author-run recipe/laya/native/VALIDATION.md (H800, frozen binary/library SHA-256s, "210 responses, 36 hidden-state comparisons and six 17-request reuse sequences, bitwise equal") — good documentation, but not executable in this repository. The fixture README is honest about this ("checks dispatch pointers/order, not numerical outputs").
• L3 (info): no ABI version handshake for the Laya bundle. Cuda::load is unsafe with a documented trust assumption ("matching native ABI… produced by this backend's build tools") and artifacts::validate_bundle hash-checks the bundle against the checkpoint, but unlike the qwen3_5 backend there is no abi_version the library and Rust must agree on; a stale bundle fails only by symbol-miss or misbehaves.
• L4 (info): device/arc pinning. laya_init hardcodes cudaSetDevice(0) and refuses non-Hopper (compute-capability major ≠ 9 → error). The LAYA_CUDA_DEVICE selection added by 0a349f3 was removed during the integration rework (no occurrence at head; README updated consistently), so multi-GPU selection is only via CUDA_VISIBLE_DEVICES, undocumented.
• **L5 (info): launch_rows (the gather/features glue) bypasses the b ≤ 16 / l ≤ 512 / l % 16 shape gate that launch/Kernel::launch enforce; correctness relies on caller-side bounds (rows ≤ 2048, n ≤ 16) in infer.

5. Frontend 400-vs-413 fix (095d831, on main)

Correct, minimal, and it fixes prior finding (1) exactly as recommended:

• Separation uses StdError::source(&error).is::<LengthLimitError>() — the check axum's own documentation prescribes for to_bytes, so only the length-limit case stays 413; a malformed/truncated body is now 400 "request body could not be read". The 504 gateway-timeout path and all client behavior (proxy bypass, no redirects, no retries, 60s deadline) are untouched — this is a pure status-mapping change at the read boundary.
• Cost: one new direct dependency (http-body-util) in src/frontend/Cargo.toml/Cargo.lock.
• Evidence is transport regression evidence, as the skill requires: a_body_that_cannot_be_read_is_not_reported_as_too_large opens a raw TCP socket, sends a chunked body whose chunk size is not hex (a client library will never send this), and asserts a bounded-time 400 with the new message and seen == 0 (the request never became work).
• The same commit narrows Report.rejected's doc (prior finding 3) and cites the review it answers.

6. CI retarget

The net ci.yml change in the delta is cargo clippy/test/build gaining --features omni-laya/serve, so CI compiles the Laya native worker and runs its root-tree tests. It arrived through the Laya stack that merged to main (commits 6ae5e0f, 8f58f15, 39357eb, 28083ea — the "ci: validate retargeted Laya host changes against main" commit 6043294 is itself a merge; author levius), and CONTRIBUTING's check commands at head document the same feature flag. It is not part of #109's diff (ci.yml at head is identical to main), so scope-appropriateness for #109 is moot; for the model addition it belongs to it is appropriate, with the ordinary consequence that the workspace CI surface now includes the laya serve feature.

7. Readiness (CONTRIBUTING)

• Counts, reported separately per the skill: authored diff = 4 files, +275/−0 (well under the 3,000 authored-line threshold; the self-review skill's large-change extras do not apply). Total branch delta vs the pinned old head = 219 files, +21888/−416, of which all but the two admission commits is merged main.
• PR state: OPEN, non-draft, MERGEABLE, base main@3d4ed19. The stack has collapsed (#34 merged), so the GitHub diff is exactly the four owned files.
• Body: complete — Purpose (units explicit, permit-as-reservation rationale), Test Plan (the repository's four documented commands), Test Result (local macOS: fmt/clippy clean, 180 passed/28 ignored — plausible given main's growth, not reproducible on this host; CI jobs cited per name), a "Rebased onto main" section that correctly explains where each prior review finding landed, a reasoned Demo/evidence N/A (the library is added unwired; there is no workload to measure — correct), and all four self-review checkboxes completed. The prior readiness gaps (missing evidence section, unchecked boxes, draft state) are all resolved. Minor: the body's opening line still says "Stacked on #34, which is still open" and cites git diff --stat 1363d47 5c37b54 — stale, and superseded by the body's own later rebase section.

8. Evidence

• Commands personally run: git fetch origin 5c37b544a…:refs/remotes/p6old/109; git rev-parse both pins; git diff --stat (branch and vs-main); git log --graph; git range-diff 1363d47..5c37b544a 3d4ed19..6ce1882; git merge-base/--is-ancestor containment checks for 3d4ed19, 095d831, 998af77, c038268, 7cbce19, 6b51652, de255a7 (all in main); per-commit diff-tree file inventories for the Laya series; git show reads of admission.rs, runtime/src/engine.rs, frontend/src/engine.rs diff, tests/runtime/admission.rs, tests/frontend/in_process.rs diff, 095d831, 6043294, ci.yml delta, model.rs (full), cuda/src/lib.rs (full), kernels/runtime.cu, tests/cuda/laya/*, tests/laya/{model,model_fixture,model_dispatch,original_encode,batch}.rs, artifacts.rs/weights.rs excerpts, VALIDATION.md, CONTRIBUTING.md; tree greps for the dropped tests and LAYA_CUDA_DEVICE; gh pr view 109/71/72/73/74; gh pr checks 109; git ls-remote origin refs/pull/109/head.
• CI observed at 6ce1882 (gh pr checks 109): rust pass (1m52s), build pass (12s), benchmarks pass (13s), deploy skipping — matches the author's Test Result claims.
• Host constraints honored: no cargo invoked (no Rust toolchain), no GPU. Per the skill these are evidence gaps, never PASS.

9. Material unverified checks

• Local Rust suite at 6ce1882 (fmt/clippy/test/build): not run — no toolchain. CI green corroborates but does not reproduce the author's local counts (180 passed / 28 ignored).
• Laya GPU behavior: kernel numerics vs the reference, graph capture/replay on a real device, workspace growth/eviction under real allocation pressure, the "CUDA failure stops this executor; /health returns 503" path — no GPU on this host; entirely unverified here.
• VALIDATION.md's frozen H800 measurements and bitwise-equality claims (pinned binary/library SHA-256s, 210-response/three-way/reuse-sequence checks): author-run evidence, not corroborated.
• The dropped-series tests (L1/L2) were confirmed absent by name search across the whole head tree; I did not exhaustively prove no equivalent coverage exists under different names (the trace oracle and dtype/residency tests do partially overlap the old suite's dispatch and precision intents).
• #72/#73/#74 content (staging reuse, capture unwinding, workspace-cache bounds) not reviewed — open follow-ups against the same merged layer.

10. Verdict

No new actionable findings in #109's own diff. The change remains what the 2026-10-08 review found it to be — a clean, genuinely optional, well-tested admission type — and prior finding (2) is fixed in-head with verified arithmetic; prior findings (1) and (3) are fixed on main by a commit that credits this review and was verified in tree; the remaining test gaps are explicitly and reasonably scoped out. The Laya series in the branch delta is merged main content via #71 (merged 2026-10-11) — #109 sits on top of it rather than subsuming it, and #72–#74 remain open, independent follow-ups. That series carries the report's actionable items, all against main rather than this PR: P3 L1 (ownership/ABI host tests dropped in merge resolution, dead fixture hooks retained) and P3 L2 (GPU numerical-parity and reuse tests deleted without an in-tree successor, leaving author-run VALIDATION.md as the only numerical evidence), plus informational ABI-version (L3), device-pinning (L4) and glue-kernel gate (L5) notes. From #109's own scope the PR is ready for review; the Laya-layer findings should be routed to #71's follow-ups (#72–#74 or the maintainers of the merged series).

[Levius-Fubuki]

Reviewed 6ce188297c7baeaf8c42d3c88e610e6a3a492f58 against base/merge-base 3d4ed19d98a2847241095abb52d11b76c5543214, including all four changed files (+275/-0).

Requesting changes for the two P2 admission issues below: saturation loses the units that permits must later release, and the borrowed-only permit API cannot transfer a reservation into the existing runtime's owned blocking work. Both were independently reproduced against the compiled PR library. The library is currently unwired; these findings do not claim a regression in an existing deployment.

Personally run on macOS arm64 / Rust 1.98.0:

• cargo fmt --all --check: passed.
• cargo clippy --workspace --locked --all-targets --features omni-laya/serve -- -D warnings: passed.
• cargo test --locked -p omni-runtime: 20 ordinary tests and 2 doctests passed.
• cargo test --workspace --locked --features omni-laya/serve: 181 ordinary tests and 2 doctests passed, 0 failed, 28 ignored.
• cargo build --workspace --release --locked --features omni-laya/serve: passed.
• Additional scoped concurrency probe: 100 rounds, 32 competing threads, capacity 4; exactly four admissions and full release in every round.
• Additional probes reproduced the release panic, the dispatch compile failure (E0521), and premature bounded-capacity release when the borrowed permit is retained in an aborted requesting task instead of dispatched work.

The 28 ignored tests remain unverified. No GPU/checkpoint execution or numerical/performance campaign was run; this PR changes a CPU admission library.

Review provenance: canonical system1-omni-review skill, SHA-256 58be3bc60806c4c5af7038a0c6a80fadb94f3360d714556a3e965c8903dde108, with its repository-contracts reference, architecture, runtime contract and CONTRIBUTING read. Remote head rechecked before submission.

[Levius-Fubuki]

[P2] Reject overflowing unit totals instead of saturating the reservation

With a valid maximum unit ceiling, saturation makes an over-capacity total pass the fit check and records fewer units than the returned permits own:

let admission = BoundedAdmission::new(2, Some(usize::MAX));
let first = admission.try_admit(usize::MAX).unwrap();
let second = admission.try_admit(1).unwrap(); // accepted beyond the unit ceiling
 drop(first);
 drop(second); // panics at admission.rs:129

I ran this against the compiled PR library: the second drop panics with a permit releases the units it was admitted with. Replacing Some(usize::MAX) with None reproduces the same panic even though that mode promises to bound requests only. Reversing the drop order also leaves the final permit unable to release its original cost.

The earlier arithmetic fix makes the check and reservation agree, but saturation still loses information required by the exact subtraction in release. Please use checked addition and refuse an overflowing total when a unit bound applies, avoid accumulating unnecessary unit accounting in request-only mode, and cover both drop orders with and without a unit ceiling.

[Levius-Fubuki]

[P2] Provide an owned permit that can be retained by dispatched work

Permit borrows the admission handle, so a caller cannot move a successfully reserved permit into the existing SerialScheduler::run closure, which requires Send + 'static:

async fn dispatch(admission: &BoundedAdmission, scheduler: &SerialScheduler) {
    let permit = admission.try_admit(1).unwrap();
    scheduler.run(move || {
        let _permit = permit;
        Ok(())
    }).await.unwrap();
}

Compiling this against the PR's public API fails with E0521 (admission must outlive 'static). The same ownership constraint prevents handing the permit to spawn_blocking or a long-lived worker queue.

Keeping the borrowed permit in the requesting async task instead of the blocking closure compiles, but breaks the outstanding-work contract on cancellation. I gated the dispatched closure, aborted and awaited the caller, and observed report().depth == 0 plus successful admission of a new request while the original blocking work was still gated. This probe demonstrates an integration workaround; no production consumer is wired to this library yet.

Please provide an owned, non-Clone permit retaining the shared Arc state (or an additional owned-admission API), so the reservation can travel with the work and survive caller cancellation. Add a scheduler integration regression that cancels after dispatch and verifies capacity remains held until execution completes; this does not require adding the deferred worker lifecycle.