fix(serve): the four P0s from the serve audit, two guards, and a 367x cancellation proof on GPU - #2469
Merged
Merged
Conversation
…opies
`test_commutativity_execute_corroborated` failed a full workspace run on a loaded
box:
Expected Corroborated, got: Err(Io(Os { code: 26,
kind: ExecutableFileBusy, message: "Text file busy" }))
`create_mock_apr` writes a shell script, chmods it, and hands the path to code
that spawns it. `fs::write` closes our handle — but a concurrent FORK elsewhere in
the test binary inherits that write fd and holds it until its own exec, and
spawning in that window is ETXTBSY. O_CLOEXEC closes the fd at the child's exec,
not before, so the window is real and only opens under load. That is why this
passes locally and fails when three agent workflows are saturating the machine.
Same class as the aprender-mcp fix earlier today (#2384), in a different crate —
which is the point: the pattern was fixed once and the sibling copy kept it.
FIXED IN THE FIXTURE, NOT IN PRODUCTION. `wait_until_spawnable` retries only
ETXTBSY, bounded to one second, and lives in the test helper. The code under test
still spawns exactly once, as it does in the field — adding a retry to production
to paper over a test-harness race would change shipped behaviour to fix a fixture.
TWO COPIES PATCHED. `create_mock_apr` exists at
`conversion_tests_c.rs:4` AND `conversion_tests_executor.rs:146` — duplicate
implementations of one fixture, 17 call sites between them. Both now wait. The
duplication itself is APR-MONO consolidation debt and is being tracked by the
monorepo sweep; this change does not unify them, because unifying a fixture while
fixing a flake would make the flake fix unreviewable.
`cargo test -p aprender-qa-runner --lib`: passes.
Refs #2384
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Six agents fixed the four verified P0s from #2465 plus two guard-blindness findings; every fix then went to an independent agent whose job was to refute it. All six were mutation-verified. Five SOLID, one NEEDS_WORK (recorded below).
The Q4K P0, proven fixed on GPU
The implementer could only compile-check the CUDA path and said so. The verifier closed that gap: built with
--features cuda-batch, served a real model on the RTX 4090 with the scheduler proven engaged (Q4K GPU inference ready (ALB-095), binary reporting its own SHA), aborted a 400-token/v1/completionsat 5s, then timed the next request — the scheduler thread is serial, so that latency is the abandoned request's remaining GPU work:It also ran a mutation the implementer never did — moving the poll to the bottom of the loop — which also goes RED. So "the poll must be at the top" is enforced, not just asserted in prose.
Corrections to #2465, which I filed
I overstated the blast radius. The Q4K path is gated on
--features cuda-batch; a plain--features cudabuild logsQ4K pool-allocator path declinedand never reaches the decode loop. My ticket claimed five live routes with no mention of that gate.Prefill is still not polled. An already-cancelled request costs
prompt_lenforward passes beforeq4k_decodeis entered. The falsifier's text says "ZERO forward passes" — true ofq4k_decodeonly. It matches the sibling loops (generate_quantized.rspolls after prefill too), so it is a convention rather than a regression, but the assertion claims more than the code delivers.The other three P0s
/v1/completionscould not end a completion —stop_tokensnever reached the generate config. Now funnelled throughcompletion_resp. Honest limit: 2 of 8 backends bypass that funnel, and passingNonestill compiles, so the guarantee is positional-reminder strength, not type-level.p.bytes().map(|b| b as u32)under a// In production, use a proper tokenizercomment. "世界" became six ids naming six unrelated vocabulary entries; ASCII was wrong too. Now uses the server's tokenizer, and an empty prompt is refused rather than handed to the model as empty context."apr"at three sites fix(mcp,code): apr 0.63.0 ran apr 0.60.0 — both subprocess backends resolved a bare apr through PATH #2424 missed, whileapr_bin.rsdocumented that it covered "all eight subprocess tools". Code and comment now agree.Two guards
OPENAI_ROUTESpreviously passed all 15,706 tests. The guard now checks both directions — advertised ⇒ mounted and mounted ⇒ advertised.pv validatepassed a contract naming a nonexistent test, becausestrict_test_binding.rs:57-63skips entries lacking atest:field.One flake fixed along the way
test_commutativity_execute_corroboratedfailed the workspace run withExecutableFileBusy.create_mock_aprwrites a script and hands it to code that spawns it; a concurrent fork inherits the write fd and holds it past our close. Same ETXTBSY class as the aprender-mcp fix earlier today, in a different crate — the pattern was fixed once and the sibling copy kept it. Fixed in the fixture, not in production: the code under test still spawns once, as it does in the field.create_mock_aprexists in two copies (17 call sites); both now wait. Unifying them is consolidation debt tracked separately — doing it here would make the flake fix unreviewable.What is NOT in this batch
fix/serve-completions-eos-and-surface-driftwas dropped. Two agents independently fixed the batch tokenizer and the router's advertised list, and hand-merging two implementations of one fix is how a silent defect ships. Its unique surface-drift work is covered by the route-surface guard. It can land separately if anything is missing.Verification
cargo test -p aprender-serve --lib— 15,654 passed, 0 failedcargo test -p aprender-qa-runner --lib— 1,896 passed, 0 failedscripts/check_assertions_exclude.sh— clean, delta 0cargo fmt --all --checkcleanA full
cargo test --workspace --libwas attempted twice and both runs were killed by resource exhaustion (SIGKILLonlibc,syn,serde_core— third-party crates that compile fine) while three agent workflows saturated the box: load average 21-52, 19 concurrent cargo processes. Rather than quote a number from a killed run, the per-crate figures above are what completed. CI runs the full workspace on this PR regardless.Closes #2465