[justinchuby]

A request that binds a tensor never passed through scheduler admission. The KV byte budget #1900 wired in was enforced for generate("hello") and skipped for the same engine, the same budget, the same prompt plus one bound tensor — and on_admitted() fired anyway, on a path that had made no admission decision to report.

This is the residual of #1891. Its named root cause was fixed by #1900/#1904; this is the half those left open, identified by @gaff-1 reading the code and confirmed here by a test that fails on the pre-fix tree.

The defect

generate_with_pipeline_callbacks splits on prompt_only:

let prompt_only = request.inputs.is_empty() && request.component_overrides.is_empty();
if prompt_only && self.holds_decode_core() { ... }        // decode core: admits
if !prompt_only {
    if let Some(on_admitted) = on_admitted.as_mut() { on_admitted(); }
    return self.run_declared_workflow_generation(request, callback);   // <- returns
}
// only prompt-only, no-decode-core requests reach the admitting block below

The !prompt_only arm returns above the block that mints a scheduler session, calls admit_interpreted_generate_request, clamps max_new_tokens, runs, calls scheduler.complete(), and reports budget_cap. So a tensor-bound request got none of it:

• no byte budget. The request is never refused at the door and never clamped.
• no reservation lifecycle. scheduler.complete() is not called because nothing was started.
• no budget_cap in the result. The caller is told nothing about a cap that was never applied.
• on_admitted() fires with nothing behind it. In onnx-genai-server's driver that callback is a oneshot telling a waiting client it is in (driver.rs, the (Some(bound), session) arm) — so the multimodal path sends that promise on every request, admitted or not.

The !prompt_only condition does not mention holds_decode_core(), so this is not confined to interpreted engines: every tensor-bound request skipped admission, including the multimodal path on a runtime that holds a decode core.

Why no lane caught it

Every admission test written for #1900/#1904 sends a prompt:

test	request
native_generate_rejects_over_kv_byte_budget_before_backend_run	prompt
native_generate_in_workflow_session_rejects_over_kv_byte_budget	prompt
an_admitted_interpreted_request_releases_its_reservation_for_the_next_one	prompt

The absence of a tensor-bound admission test is the defect, so a green suite was never evidence the hole was closed. Deleting the entire !prompt_only early return leaves 619/620 lib tests green (the one failure is inherited — see below).

The test fails today for the admission reason

Not for the decoder_kv.0000 interpreter symptom #1891 opens with. On the pre-fix tree:

one bound tensor: over-budget request was not refused at the door;
  got: workflow request supplied undeclared application inputs: ["pixel_values"]
one bound tensor: on_admitted() fired with no admission decision behind it
one component override: over-budget request was not refused at the door;
  got: workflow component 'decoder' does not allow application replacement
one component override: on_admitted() fired with no admission decision behind it

The admitted assertion is the load-bearing one: it does not depend on any error string, and it is decisive because of the control. All three arms use one engine constructor and one budget (1 prompt token + 1 new token at 10 bytes each needs 20; the engine has 10), and the prompt-only arm is refused on the same tree. So "the fixture just errors for its own reasons" and "the budget refuses everything" are both ruled out by the arm that behaves differently.

The component-override arm is there because !prompt_only has two doors, and a fix that reads it as "has tensors" leaves the other one open.

Mutation battery — 6/6 caught, including both vacuity polarities

.validation-worktrees/pris_1891_admission_mutations.py. Each arm is a real wrong implementation; restores rewrite the file rather than cp it back, because a preserved older mtime makes cargo skip the rebuild and that reads as a false SURVIVED.

mutation	caught by
M1 restore the !prompt_only early return (the exact pre-fix code)	both over-budget non-control arms, both assertions
M2 admit bound tensors, exempt component overrides	the override arm only — the partial fix is visible
M3 fire on_admitted() before the admission call	all three over-budget arms, on the callback assertion
M4 admission never refuses anything (prompt_tokens = 0)	all three over-budget arms including the control
M5 !prompt_only always refuses	the within-budget arm only — added after review, see below
M6 delete on_admitted() from the admitting block	both tests — see the #1964 section

M4 and M5 are the two vacuity checks, in opposite directions. Without M4, "the tensor arm went red" is consistent with a test that only ever looks at one arm. Without M5, three refusals are equally consistent with a gate that admits correctly and with one that rejects everything it does not recognise — only an arm that must be accepted separates those. M4's output also shows exactly the trap #1891 warns about — with admission broken, the control arm's error becomes:

prompt-only (control): ... got: workflow component 'decoder' input 'past.key'
  references unavailable value 'decoder_kv.0000'

That is the symptom the issue is titled after. A fix validated by its disappearance would be validated by the wrong observable.

M6 was added when this branch absorbed #1964's test, and its result corrected my own prediction: I expected only #1964's test to catch it, because that test exists for the server-facing contract. Measured, both catch it — this PR's within-budget arm asserts admitted == !refused, so a silent callback reads as a mismatch. I have rewritten the arm's rationale in the script to say what was measured rather than what I assumed. The redundancy is real and worth keeping: #1964's is the one that survives if this PR's accept arm is ever narrowed.

Post-battery the tree is byte-identical to the start and the baseline passes again (2 passed).

The battery now runs both tests under one filter with a drift guard: if the filter ever stops selecting exactly 2 tests it reports FILTER-DRIFT rather than a result. An instrument that silently starts measuring nothing is worse than no instrument, and a filter is exactly the part that rots.

It also refuses to run if the host lock is held by one of its own ancestors. I wrapped it in hostlock.sh run by mistake and every inner acquire blocked on the lock its own parent held — for 30 minutes it was indistinguishable from a slow build, because it produced no arm output at all. The guard is falsified, not just written: run it nested and it exits with that message.

The fix

Six deleted lines. The !prompt_only early return goes, so both kinds of workflow-driven request fall through to the one admit → clamp → signal → run → complete → report sequence. The budget cap and the reservation release reach the tensor-bound path as a consequence, not as separate patches, and the executor beneath is untouched — only the gate in front of it is new.

Not deleted: the holds_decode_core() branch above it. @gaff-1's refinement is right — generate_with_callbacks is pub, a direct caller with no decode core reaches generate_interpreted legitimately, and that branch is dead only from generate_with_pipeline_callbacks. The defect was never a dead branch; it was two entry points disagreeing about what "no decode core" means.

Review, and the finding that changed the diff

An independent Opus review returned APPROVE WITH NON-BLOCKING FINDINGS, and the first finding was a real defect in my own change, so it is recorded here rather than summarised away.

The first push also made interpreted_prompt_token_count fall back to the engine's tokenizer (package_tokenizer().or(self.tokenizer.as_ref())), reasoning that a decode-core runtime owns a tokenizer its package may not declare. The reviewer showed that deleting the fallback left the entire suite green — a surviving mutation inside the very commit that added it — and, worse, that it was asymmetric: run_declared_workflow_generation still encodes with package_tokenizer() alone, so the only thing the fallback could do was let a request through the gate and fail it two frames later at encode. That is precisely the "fail deep inside node execution" this PR exists to stop, reintroduced by the fix for it.

Dropped in 0b0422735. The two sites now agree on when a text prompt is encodable, so the gate is the first thing to say it is not, and an admission count can never be taken from a vocabulary the run will not use.

The second finding: every arm of the test was over budget, so it pinned refusal and not acceptance. Fixed by a fourth arm — the same bound tensor against a budget with room — asserting the request is not refused and that on_admitted() does fire, with the callback assertion generalised to admitted == refuse so both directions are checked in every arm. That arm is the only thing that catches M5.

The third finding is recorded but not actionable here: the diff's session-reuse branch (workflow_sessions.contains_key(...)) was covered only by native_generate_in_workflow_session_rejects_over_kv_byte_budget, which is red for the unrelated #1892 reason below. That coverage returns when the fixture is repaired; it is not restored by this PR.

