test: LLM transport + fallback-chain coverage (llm.py 54% → 69%)#11
Merged
Conversation
The fallback chain routes to the PAID Anthropic API and every transport was untested. Pins: fallback order (ollama->local->claude), provider= claude tries claude first, claude-only NEVER falls back (it short- circuits around the _PROVIDERS table — found because the first version of that test accidentally made a real API call), prompt-caching payload shape (system block cache_control: ephemeral), per-provider last_errors recording on HTTP/timeout/connection/empty failures, no-key guard never POSTs, <think>-block + reasoning-preamble cleaning, OpenAI-shape extraction variants. A module-wide autouse fixture deep-copy-restores llm._state, replaces the API key, and arms a network tripwire (_SESSION that pytest.fails on any unmocked call) so no test in this file can ever touch the network. Tests: 278 -> 294
Reviewer's GuideAdds a comprehensive test suite for the LLM transports and fallback chain, including a network tripwire fixture, prompt-caching payload checks, error-recording behavior, and fallback ordering semantics, and updates docs to reflect the increased test count and coverage. Sequence diagram for LLM fallback-chain behavior under testssequenceDiagram
participant Caller
participant LLM
participant Ollama
participant Local
participant Claude
Caller->>LLM: send request
alt [provider is ollama or default]
LLM->>Ollama: forward to Ollama
alt [success]
Ollama-->>LLM: return response
else [failure]
LLM->>Local: forward to Local
alt [success]
Local-->>LLM: return response
else [failure]
LLM->>Claude: forward to Claude
Claude-->>LLM: return response
end
end
else [provider is claude]
LLM->>Claude: forward to Claude
Claude-->>LLM: return response
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/test_llm_transports.py" line_range="51-56" />
<code_context>
+ return self._payload
+
+
+class _Session:
+ """Scripted fake for llm._SESSION — returns/raises per call."""
+
+ def __init__(self, *outcomes):
+ self.outcomes = list(outcomes)
+ self.calls: list[dict] = []
+
+ def post(self, url, **kwargs):
</code_context>
<issue_to_address>
**nitpick (testing):** Consider asserting that _Session has no remaining scripted outcomes at the end of a test.
Right now `_Session` pops from `self.outcomes` but never checks that all scripted outcomes were used, so over‑scripting (e.g. two outcomes but only one call) can go unnoticed. Consider adding a helper (or `__del__`/context manager check) used in tests to assert that `self.outcomes` is empty after the expected calls, so tests fail when some scripted behavior isn’t exercised.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+51
to
+56
| class _Session: | ||
| """Scripted fake for llm._SESSION — returns/raises per call.""" | ||
|
|
||
| def __init__(self, *outcomes): | ||
| self.outcomes = list(outcomes) | ||
| self.calls: list[dict] = [] |
There was a problem hiding this comment.
nitpick (testing): Consider asserting that _Session has no remaining scripted outcomes at the end of a test.
Right now _Session pops from self.outcomes but never checks that all scripted outcomes were used, so over‑scripting (e.g. two outcomes but only one call) can go unnoticed. Consider adding a helper (or __del__/context manager check) used in tests to assert that self.outcomes is empty after the expected calls, so tests fail when some scripted behavior isn’t exercised.
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.
Summary
Coverage wave for the riskiest untested code: the LLM fallback chain routes to the paid Anthropic API, and every transport body was uncovered.
16 new tests pin:
ollama → local → claude;provider=claudetries claude first; success short-circuits the chainclaude-onlynever falls back — it short-circuits around the_PROVIDERStable; the first draft of this test patched the table and accidentally made a real API call, which is exactly why this file now arms a network tripwire (autouse fixture replaces_SESSIONwith an object thatpytest.fails on any unmocked call and swaps the real API key out)systemblock carriescache_control: {type: ephemeral}(pins the PR fix: close analyze_site sanitize bypass + harden web API + perf wins #6 cost fix)last_errorsrecording for HTTP errors, timeouts, connection failures, empty responses; success clears the error; missing key never POSTs<think>-block + reasoning-preamble cleaning; OpenAI-shape extraction variants (reasoning_contentfallback)State isolation via deep-copy restore (the nested
last_errorsdict made shallow snapshots leak between tests — a latent issue the review flagged).Test plan
ruff checkclean🤖 Generated with Claude Code
Summary by Sourcery
Increase LLM transport and fallback-chain test coverage and document the expanded test suite.
Documentation:
Tests: