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

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

Independent review — PR #149 "decider: bound pending requests and enforce HTTP deadlines"

Reviewer: independent code review (ZCode agent). Not posted to GitHub; no repo modifications. Local review only.

Provenance

Note: the skill references CONTRIBUTING.md#large-code-changes, but CONTRIBUTING.md at both origin/main and the PR head contains no such section (headings verified). The PR is below any plausible large-change threshold anyway, and the author supplies the split regardless.

Diff inventory and authored/total split

Full PR diff vs merge base: 14 files, +1308/−23. Decomposition verified by per-commit diffs:

• Inherited #109 prerequisite (merge 9412f71): src/runtime/src/admission.rs (+208), tests/runtime/admission.rs (+302), src/runtime/src/lib.rs (+1), src/runtime/Cargo.toml (+4) = 4 files, +515.
• Authored integration commit d463ed5: 12 files, +793/−23 — decider runtime.rs (new, +126), serve.rs (+121/−23), engine.rs (+26), executor.rs (+13), lib.rs (+1), Cargo.toml (+9); runtime control.rs (new, +73), lib.rs::run_with_context (+21), Cargo.toml (+4); tests tests/decider/runtime.rs (+69), tests/decider/serve_runtime.rs (+118), tests/runtime/control.rs (+235).
• The #109 content is byte-identical to approved #109 head 56ce14a (git diff 56ce14a origin/pr/149 -- src/runtime/src/admission.rs tests/runtime/admission.rs is empty). "Unchanged prerequisite" claim verified.
• No generated artifacts, data dumps, logs or plans committed. Cargo.lock unchanged despite the dev-dep addition (tokio io-util already in the lockfile graph). #[path]-included tests/decider/serve_runtime.rs follows the existing declared-target pattern (batching.rs etc. already declared [[test]]); it is a maintained test, not run output.

The PR body's stated split ("12 files, 793 additions / 23 deletions" integration; "14 files, 1,331 changed text lines total") matches my computed numbers exactly.

Findings

Medium

M1 — src/runtime/README.md (the normative runtime contract) is now stale relative to shipped capability. The contract still says: "Queue-length limits, token budgets, compatibility grouping, shared processing orchestration and GPU batching remain planned." This PR ships opt-in queue-length limits and token budgets (admission::BoundedAdmission) and adds a second new public module (control::ExecutionContext / ExecutionStopped / CancellationGuard) plus a new public SerialScheduler::run_with_context, none of which the contract describes. The relationship between the scheduler's existing execution-unit admission (SerialSemaphore, one unit) and the worker-level BoundedAdmission request/token budgets — two coexisting admission layers — is documented only in module doc comments. docs/architecture.md also says "the scheduler owns queue and admission policy rather than duplicating it inside each model implementation" (scheduling contracts), while the policy entry point (env parsing, admit/reserve sequencing) now lives in the model worker (src/models/decider/native/src/runtime.rs) with the shared type in the runtime — defensible, but exactly the boundary the contract README should state. No behavior bug; a contract-update requirement before or with merge. Note the decider's own module doc in admission.rs/control.rs is otherwise excellent and internally consistent.

Low

L1 — New operator-facing configuration is documented nowhere a user looks. DECIDER_MAX_PENDING_REQUESTS, DECIDER_MAX_PENDING_TOKENS, DECIDER_REQUEST_TIMEOUT_MS and the 503/504 semantics exist only in Rustdoc and the PR description. There is no decider worker README; recipe/decider/ contains only download_weights.py; the decider worker is not mentioned anywhere in docs/ or the top-level README (verified by git grep). Repo pattern documents worker opt-in env behavior in docs/recipes (e.g. CUA_S1_GRAPH=1). Suggest a recipe/decider/README.md (serving, limits, deadline semantics, health fields) as a fast-follow; not a merge blocker for a worker that is already undocumented as a whole.