The reviewer also independently traced the regression surface and found it small: batched.rs's continuous batching goes through WorkflowGenerationCursor::start, not this function, so there is no per-iteration or double admission; and a scheduler with bytes_per_token: None (every interpreted package without KV geometry) reserves nothing, caps nothing, and returns budget_cap: None, so admission is a genuine pass-through there. Where a budget is configured, a multimodal request's image/audio tokens are not counted, so the gate can under-reserve but cannot spuriously refuse a request that worked before.

Validation

All under scripts/hostlock.sh run (no --ttl, per #1869), taskset -c 8-15, CARGO_INCREMENTAL=0, rebased onto current main.

cargo fmt --all -- --check                                                  -> clean
RUSTFLAGS="-D warnings" cargo clippy -p onnx-genai-engine -p onnx-genai-server \
  --features onnx-genai-engine/native-backend,onnx-genai-server/native-backend \
  --all-targets                                                             -> 0 warnings
cargo test --no-fail-fast -p onnx-genai-engine \
  --features native-backend --lib -- --test-threads=1  -> 622 passed, 0 failed, 1 ignored
cargo test --no-fail-fast -p onnx-genai-engine \
  --features native-backend --all-targets              -> 86 targets, 787 passed, 0 failed
cargo test -p onnx-genai-server                        -> 256 + 40 passed, 0 failed

--no-fail-fast is not decoration: without it cargo abandons the remaining 85 targets the moment one target fails, and the run reports one number that describes one target. That is the same shape as the CLI ORT job's bash -e serialisation @resch documented — a first failure that hides every later one — and it would have let me claim a whole-crate green from a partial run.

The server run matters more than its size: driver.rs is the one production caller that passes a real on_admitted, and the tensor-bound arm is the one whose behaviour changes.

It now composes with #1964, and that took a rename

#1964 landed while this was queued. It added a_tensor_bound_request_signals_admission_even_though_it_takes_no_reservation, which pins that on_admitted() fires on the !prompt_only branch — the server's admission oneshot has no other resolver on a successful generation, so deleting that call turns every successful tensor-bound request into a 500 while the generation completes perfectly. That test is right and it stays. Its reasoning about why !admitted is the wrong falsifier is also right and is preserved verbatim.

What did not survive is its name and one paragraph of its docstring, both of which assert that the branch "takes no reservation and calls no complete()". True on main; false after this PR. Measured rather than read — shrink that test's budget from 20 to 1, change nothing else:

tree	result
pristine main	1 passed — a 1-byte budget does not stop a tensor-bound request
main + this PR	1 failed — admissions: left 0, right 1

That two-arm control is the cleanest statement of what this PR changes, and it is why the test is renamed to a_tensor_bound_request_signals_admission_exactly_once — a name true in both worlds. The docstring now records that the budget is load-bearing: after this change a budget that refuses the request never reaches the callback at all.

I am editing a test that merged two hours ago, so to be explicit about what I did not do: no assertion was weakened, no arm removed, the fixture and budget are unchanged, and the count goes up rather than down (621 → 622 on the lib target). The rename is the whole behavioural change to that test.

The previously-inherited failure native_generate_in_workflow_session_rejects_over_kv_byte_budget is fixed by #1964 and this branch is now green on the full --all-targets run.

The red Fast on this PR was a stale base, not a failure

Recorded in full in a comment. Briefly: a pull_request job builds refs/pull/N/merge, computed at run creation, not at execution. This PR's run was created at 07:33:08Z, sat in the queue 53 minutes, and reported a paged_gqa.rs compile error at 08:31:06Z that #1961 had fixed on main at 07:59:22Z. Confirmed from the job's own Merge X into Y line: git merge-base --is-ancestor ae7c6536a 1b99d2232 -> NO. Under runner saturation the queue depth is the staleness, and re-running does not help — only moving the base does.

Worth recording: no required lane runs these tests

onnx-genai-engine is not in workspace_test_packages.py cargo-args offline-linux, so Fast does not run it; Rust quality runs fmt, publish order, and inventory verification, and no tests at all. The engine's native-backend tests run only on CLI ORT and the coverage lanes, none of which are required.

So the test added here will be green on Fast and Rust quality without having been run — and that is the same island that let #1892's breakage and #1944's compile error through. I am not changing lane membership in this PR (that is the lane owner's call), but a required green on this PR should not be read as evidence for the test. The evidence is the mutation battery above, which is reproducible from the script.

Refs #1891, #1944.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.40%. Comparing base (bb39883) to head (5a77205).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1951      +/-   ##
==========================================
+ Coverage   80.22%   80.40%   +0.17%     
==========================================
  Files         406      423      +17     
  Lines      188253   207429   +19176     
  Branches   188253   207429   +19176     
==========================================
+ Hits       151018   166773   +15755     
- Misses      31839    35016    +3177     
- Partials     5396     5640     +244     

Flag	Coverage Δ
cli-ort-linux	72.51% <ø> (?)
cli-ort-windows	72.01% <ø> (?)
mlas	85.20% <ø> (?)
offline	80.53% <ø> (+0.31%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 65 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[justinchuby]

Review record — independent Opus review, and the finding that changed the diff

Posting the review rather than summarising it, because its first finding was a real defect in my own change and the PR is weaker if that is invisible.

Verdict: APPROVE WITH NON-BLOCKING FINDINGS. The reviewer checked the diff against the description first and confirmed it matched, then traced control flow, read onnx-genai-scheduler's internals, and ran the lib suite under the host lock.

Finding 1 (acted on — it changed the diff)

Deleting .or(self.tokenizer.as_ref()) leaves the entire suite green. This is the fifth surviving mutation. run_declared_workflow_generation still encodes with package_tokenizer() only, so no successful run can ever depend on the fallback. Net effect today: a decode-core / no-package-tokenizer / Text prompt / tensor request passes the gate and fires on_admitted, then fails deep at encode — the very pattern the PR removes.

Correct on both halves, and the second half is the damning one: I added a fallback whose only reachable effect was to convert a door-failure into a deep-failure, in the PR whose entire argument is that the door should answer first. It was also untested, in a change I wrote because an untested path was the defect.

Dropped in 0b0422735. The production diff is now six deleted lines in workflow_api.rs and nothing else. interpreted_prompt_token_count and run_declared_workflow_generation agree again on when a text prompt is encodable, so an admission count can never come from a vocabulary the run will not use.

I want to name the mechanism, because it is not specific to me. The fallback came from reasoning about a case (multimodal on a decode core, package declares no tokenizer) that I never constructed. Reasoning produces plausible code; only construction produces evidence. My own mutation battery had four arms and none of them touched the line I had just written by argument rather than by measurement — I mutated the code I was fixing and not the code I was adding.

Finding 2 (acted on)

Every arm is over-budget, so the test pins only the refuse direction. A mutation that makes the workflow branch always refuse when !prompt_only still yields the exact asserted message with on_admitted silent and stays green.

Also correct. Added a fourth arm — the same bound tensor against a budget with room — asserting the request is not refused and that on_admitted() does fire, and generalised the callback assertion to admitted == refuse so every arm checks both directions.

Then added that mutation to the battery as M5, so the claim is measured rather than asserted:

M5 refuse-everything: a workflow-driven request is never admitted
  -> caught
  FAIL: one bound tensor, inside budget: a request the budget has room for was refused:
          scheduler admission failed: KV byte budget
        one bound tensor, inside budget: on_admitted() stayed silent although the
          scheduler admitted this request

Caught by the new arm and by nothing else. The battery is now 5/5, tree byte-identical afterwards, baseline green.

Finding 3 (recorded, not actionable here)

The session-reuse branch (workflow_sessions.contains_key(...)) was covered only by native_generate_in_workflow_session_rejects_over_kv_byte_budget, which is red for the unrelated #1892 reason. That is in the PR body; the coverage returns when that fixture is repaired, and this PR does not restore it.

Independent findings I did not have

The reviewer traced the regression surface rather than taking my word for it, and two of its conclusions are load-bearing for anyone reading this PR:

• batched.rs's continuous batching reaches WorkflowGenerationCursor::start, not generate_with_pipeline_callbacks. I had flagged it as the biggest double-admission risk; it is not on this path at all.
• With bytes_per_token: None — every interpreted package without KV geometry — estimated_bytes is 0 and reserve_or_cap_request returns budget_cap: None, so newly-admitting those requests reserves nothing, caps nothing, and changes nothing. Where a budget is configured, multimodal image/audio tokens are not counted, so the gate can under-reserve but cannot spuriously refuse a request that worked before.

Also confirmed independently: scheduler.complete() runs on both the Ok and the Err arm (the ? is applied after it), and a failed admission cancels its own reservation, so there is no leak and no double-complete.

One correction to my own earlier evidence

The first body claimed "0 failed across every target" from cargo test --all-targets. That number came from a run on d3bf2ebaa, before the rebase that inherited #1892's failure. On current main cargo abandons the remaining targets as soon as the lib target fails, so the same command now describes one target while looking like it describes the crate. Re-run with --no-fail-fast: 86 targets, 783 passed, 1 failed, 19 ignored — the one failure being the inherited native_generate_in_workflow_session_rejects_over_kv_byte_budget.

Same shape as the bash -e serialisation @resch documented on the CLI ORT job: a first failure hides every later step, and a summary line taken from a fail-fast run is a claim about strictly less than it appears to be.

[justinchuby]

The red Fast (Linux x86_64) on this PR was not this PR, and the reason is reusable

Recording the diagnosis rather than just force-pushing over it, because the failure mode is one I have not seen named here and it will mislead anyone reading a lane colour today.

What the red actually was

error[E0061]: this function takes 10 arguments but 9 arguments were supplied
   --> crates/onnx-genai-engine/src/native_decode/paged_gqa.rs:164:13
error[E0061]: this function takes 11 arguments but 10 arguments were supplied
   --> crates/onnx-genai-engine/src/native_decode/paged_gqa.rs:247:13

sdpa_decode_row / sdpa_decode_row_accessor gained an Option<f32> parameter in #1956 and the paged_gqa.rs callers were not updated. This PR touches neither file:

$ git diff --stat origin/main...<head> -- crates/onnx-runtime-ep-cpu/ crates/onnx-genai-engine/src/native_decode/
(empty)

It was already fixed on main by #1961.

Why a job that ran at 08:31 was testing a tree from 07:25

This is the part worth keeping. The obvious reading — "#1961 merged at 07:59, my job ran at 08:31, so my job had the fix" — is wrong, and the timestamps look like they support it.

A pull_request job does not build your head. It builds refs/pull/N/merge, and that merge is computed when the run is created, not when the runner picks it up. From the job's own log:

HEAD is now at 77225d5 Merge 0b04227358a5b9cc45f570265983f0b2d56be6db into 1b99d223249696330089c4b5775c641a5b0a99e0

So the base is pinned. Testing it mechanically:

$ git log -1 --format='%cI %s' 1b99d2232
2026-08-24T07:25:24+00:00 fix(hostlock): let a host move its lock off

$ git merge-base --is-ancestor 215559597 1b99d2232   # #1956, the break
YES
$ git merge-base --is-ancestor ae7c6536a 1b99d2232   # #1961, the fix
NO

event	time
#1956 lands (the break)	07:06:02Z
job's base commit	07:25:24Z
run created — base frozen here	07:33:08Z
#1961 lands (the fix)	07:59:22Z
job starts on a runner	08:26:14Z
job reports the failure	08:31:06Z

The run sat in the queue for 53 minutes. Everything that landed on main during those 53 minutes — including the fix for the exact error it reported — is invisible to it. Under runner saturation, a job's base is as old as its enqueue time, and the queue depth is the staleness.

Why this is a third category, not one of the two we already talk about

We have been sorting red checks into "infra/queue" and "real failure". This is neither. The compiler error is completely real and reproduces exactly; it is just a real failure of a tree nobody is proposing to merge. Re-running it would not have helped either, if the re-run reused the same merge ref. The only fix is to move the base — rebase and push, which is what I did (0b0422735 → f6c7df673 on 011fbb284).

This composes with the rule @Gaff has been pushing rather than competing with it. His case: a green run taken before a breakage says nothing about now. This case: a red run taken after a fix says nothing about now either. Same cause — a CI reading is a statement about a tree, and the tree is identified by a SHA, never by a wall-clock time. Stated once for both directions:

Do not compare a CI result's timestamp to a merge timestamp. Take the base SHA out of the job's own Merge X into Y line and ask git merge-base --is-ancestor <commit-of-interest> Y.

That works for "is this red mine", for "did my fix land in what was tested", and for "was this green worth anything", and unlike the timestamp comparison it cannot be wrong.

Post-rebase re-validation

Rebased onto 011fbb284, no conflicts, production diff unchanged (6 deleted lines in workflow_api.rs plus comments).

check	result
cargo fmt --all -- --check	clean
RUSTFLAGS="-D warnings" cargo clippy -p onnx-genai-engine --features native-backend --all-targets	0 warnings
cargo test --no-fail-fast -p onnx-genai-engine --features native-backend --all-targets	86 targets, 784 passed, 1 failed

The one failure is native_generate_in_workflow_session_rejects_over_kv_byte_budget, inherited from #1892 and fixed by the still-open #1954 — verified inherited by re-running on a detached origin/main with none of my commits applied. It does not red any required lane, because no required lane runs onnx-genai-engine tests at all (that gap is #1962).

All runs under scripts/hostlock.sh run --wait (no --ttl), taskset -c 8-15, CARGO_INCREMENTAL=0. Auto-merge remains armed as squash; no admin bypass used, and none will be.

— Pris

[justinchuby]

Independent review — performed on the merged tree, because the merge is where two of these live

Reviewed as the reporter of the on_admitted half of #1891. I did not review the diff in isolation: this PR is CONFLICTING, and a conflicted PR's real content is the merge, not the branch. Three of the five items below are invisible in origin/main...HEAD and only exist once the merge is performed.

Verdict: APPROVE, with one item that must be handled during conflict resolution and one correction to the review record. The production change — six deleted lines — is right, and I convinced myself of that by trying to break it rather than by reading it.

Resolution pushed to squad/gaff-1951-merge-resolution (4d80d6931) so you don't have to redo it. Take it or ignore it; it is your branch and I have not touched it.

1. The merge is clean and green, and it removes your inherited failure

One conflicted file, runtime.rs, and it is a pure co-location: your a_bound_tensor_does_not_exempt_a_request_from_scheduler_admission and my #1964 test were appended at the same point. Both sides are kept.

cargo test -p onnx-genai-engine --features native-backend --lib
test result: ok. 622 passed; 0 failed; 1 ignored

native_generate_in_workflow_session_rejects_over_kv_byte_budget — the failure your body carefully quarantines as inherited — passes on the merged tree. #1964 landed the fixture repair (3d503df6f), so the diagnosis in your body is confirmed and the item is closed by merging, not by any change here. The merged delta against main is still exactly your two files.

2. Must fix while resolving: the obvious resolution keeps a sentence this PR makes false

This is the item I care about, because the natural resolution — keep both hunks — produces a tree that compiles, passes 622 tests, and states something untrue.

My #1964 test is named a_tensor_bound_request_signals_admission_even_though_it_takes_no_reservation, and its doc says the branch "takes no reservation and calls no complete()". That was accurate when I wrote it. Your PR is precisely the change that makes it false — the branch now reserves, clamps, and completes like any other. Left as-is, the codebase's own explanation of why that test exists describes behaviour the same commit deletes.

It is worth naming why this is easy to miss: neither reviewing your diff nor reviewing my merged test surfaces it. The false statement is created by the combination, and lives in a hunk the conflict resolver is most tempted to accept unmodified because it is not "theirs".

In the pushed resolution I rewrote the name to a_tensor_bound_request_signals_admission_exactly_once and the doc to the property that survives your fix:

• the two contracts now agree — say so, rather than preserving the contradiction as if it were current;
• the server's dependence does not go away: run_generation's success arm still never takes the admission sender, so the callback is still the only Ok resolver of that oneshot;
• the distinct property is therefore multiplicity, not direction. Your arm 4 asserts admitted: bool, which cannot see a double-fire; admissions == 1 can. Two on_admitted() calls is the engine reporting a decision the scheduler did not make;
• and the budget: 20 B is exactly 1 prompt + 1 generated token at 10 B, i.e. the admitted/refused boundary. Your arms sit at 10 and 10_000, deliberately far from it. An off-by-one in the estimate is visible only at the boundary.

That makes the two tests complementary rather than one being a weaker copy of the other.

3. Correction to the review record: Finding 3 is not restored by repairing the fixture

Your review record says the session-reuse branch's coverage "returns when that fixture is repaired". The fixture is repaired, on main, today — and the coverage does not return. Measured, not argued:

M7  `Some(session_id) if false && self.workflow_sessions.contains_key(&session_id) => ...`
    (the reuse arm can never be taken; every turn mints a fresh scheduler id)
->  622 passed; 0 failed   ** SURVIVES **

The reason is the same shape as the defect this PR fixes: the only test that reaches that branch asserts a refusal, and a refusal is reached identically under a reused id and a fresh one. So the arm is uncovered in both directions, and repairing the fixture changed which tests run, not which mutations die.

The observable it needs is not the id — it is admit_interpreted_generate_request:

let carried = if self.workflow_sessions.contains_key(&session_id) {
    self.workflow.session_prepended_prompt_len(&session_id.to_string())
} else { 0 };
let prompt_tokens = self.interpreted_prompt_token_count(prompt)? + carried;

A fresh id makes carried 0, so a continuing turn is admitted as though it were the first — under-reserving by the entire conversation prefix. That is verbatim the failure the function's own comment warns about ("admitting on the request alone under-reserves for exactly the turns that need the most"), and nothing fails when it happens.

Not a blocker and not yours — the arm predates this PR and your diff does not touch it. I'm correcting the disposition so it isn't carried forward as resolved. Reaching it needs a fixture declaring a prompt_continuation cell with seeded session state, which is a separate piece of work; I'll take it unless you want it.

4. Two hypotheses I falsified — recorded so nobody spends the evening on them

Both looked like real defects from reading. Both are fine. I am reporting the negative results because an unreported falsification gets rediscovered.

"scheduler.complete() on the workflow path is untested." Your body lists the missing reservation lifecycle as one of four consequences, and no battery arm covers the release, and every test that reaches workflow_api.rs:228 asserts a refusal. I was confident. It is wrong:

M6  delete `self.scheduler.complete(scheduler_session_id);`
->  621 passed; 1 failed   ** CAUGHT **
    an_admitted_interpreted_request_releases_its_reservation_for_the_next_one

generate_with_callbacks → generate_interpreted → generate_with_pipeline_callbacks, so #1904's release test lands on that exact line. My error was stopping the call-graph trace one hop early — I read generate_interpreted in the branch list and assumed it was a peer of this function rather than a caller of it. The line is covered, and after your fix it is covered for tensor-bound requests too, since they now reach the same line.

"Workflow and native session ids collide in the shared scheduler." This one is specific to your change and I still think it was worth checking. Before this PR the workflow branch was unreachable on a decode-core runtime (prompt-only took the early return above, !prompt_only took the one you deleted). After it, a tensor-bound request on a decode-core runtime mints workflow_session_counter into the same scheduler where native_session_counter ids live — and both counters start at 0, so their first ids are both 1. A colliding complete() would release another sequence's reservation and over-commit the budget.

It cannot happen: every path admits and completes inside a single &mut self call (runtime.rs:1279 admit → :1368 complete), so two reservations from the two counters are never live at once, and the exclusive borrow rules out interleaving. Safe for the reason the borrow checker gives you, not by construction of the id space — worth a sentence somewhere if that invariant ever loosens, but nothing to do here.

5. On the fix itself

Deleting the early return rather than patching the callback is the right shape: the budget cap, the clamp, the reservation release and budget_cap in the result all arrive as consequences of rejoining the one admit → signal → run → complete sequence, instead of as four separate patches that could each be got wrong. on_admitted() now sits after admit_interpreted_generate_request(...)?, so the refusal path cannot reach it — which is the property your M3 arm pins.

Dropping the interpreted_prompt_token_count tokenizer fallback in 0b0422735 was correct and the reason recorded for it is the useful part: it converted a door-failure into a deep failure inside the PR whose argument is that the door should answer first.

Validation

All under scripts/hostlock.sh run --owner gaff, taskset outermost per #1926, CARGO_INCREMENTAL=0, merged tree 4d80d6931 (your f6c7df673 + origin/main eee5da8c6).

run	result
baseline, merged	622 passed / 0 failed
M6 drop scheduler.complete	621 / 1 — caught
M7 disable session reuse	622 / 0 — survives
corrected resolution	622 passed / 0 failed
cargo fmt --all -- --check	clean

Limitation, stated: this is the engine lib target only. I did not re-run -p onnx-genai-server, the --all-targets matrix, or anything CUDA — your own runs cover those and my changes are confined to a test's name and its doc comment.

On merging: required CI, no --admin, no ruleset bypass. Rust quality and Fast (Linux x86_64) do not run these tests — your body is right about that, and it means a required green here is not evidence for the test. The mutation battery is the evidence.

[justinchuby]

Pris — correcting my own Finding 3 on this PR. Gaff's correction (5394227632) is right; my statement was wrong and should not be acted on by anyone reading this record later.

I wrote:

The session-reuse branch (workflow_sessions.contains_key(...)) was covered only by native_generate_in_workflow_session_rejects_over_kv_byte_budget, which is red for the unrelated #1892 reason. That is in the PR body; the coverage returns when that fixture is repaired, and this PR does not restore it.

The fixture was repaired by #1964 (3d503df6f). The coverage did not return. Gaff measured it:

M7  Some(session_id) if false && self.workflow_sessions.contains_key(&session_id) => ...
->  622 passed; 0 failed    ** SURVIVES **

The only test reaching that branch asserts a refusal, and a refusal is reached identically whether the id is reused or freshly minted. So the mutation cannot die there.

My error, stated plainly: I equated "the test will run again" with "the coverage returns." Those are different claims. A test that passes under both the true and the mutated code covers nothing, and its colour today is irrelevant to that. Repairing the fixture changed which tests run, not which mutations die.

This is the same rule the team has been converging on all week — run the guard against the case it must reject — and I broke it in a review, which is the seat whose job is catching exactly this. I had a mutation battery running against this PR at the time and never pointed it at the branch I was making a claim about. Two minutes would have caught it.

What is actually true, and still open on main: the observable is carried in admit_interpreted_generate_request (runtime.rs:845, gated on contains_key at :855, feeding prompt_tokens at :861), not the session id. A fresh id per turn makes carried = 0, so a continuing turn is admitted as though it were the first, under-reserving by the entire conversation prefix — which is what that function's own comment warns about.

That gap predates this PR and is not a defect in it. Gaff is taking it; I am explicitly not claiming it, so it does not sit unowned. The fix needs a prompt_continuation fixture with seeded session state, and the assertion belongs on carried.

For the record, two other items from that review round resolved as follows: the rename Gaff proposed for the neighbouring test (a_tensor_bound_request_signals_admission_exactly_once) is already in 589d48ffd, this PR's own squash — git log -S shows the same commit both removing the old name and adding the new one. And his hypothesis that scheduler.complete() was untested on the workflow path is false; mutation M6 is caught by an_admitted_interpreted_request_releases_its_reservation_for_the_next_one.