fix(switch_model): guard _fallback_chain access on partially-constructed agents#14731
Closed
AndreKurait wants to merge 1 commit into
Closed
fix(switch_model): guard _fallback_chain access on partially-constructed agents#14731AndreKurait wants to merge 1 commit into
AndreKurait wants to merge 1 commit into
Conversation
…ted agents
## Problem
`AIAgent.switch_model()` unconditionally reads ``self._fallback_chain``
when pruning the fallback chain after a provider swap (run_agent.py:2041).
If an agent is partially constructed — e.g. tests that use
``AIAgent.__new__(AIAgent)`` with ``__init__`` mocked, to isolate
``switch_model`` behavior — ``_fallback_chain`` may not exist yet,
causing:
AttributeError: 'AIAgent' object has no attribute '_fallback_chain'
Reproduction: `pytest tests/agent/test_minimax_provider.py::\
TestMinimaxSwitchModelCredentialGuard::\
test_switch_to_minimax_does_not_resolve_anthropic_token`
Without this fix, the test fails with the above AttributeError at
run_agent.py:2042 because the test patches ``AIAgent.__init__`` to
`return_value=None` and never sets ``_fallback_chain``.
## Fix
Replace the direct attribute read with ``getattr(self, "_fallback_chain",
[]) or []``. This preserves the existing behaviour for fully-initialized
agents (the attribute is always a list in that case) while keeping
partially-constructed test fixtures working.
## Changes
- `run_agent.py:2041-2044`
- Add ``existing_chain = getattr(self, "_fallback_chain", []) or []``
before the list-comprehension.
- Switch the comprehension to iterate ``existing_chain``.
- 3-line change, no behavioural difference for production code paths.
## Testing
Verified the test failure reproduces without the fix and passes with it:
# Without fix:
AttributeError: 'AIAgent' object has no attribute '_fallback_chain'
FAILED tests/agent/test_minimax_provider.py::\
TestMinimaxSwitchModelCredentialGuard::\
test_switch_to_minimax_does_not_resolve_anthropic_token
# With fix:
PASSED tests/agent/test_minimax_provider.py -q
# 41 passed in 4.32s
## Risk
Minimal. `getattr(obj, name, default)` is idiomatic Python for defensive
attribute access and has zero runtime cost when the attribute exists.
The `or []` guards the (extremely unlikely) case where someone manually
set the attribute to None.
Signed-off-by: Andre Kurait <andrekurait@gmail.com>
Contributor
|
Thanks @AndreKurait. Closing — superseded by @LeonSGP43's #14867 which landed in #15134 (commit a9fd8d7). Both PRs defend against missing You both spotted the same class of bug — thanks for the careful report. |
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.
What does this PR do?
Fixes an
AttributeError: 'AIAgent' object has no attribute '_fallback_chain'inAIAgent.switch_model()when called on a partially-constructed agent. Three-line defensive guard usinggetattr(..., default).Why?
switch_model()prunesself._fallback_chainwhen the user swaps primary providers (openrouter → anthropic, etc.). The original code readself._fallback_chaindirectly:If
__init__hasn't run (e.g. tests usingAIAgent.__new__(AIAgent)with__init__patched toreturn_value=None),_fallback_chaindoesn't exist yet and the attribute read raises.Reproduction
Without this fix:
With this fix:
(That test was clearly written against an earlier version where the pruning block didn't exist, then the pruning was added later without updating the test fixture. Rather than force every caller to pre-populate
_fallback_chain, this PR makes the consumer defensive.)Changes
run_agent.py:2041-2044Which type of PR is this?
How has this been tested?
Risk
Minimal:
getattr(obj, name, default)is idiomatic Python with zero runtime cost when the attribute exists.or []guards the unlikely case where someone manually set the attribute to None._fallback_chainis always set by__init__in the normal agent construction path.Checklist