L2 — A permanently inadmissible request is answered 503 + Retry-After: 1. A request whose processed_tokens() exceeds the entire DECIDER_MAX_PENDING_TOKENS ceiling can never be admitted, yet it receives the transient-overload response (503, "inference capacity exhausted", Retry-After: 1), which invites deterministic retry loops from frontends that honor Retry-After. A 4xx (413/422, "request exceeds configured token budget") would distinguish request-property failures from transient capacity. This follows #109's reviewed refusal→503 mapping and is disclosed in the body, so it is a design note, not a contract violation. Cosmetic sibling nit: the header is inserted lowercase as "retry-after" (valid HTTP/1.1, unconventional casing).

Info / maintainer awareness

• I1 — Strict post-execution deadline check discards completed work. On the success path, infer re-checks the context after finish(rows); if the deadline expired between completion and response, a fully computed body is discarded for a 504 (serve.rs success arm). Deliberate strictness, covered by tests, but it converts "just missed" into "all cost paid, nothing delivered." Fine if intended; flagging so it is a decision rather than an accident.
• I2 — Deadline expiry abandons but does not preempt. timeout_at drops the request future (releasing permits via shared Arc<Permit> and setting cancelled only when the whole handler scope ends), while detached blocking prepare/execution runs to completion and retains capacity until then; queued followers wait behind the doomed unit. This matches the architecture contract ("Once dispatched, the runtime retains admission and captured executor resources until the blocking closure completes") and is disclosed in the body and encoded in tests. Absolute-deadline checks at every boundary (pre-queue, closure start, post-prepare, post-execute) stop anything that has not started.
• I3 — Health metrics are conditional. /health publishes depth/capacity/rejected only when a request bound is configured; token-only deployments publish no queue metrics (documented in code and body). /health itself is not admission-gated — correct per repo pattern.
• I4 — Author-reported human-gate provenance. The post-#138 template's "Required human contributor answers" cites "the contributor's explicit 2026-10-16 development-task instruction" as the answer source. I cannot verify a private instruction; reported as author-asserted with a traceable-source claim, which is what the template requires.

Per-focus analysis

1. BoundedAdmission integration — conforms to approved #109 and the runtime contract.

• The worker uses the approved type unchanged (byte-identical to 56ce14a): try_admit → owned Permit, released exactly once via Drop (Permit is deliberately not Clone/Copy; ExecutionContext::retain wraps in Arc<Permit> so clones share one reservation); oversized costs are refused, never clamped (units > ceiling || held.units > ceiling - units, with a dedicated usize::MAX-cost test); reservations store what was actually reserved; poisoned-lock recovery; checked_sub guards.
• Units: request budget counts 1 per request (acquired before body read, so it covers upload + preparation + queueing + execution); token budget uses PreparedRequest::processed_tokens() = sum of complete row token lengths, including repeated state and Score expansion (verified: it is a plain sum(), distinct from the pre-existing prefix-dedup unique_tokens). Bytes are not a unit here; body size is separately capped by the existing 8 MiB DefaultBodyLimit. Units are the caller's, per the type's contract.
• Permit lifetime through preparation AND execution: the request permit lives in the ExecutionContext that is cloned into the detached spawn_blocking prepare closure and into run_with_context's blocking closure (drop(context) after work()); the token permit is acquired post-prepare and follows the same path. Client disconnect or 504 abandonment drops the async clones while the blocking closure retains its clone until the work ends — the #129 omnijev lesson is directly implemented and tested (cancellation_after_dispatch_keeps_both_reservations_until_work_finishes: after task.abort(), both admissions still held; deadline_covers_preparation_and_returns_504: depth still 1 after the 504, then 0 after the blocking work ends).
• Cancellation before/after dispatch per contract: run_with_context checks before queueing; the blocking closure re-checks at start ("dispatch itself can wait behind unrelated blocking work" — covered by blocking_pool_backlog_checks_deadline_before_starting_work and ..._cancellation_before_starting_work with max_blocking_threads(1)); after dispatch, retention until completion, including panic/error paths (errors_and_panics_release_owned_capacity).
• No deadlock: strict request→token acquisition ordering everywhere; single admission call sites (Runtime::admit, Runtime::reserve_tokens).

