fix(test): stop letting a busy CI runner adjudicate a performance claim (evaluator <10ms flake) - #2333
Merged
Merged
Conversation
…im (evaluator <10ms flake) `test_all_gates_pass_normal_message` failed CI at 10.72971ms against a hardcoded `decision_time_ms < 10.0`. The gates did not regress — the runner was busy. It is red on PR #2331, whose diff is CI workflow config and cannot possibly have touched persona gating. A correctness test that a loaded machine can fail is not measuring the code, it is measuring the machine. Worse, it spends the reviewer's trust every time it flakes: the next red is assumed to be noise, and eventually a real one is. The performance claim is real and worth keeping, so it is NOT deleted — it MOVES to a `#[cfg(feature = "stress-tests")]` block (CLAUDE.md § test rules, item 2), where default `cargo test` does not run it and a quiet machine can. The stress version also takes the MEDIAN of 50 runs instead of trusting one sample, since a single timing of a live system is not a fact about it — one scheduler hiccup is not a regression. The correctness test keeps its correctness assertion (`should_respond`) and now asserts nothing about the clock. Same treatment as the grounding-cost flake in PR #2330 — second instance of this class, so it is a missing constraint, not two bugs. Surveyed the rest of the tree: 3 siblings exist (command_executor <500ms, rag/engine <250ms, sentinel/parallel <180ms). Deliberately NOT touched — none is failing and each has 20-50x more headroom than the 10ms budget that broke. Named in a comment at the fix site so the next person has the map instead of rediscovering it. cargo test persona::evaluator: 33 passed, 0 failed. cargo check --features stress-tests --profile test: clean (the gated block builds). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo
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.
What broke
test_all_gates_pass_normal_messagefailed CI at 10.72971ms against a hardcodeddecision_time_ms < 10.0.The gates did not regress. It is red on #2331, whose entire diff is CI workflow config and cannot have touched persona gating. The runner was busy.
Why not just raise the number
A correctness test that a loaded machine can fail isn't measuring the code — it's measuring the machine. And it spends reviewer trust every time it flakes: the next red gets assumed to be noise, and eventually a real one is.
Fix
The performance claim is real, so it isn't deleted — it moves to
#[cfg(feature = "stress-tests")](CLAUDE.md § test rules, item 2). Defaultcargo testno longer adjudicates it; a quiet machine still can.The stress version also takes the median of 50 runs rather than trusting one sample — one scheduler hiccup is not a regression.
The correctness test keeps its correctness assertion (
should_respond) and now asserts nothing about the clock.Class, not instance
Second time this shape has bitten (first: the grounding-cost flake in #2330), so it's a missing constraint rather than two bugs. Surveyed the tree — 3 siblings exist and are deliberately not touched, since none is failing and each has 20–50× more headroom than the 10ms budget that broke:
command_executor.rsrag/engine.rssentinel/steps/parallel.rsThey're named in a comment at the fix site, so whoever hits one next has the map instead of rediscovering it.
Unblocks the
cargo test -p continuum-core --libjob on #2331 and #2332.cargo test persona::evaluator→ 33 passed, 0 failedcargo check --features stress-tests --profile test→ clean🤖 Generated with Claude Code
https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo