fix(tests): remove two xfail markers left by the parity merge (route-aware fast-mode + deterministic MoA parallelism) - #426
Merged
Merged
Conversation
Route-aware fast-mode call-site adoption (card t_05fcd58f): converge the remaining request-enforcement call sites onto resolve_fast_mode_capability() instead of the model-only model_supports_fast_mode() wrapper, preserving model-only-route behavior (a widening, not a behavior change). Removes the xfail on tests/cli/test_fast_route_capability.py, which had been red on fork/main since before the merge (latent debt, not merge damage). Deterministic MoA parallelism proof (card t_78a2fa21): test_references_run_in_parallel proved concurrency with a wall-clock stopwatch, which flips under CI's 12-worker parallelism when the scheduler serializes the coroutines. Replaced with an overlap assertion inside call_llm that fails deterministically on serialization. RED-proved: forcing workers=1 yields 'references never overlapped inside call_llm' instead of passing. 246 tests green across 7 files under the canonical runner; 0 xfail markers remain in either file; ruff clean.
Collaborator
Author
FleetReviewConfidence: 3/5 Findings
FleetReview provenance · models: B=gpt-5.6-sol, C=claude-code-opus-4-8, F=gpt-5.6-sol, G=grok-4.5 · cost: $10.86 · duration: 8m 56s · rounds: 1 · files examined: 5 |
Kyzcreig
enabled auto-merge (squash)
July 26, 2026 03:33
Kyzcreig
added a commit
that referenced
this pull request
Jul 27, 2026
…kes) (#438) Three wall-clock flakes hit in a single day. They share one defect: the test proves a CONCURRENCY or NON-BLOCKING property by measuring elapsed real time, which makes the OS scheduler part of the assertion. Under load the inequality flips with nothing wrong in the code under test. The worst one blocked the merge queue: tests/gateway/test_session_hygiene.py failed `assert elapsed < 2.0` with 2.005265276999978 -- five milliseconds -- while sitting at the head of a queue for a PR touching zero gateway files. Profiling that test shows the bound was not even measuring the behavior it protects: ~1.5s of the 2.3s window is models_dev.fetch_models_dev -> _save_disk_cache -> atomic_json_write (~600k json-encoder calls), i.e. one-time provider-metadata cache work. Instrumented runs on an IDLE box measured 1.40s, 1.87s and 2.39s -- the 2.0 threshold sits inside the natural distribution, so the failure was not an outlier. This applies the pattern from #426 (barrier, not stopwatch) to the rest of the family: - tests/gateway/test_session_hygiene.py -- ordering witness: an Event set in a finally on every worker exit path, asserted UNSET when the handler returns. - tests/agent/test_context_refs_concurrent.py -- asyncio.Barrier(3): all three @url: fetches must be in flight at once before any may return. - tests/tools/test_mcp_tool.py -- threading.Barrier(3) for parallel shutdown. - tests/plugins/memory/test_mem0_rerank_guard.py -- alert_returned witness replaces `elapsed < 0.1` across a 16-thread pool. - tests/agent/test_memory_boundary_commit.py -- assert the provider recorded NOTHING yet, proving /new was not gated on the slow extraction. Every conversion keeps a finite barrier/wait timeout (10-30s) so a genuine regression fails fast instead of hanging, but that is orders of magnitude above real rendezvous latency, so it is not itself a timing assertion. RED-PROVEN: each was verified by breaking the behavior it protects and confirming the new assertion fails by name -- removing the wait_for timeout wiring, replacing asyncio.gather with a serial loop (x2), making observe() join the alert thread, and running the boundary commit inline (the pre-NousResearch#16454 blocking bug). Two of the five first drafts could NOT fail and were caught and strengthened by that exercise. All source mutations reverted; this diff is test-only. Side effect: the barrier forms delete fixed sleeps, so test_mcp_tool shutdown drops ~1s -> ~0.1s and the mem0 rerank test 7.0s -> 1.7s. Deliberately NOT changed: hang-guards. `assert ev.wait(timeout=5)` asserts the EVENT, not the duration; lower bounds like `assert elapsed >= 0.04` prove an injected wait happened and get MORE reliable under load; and ceilings an order of magnitude above the hang they guard fail only on real regressions. The rule applied: convert when load can cause failure, leave alone when only a real regression can. Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
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
1 participant
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.
Closes the two documented xfails from PR #420.
t_05fcd58f — route-aware fast-mode adoption.
test_fast_route_capabilityis an AST enforcement test demanding every request-enforcement call site use the route-awareresolve_fast_mode_capability()rather than the model-only wrapper. It had been red on fork/main since before the merge (latent debt — verified identical on baseline37fa0a353). Call sites converged, preserving model-only-route behavior; marker deleted.t_78a2fa21 — deterministic parallelism proof.
test_references_run_in_parallelproved concurrency with a wall-clock stopwatch, which flips under CI's 12-worker parallelism when the scheduler serializes coroutines. Replaced with an overlap assertion insidecall_llm. RED-proved: forcingworkers=1yieldsreferences never overlapped inside call_llminstead of silently passing.Verification (mine, re-run independently of the worker): 93 passed across both files. Worker's canonical-runner sweep: 246 tests / 7 files green, 0 xfail markers remaining, ruff clean.
Not upstreamable — both patches target fork-only regions of
slash_commands.py/models.py(verified:git apply --checkfails againstorigin/main).