2. HTTP deadlines.

• One deadline, started + DECIDER_REQUEST_TIMEOUT_MS, measured from admission; enforced by tokio::time::timeout_at wrapping the entire request future — body upload included (verified with a real socket test: declared Content-Length 100, one byte sent → 504, depth back to 0) — plus absolute-deadline check()s at every cooperative boundary. There is no separate queue-wait deadline; queue wait is inside the same envelope, and a queued unit whose deadline expires while sitting in Tokio's blocking pool is rejected at closure start without executing.
• Expiry mapping follows #109's reviewed pattern: refusal/overload → JSON 503 + Retry-After: 1 (Overloaded); deadline expiry (timeout branch or ExecutionStopped::Deadline) → JSON 504. Errors are typed (Overloaded, ExecutionStopped implement std::error::Error), envelope {"detail": ...} consistent with the existing worker helper; 415/413/422 paths preserved (413 exercised by test).
• Expiry abandons the response and cancels cooperatively; it does not preempt in-flight blocking/device work; permits release exactly once in both the abandon case (async clones drop; blocking clone drops at work end) and the natural-completion case. See I1/I2.
• Deadlines are NOT documented in any recipe/doc (L1); they are documented in code, the health metadata (timeout_ms under runtime_limits), and the PR body.

3. Interaction with the decider's existing paths (#111–#113 content).

• Admission wraps the whole decision uniformly: decide → content-type check → readiness check (both before admission; a non-ready or malformed request consumes no slot) → within_budget (admit) → read body → infer (prepare outside the model mutex → reserve_tokens → execute_with_context). There are no per-mode differences to worry about: the native decider is eager-only and rejects graph mode at load ("Decider eager execution requires CUA_S1_GRAPH unset or 0", executor.rs:202), so there is no Graph capture path a deadline could fire mid-capture. The legacy Executor::execute is preserved unchanged (delegates with a default, deadline-free context), and SerialScheduler::run itself is untouched, so #111/#113 behavior for non-HTTP callers (warmup, Engine::predict, batching tests) is unaffected.
• The token reservation happens after preparation, so a token-bound worker does preparation work for requests it then refuses — unavoidable at this granularity, bounded by the request budget.

4. /health and error envelope. /health is not admission-gated (repo pattern) and now additionally reports depth/capacity/rejected when a request bound is configured (I3). Envelope/status mapping consistent; see focus 2 and L2.

5. Tests.

• Concurrency: the #109-pattern test is present (concurrent_admissions_never_exceed_the_bound_and_all_come_back: 32 threads, Barrier, 100 rounds, capacity 4, peak and leak assertions).
• Deadline/cancel: 6 control tests (expired queued work never executes; post-dispatch cancellation retains both reservations until work finishes; blocking-pool backlog deadline and cancel checks; errors/panics release; cancel-while-waiting releases without dispatch) + 4 HTTP tests (503/Retry-After without reading the body; slow-upload 504 over a real TCP socket with capacity recovery; preparation deadline 504 with depth 1→0; success/413/415 preservation).
• Drain/shutdown: explicitly out of scope ("This PR does not add full worker lifecycle/drain" in the body); no drain tests added. Consistent scoping, noted for the record.
• Weight-free Python (run by me at the extracted head tree): python -m unittest discover -s tests/decider -p 'test_*.py' → 16/16 OK (0.017s). These files are unchanged by the PR; run confirms no regression at head.
• Rust tests: not runnable here (no toolchain, per instructions). Observed CI rust job PASS at the exact head (below); author reports 226 passed / 0 failed / 38 ignored (macOS arm64, --features omni-laya/serve) — author-reported, not independently verified. The 38 ignored include checkpoint/GPU-dependent tests; author's GPU evidence is separate (below).

6. Docs, template, hygiene.

• Docs: no documentation files changed (M1, L1). docs/architecture.md statements remain true as written (the new capability is worker-opt-in, not shared orchestration), but the runtime README is stale (M1).
• Template readiness (post-#138): PRESENT and complete. "Required human contributor answers" — all four questions answered with a named source (2026-10-16 development-task instruction), explicit "PR needed? Yes" decision with rationale, and the non-bugfix evidence slot answered (not applicable + fault-removal probe offered). Self-review checklist: all four boxes checked, with an explicit upfront statement that the checks are agent-executed and do not attest human review — the honest framing the template wants. I did not verify the private instruction (I4).
• Authored vs total: reported and verified (12 files +793/−23 authored; +515 inherited #109; 14 files +1308/−23 total).
• Artifact hygiene: clean. Source, manifests and maintained root-level regression tests only; no plans/logs/reports committed; Cargo.lock untouched; evidence (sealed raw records) is external in a fork release.

Commands run and results

Not run: any cargo command (host has no Rust toolchain — contract honored); no GPU validation (host has none); fork release asset contents not downloaded.

Material unverified checks

1. Rust test/clippy/build execution — covered by observed CI PASS at head, but not independently re-executed; author's "226 passed / 38 ignored" macOS run and separate Linux run are author-reported.
2. GPU HTTP parity and timeout campaign (RTX 4090, 100 parity requests, 10 runtime POST checks incl. slow-upload/active/queued timeouts and capacity-hold-after-timeout) — release exists with 4 sealed assets; contents and results not independently verified; no GPU on this host. This is the only end-to-end evidence that deadlines behave correctly against the real executor (the CPU tests use synthetic closures), so it is the main residual evidence gap.
3. Post-#138 human-gate source — the cited private contributor instruction is unverifiable by me (author-asserted).
4. --features omni-laya/serve variants of the author's checks — the extra feature flag goes beyond the CI baseline; not re-verified here (CI ran the baseline commands and passed).

Verdict

No blocking correctness findings. The integration is faithful to approved #109 (type unchanged, exactly-once release, oversized-refused, slot-held-until-work-ends including disconnect/abandonment), the deadline machinery matches the runtime contract's cancellation/retention rules, the tested behaviors are the right ones (including the two regressions that encode the hard-won lessons), CI is green at the exact head, and the PR body's counts and SHA claims all check out against the live revision. Required before/with merge: update src/runtime/README.md so the contract no longer describes shipped queue-length/token budgets as "planned" and states the two-layer admission relationship (M1); recommended fast-follows: operator docs for the three env vars and overload/timeout semantics (L1), and reconsider 503/Retry-After for permanently oversized requests (L2). Scope is coherent (no drain, no batching, graph modes N/A in the native worker), and template readiness is complete with honest agent-execution disclosure.

Incremental re-check at new head 06e32caa39: one new commit since my review at d463ed544b — "docs(decider): explain runtime admission and HTTP limits" (5 files, +191/−15, docs/mkdocs only); the four reviewed commits are patch-identical (git range-diff, named refs, cat-file verified).

The P2 and L1 from my review are fixed, verified in the diff: src/runtime/README.md now documents the two admission layers and their distinct lifetimes (SerialScheduler FIFO execution vs the worker's BoundedAdmission refusals), the decider's reservation semantics (request reservation covering preparation; token reservation after preparation), the costs-above-budget refusal (never clamped), and control::ExecutionContext's deadline/cancellation structure — resolving the "remain planned" contract staleness. recipe/decider/README.md (+121) and the mkdocs/recipe registrations close the worker-docs absence. Decider Python tests pass at this head; mkdocs build --strict passes (mkdocs.yml touched).

One Low remains, unchanged (code untouched by this commit): a request whose tokens exceed the whole ceiling still gets 503 + Retry-After: 1 — inviting futile retries where a 4xx would fit better. Verdict: COMMENTED (that Low). Standing unverified: Rust suite CI-corroborated only; the RTX 4090 HTTP/parity campaign author-run; evidence release contents not re-derived.

Incremental re-check at new head 6129a0cfec: one new commit since my review at 06e32caa39 — "fix(runtime): skip unit totals for request-only admission" (admission.rs + 39 lines of tests); the five reviewed commits are patch-identical (git range-diff, named refs, cat-file verified).

The new commit is reviewed and sound: with units: None (request-only admission), held.units is no longer maintained — removing the overflow-panic class where unbounded caller costs accumulated toward usize::MAX (the pass-50 ceiling tests exposed exactly this class). With a unit ceiling configured, tracking is unchanged (guarded increment, checked_sub release under is_some()), preserving the approved refusal-not-clamp semantics; Reservation still stores the requested cost. New tests pin the behavior: a usize::MAX-cost permit in request-only mode admits, releases, and re-admits, in both large-first and small-first orderings, plus recovery.

The Low from my review remains open and untouched by this commit: a request whose cost exceeds the whole unit ceiling still gets 503 + Retry-After: 1 rather than a 4xx. Fork CI at this exact head: benchmarks/build/cuda-contract (and rust) pass.

Verdict: COMMENTED (that Low remains). Standing unverified: Rust suite CI-corroborated only; GPU HTTP/parity campaign author-run.

Incremental re-check at new head cb5dd10902: one new commit since my review at 6129a0cfec — "fix(decider): reject requests above the token capacity with 422" — plus a merge of current main (own reviewed commits patch-identical; git range-diff, named refs, cat-file verified; merge resolutions carry the absorbed jemm/decider lineage, consistent with the pass-57 wave verification).

The Low from my review is fixed, verified in the diff: a single request whose processed-token cost exceeds the entire token ceiling now returns 422 {"detail":"request exceeds processed-token capacity"} with no Retry-After header, rejected before execution — while 503 + Retry-After is reserved for temporary request/token contention (docs in README + recipe updated to the same effect, and rejected now counts ceiling-exceeding refusals). New OversizedRequest error type; a request costing exactly the ceiling is still accepted when capacity is free.

Also reviewed: the merge resolutions are docs-only unions consistent with the pass-57 wave verification of this stack.

Fork CI at this exact head: benchmarks/build/cuda-contract (and rust) green. No actionable findings remain. APPROVED. Unverified: the RTX 4090 HTTP/parity campaign remains author-run (no GPU on this host); Rust suite covered by exact-head CI; GPU execution of ignored device tests author-run.

Incremental re-check at new head 428f834fad: the branch merged the decider stack (approved #111/#112/#113 lineage) beneath the reviewed admission/deadline work. Verified: src/runtime/src/admission.rs, lib.rs and Cargo.toml are byte-identical between the approved head cb5dd1090 and this head — the PR's own content is unchanged — and the own-commit series is patch-identical per git range-diff (named refs, cat-file verified). The absorbed decider content is exactly what I approved at 5e340cab24/#113 and 3de420ccf2/#112.

My pass-73 approval at cb5dd1090 carried zero open findings (the L2 oversize→422 Low was fixed by cb5dd109 itself, verified then). CI at this exact head: benchmarks/build/cuda-contract (and rust) pass on the fork.

APPROVED (verdict carries — own content unchanged; absorbed content previously approved). Unverified: GPU HTTP/parity campaign author-run (no GPU on this host); Rust suite covered by exact-head fork CI. Stack-order note: this branch now contains the #111→#113 stack; merge order remains with the maintainer.

State correction: my approval of this PR at this exact head (428f834fad, pass 74) was posted with the wrong review event type (COMMENT instead of APPROVE) due to the same posting-script error — the full approval verdict and verification are in that review body directly below and are unchanged: own admission/deadline content byte-identical to the approved cb5dd1090, zero actionable findings (the oversize→422 Low was fixed by cb5dd109 itself), fork CI green at this head. This review corrects the recorded state to match that verdict. Unverified: GPU HTTP/parity campaign author-run; Rust suite via exact-head CI.

PR #149 incremental review — pass 84 (head 4733042bd)

Verdict: APPROVED. Prior on-record state: APPROVE at 428f834fa (state-correction review, 2026-10-22T10:07Z, correcting the mis-evented pass-74 approval).
Date: 2026-10-23.

Re-pinned state (source inspected)

• Live head re-confirmed: 4733042bd35263aab7e0c6ffabe9be0f5593b977 (git rev-parse origin/pr/149 after fetch; gh pr view 149 agrees). Non-draft, title unchanged ("decider: bound pending requests and enforce HTTP deadlines").
• First-parent chain exactly as pinned, all objects exist (git cat-file -e OK):
  428f834fa → fd93d8d4 (merge) → 2c56521b (merge) → 362ead0c (merge) → 4733042bd (authored).
• Own-series integrity: NO own commit rewritten. All four new commits sit on top of the previously reviewed head 428f834fa; cb5dd1090 and 428f834fa remain ancestors unchanged. Range-diff gate not triggered (no rewritten own range), so no range-diff was required.

Merge hygiene (all three merges)

• fd93d8d4 (parents 428f834fa + 017a71b37 = approved #109 head): MANUAL resolution (merge-tree 9b6654c6 ≠ actual tree d84ef6d0). Every resolution hunk read. Both code conflicts take #109's canonical side (held.units += charged; and .checked_sub(reservation.charged)) over #149's guard-based equivalent — semantically identical (charged=0 when no unit budget). Doc comments are merged wording, verified accurate against the code.
• 2c56521b (parents fd93d8d4 + 782db6f4 "Merge latest shared-prefix executor…"): PURE auto-merge (tree matches git merge-tree --write-tree).
• 362ead0c (parents 2c56521b + b3cd06a2 = #113's authored commit): PURE auto-merge.

Entering content provenance (the key question)

Net delta 428f834fa..4733042bd: 16 files, +649/−58. Attribution by per-merge net diff:

1. Via fd93d8d4 from approved #109 head 017a71b37:
   • src/runtime/src/admission.rs (+42 net): CODE byte-identical to approved 017a71b37. The ONLY divergence is the Reservation struct doc comment (lines 70–74), which replaces #109's known P3 (stale clamping description) with an accurate description of units/charged semantics — verified line-by-line against the code (refusal not clamping at line 133; charged=0 for request-only at 142; release subtracts charged once at 179). NOTE with attribution: #109's P3 is carried in name but actually RESOLVED in this tree by the merge resolution.
   • tests/runtime/admission.rs (+29): clean UNION — #149's own a_request_only_bound_does_not_accumulate_units retained AND #109's paired release-order tests (request_only_admission_releases_{large,small}_cost_first) added. No test lost; overlapping coverage only.
2. Via 362ead0c from #113's authored b3cd06a2 (under parallel #113 review): recipe/decider/validation.md (+3/−1, pins the graph regression to ordered_shape_replay_current_ids_growth_and_bounded_cache, which exists at tests/qwen3_5/graph.rs:7) and tests/qwen3_5/graph_loading.rs (assertions "require merged weights" → "only merged weights", matching the actual message at model.rs:1661). Both files byte-identical to b3cd06a2 (empty diff). Cross-referenced, not re-reviewed in full.
3. Via 2c56521b from the 782db6f4 side (main/#93/#98/#112 lineage): 10 files, +588/−33.
   • Byte-identical to current origin/main (canonical main content): benchmarks/README.md, benchmarks/bench.py (+--score-decimals, Decider zero-fit exception, exec bit restored), tests/benchmarks/test_score_wire.py (+225), src/backends/cuda/contract.md, src/backends/cuda/qwen3_5/gemm.cu.
   • contract.md example "abi_version": 7→4: aligns the PR with current main (origin/main has 4; the 7 was the side-lineage anomaly at 5e340cab). Real manifest is 9; the example's staleness is pre-existing on main, not introduced here.
   • Net-new (not on main) side-branch content, reviewed in full: tests/qwen3_5/model.rs (+226 — child-process profile-refusal test with compiled fake CUDA library; sound), tests/qwen3_5/profile_fake.c (+20 — CPU test double), gemm.cu reduction-scheme selection relocated into a ternary in plan_for (semantics-preserving), src/models/qwen3_5/native/src/model.rs (ensure_fixed_profile() extraction with reworded error message; PR-side ahead of main with consistent docs).
   • docs/architecture.md, src/backends/cuda/qwen3_5/README.md: PR-side ahead of main (Decider paragraphs); only the small merged-weights sentence/ABI rewording entered from the 782db6f side.

Authored commit 4733042bd ("Consolidate Decider recipe navigation")

Docs-only, no code smuggled in: removes the DUPLICATE mkdocs nav entry "Decider native text worker" (a leftover of the earlier additive conflict resolution) and consolidates recipe/README.md's two Decider bullets into one. Head has exactly one nav reference each to recipe/decider/README.md (line 75) and recipe/decider/validation.md (line 76); both files exist. No dead links introduced. mkdocs build --strict passes on the exact head.

Note (not a finding): intermediate state

First-parent intermediate 2c56521b is self-inconsistent: model.rs already said "only merged weights" while graph_loading.rs still asserted "require merged weights" — the profile-refusal test fails at that intermediate commit. 362ead0c (#113's b3cd06a2) fixes it. Head is consistent; bisectability note only.

CI (observed)

At the exact head 4733042bd, BOTH repos' Actions APIs show: rust=success, build=success, benchmarks=success, cuda-contract=success, deploy=skipped (intentional for PRs). ThinkFlowLab and InferMatrixLab runs 39886021650 / 39886021731. Green.

Host checks (commands run by me, miniconda python, no cargo/GPU)

• git worktree add /tmp/pr-review-pass84/wt149 origin/pr/149 (removed after review).
• python -m mkdocs build --strict → PASS (0.40s, zero warnings).
• PYTHONPATH=. python -m pytest tests/benchmarks -q → 65 passed.

PR body / template

Template answers all present (Purpose, Dependency and review scope, Configuration/reproduction, Test plan and results, Demo/evidence, Required human contributor answers, Self-review). The body's top section correctly pins the current head: "Current head 4733042bd…, tree 579d40ad…" — I verified the tree hash equals git rev-parse origin/pr/149^{tree} exactly — and cites the current CI/Docs runs 39886021731/39886021650 as "success at this exact current head" (same runs the Actions APIs return at this head_sha). The evidence release link is re-pinned to pr113149-main-abi9-20261023.

Findings

Zero P0–P3 findings. (An initial P3 candidate — "body pins the old head" — was checked against the actual body and does not hold: the 428f834fa pin sits under the dated historical heading "## PR113 compatibility integration — 2026-10-22", and the body explicitly states "older sections below describe their explicitly pinned historical heads", with the top section carrying the current head, tree hash, and CI links. Dated, explicitly-pinned history is the intended structure; the historical section even honestly notes "this newly combined head needs fresh review; older approvals do not attest to it." Correctly classified as structure, not staleness.)

Informational notes (not findings): #109-attributed admission carry, with #109's known P3 (stale Reservation doc) actually resolved in this tree by the merge resolution — #109 itself still carries it; admission tests unioned with no loss; intermediate 2c56521b is self-inconsistent (test/message mismatch fails at that commit, fixed by the next merge — bisectability note only); main-side content mostly byte-identical to main; contract.md example 7→4 aligns with main.

Prior-finding status: the oversize→422 Low remains fixed (unchanged since cb5dd109); #109's P3 does not regress — it is resolved in this tree; no prior finding regresses.

Verdict

APPROVED — non-draft, zero findings, template answers present, merge resolutions verified (one manual resolution in admission.rs taking #109's canonical form, two pure auto-merges), entering content fully attributed (#109 code byte-identical to its approved head, #113's authored content byte-identical, main-side content reviewed), CI green at the exact head in both repos, mkdocs build --strict and 65 benchmark tests pass locally.

Material unverified checks (approval is not complete validation): A800 GPU/HTTP qualification remains author-reported with the named release archive (no GPU on this host; raw evidence audited by the contributor, not re-derived here); the Rust suite rests on the exact-head CI runs (green), not local execution; the intermediate-commit test inconsistency means a bisect through 2c56521b hits a failing test (head state is consistent).