test: MockProvider rejects transcripts a real provider would - #160
Merged
Conversation
It discarded StreamConfig entirely, so 600+ tests drove the agent loop without one of them noticing when it built a message sequence every provider rejects. That is not a coverage gap more tests would close — it is structural, and it is exactly how loop detection reached a release branch injecting a steering message between an assistant's tool_use blocks and their tool_results. A history that shape poisons the agent: it is kept, so every later prompt fails too. The mock now checks the one invariant every provider enforces — a tool_use is answered by its tool_results before anything else intervenes — and panics with the reason a provider would give. Panics rather than returns Err on purpose: a malformed transcript is a defect in the code under test, and returning a provider error would be swallowed by the very retry path that should never see it. Opt out with MockProvider::without_transcript_validation() for tests that build a malformed history deliberately. Nothing needed it. Effect, measured by reverting the 0.18 steer fix: 5 tests now fail with the precise diagnostic, where the whole suite previously stayed green. 600+ existing tests became transcript guards without writing one. One real fixture bug found: test_continue_from_tool_result resumed from [user, tool_result] — a tool result answering nothing, which no provider accepts and which therefore cannot arise in production, so the test proved nothing about the path it named. It now includes the assistant turn that made the call. 629 passed, clippy clean under -Dwarnings.
4 tasks
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.
The highest-leverage change available: it retroactively upgrades every existing test.
The structural hole
MockProvider::streamtook_config: StreamConfigand discarded it. So 600+ tests drove the agent loop without a single one noticing when it built a message sequence every provider rejects.That is not a coverage gap more tests would close. It is why loop detection reached a release branch injecting a steering message between an assistant's
tool_useblocks and theirtool_results — a history that shape poisons the agent, because it is kept, so every later prompt fails too. The suite was green throughout.The change
The mock now checks the one invariant every provider enforces — a
tool_useis answered by itstool_results before anything else intervenes — and panics with the reason a provider would give.Panics rather than returning
Errdeliberately: a malformed transcript is a defect in the code under test, and a provider error would be swallowed by the very retry path that should never see it.MockProvider::without_transcript_validation()exists for tests that build a malformed history on purpose. Nothing needed it.Effect, measured
Reverting the 0.18 steer fix:
600+ existing tests became transcript guards without writing one.
One real fixture bug found
test_continue_from_tool_resultresumed from[user, tool_result]— a tool result answering nothing. No provider accepts that, so it cannot arise in production, so the test proved nothing about the resume path it named. It now includes the assistant turn that made the call.That it was the only failure across 629 tests is itself the useful signal: the loop's own transcript-building is clean, which is evidence the 0.18.1 fixes were right.
cargo test --all-features: 629 passed, 0 failed. Clippy clean under-Dwarnings.Test-infrastructure only — no library behaviour change.
🤖 Generated with Claude Code