-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Use correct agent's instructions when telling model to retry output #3209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use correct agent's instructions when telling model to retry output #3209
Conversation
Add test case that reproduces issue pydantic#3207 where Agent2's instructions are ignored when running sequentially with Agent1's message history. The test verifies that each agent uses its own instructions rather than inheriting instructions from previous agents in the workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…c#3207) When running multiple agents sequentially with message_history, retry requests created by CallToolsNode were missing instructions. This caused Model._get_instructions() to fall back to searching message history and incorrectly return the previous agent's instructions instead of the current agent's. Fixed by ensuring all retry ModelRequest objects created in CallToolsNode are initialized with instructions from the current agent via ctx.deps.get_instructions(run_context). Changes: - Set instructions on empty retry requests (line 572-576) - Set instructions on tool retry requests (line 638-642) - Update test assertion to expect instructions on retry requests Fixes pydantic#3207 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The try/except block in test_multi_agent_sequential_instructions_with_output_type was never executed since the fix ensures Agent2 works correctly. Removing it achieves 100% test coverage as required by CI. Related to pydantic#3207
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move this to test_agent.py
|
|
||
| # Run Agent2 with Agent1's message history, capturing messages | ||
| # This is the scenario that triggers the bug in issue #3207 | ||
| with capture_run_messages() as agent2_messages: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't need this, we can use result.{new,all}_messages()
| instructions_in_agent2_requests = [req.instructions for req in agent2_requests if req.instructions is not None] | ||
|
|
||
| # Agent2 should use its own instructions, not Agent1's | ||
| agent2_instructions_found = 'Agent 2 instructions' in instructions_in_agent2_requests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can directly assert this, not need for the LLM assertion message stuff below
| result1 = agent1.run_sync('Hello') | ||
|
|
||
| # Run Agent2 with Agent1's message history | ||
| result2 = agent2.run_sync('Hello again', message_history=result1.new_messages()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't this always working correctly because there's a user prompt?
| result1 = agent1.run_sync('Hello') | ||
|
|
||
| # Run Agent2 WITH a user_prompt (workaround) | ||
| result2 = agent2.run_sync('Continue', message_history=result1.new_messages()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't the scenario from the issue one where there's no user_prompt and there is an output type, so that we end up with a "retry" ModelRequest? Which is tested in the first test?
I'm not sure we're testing exactly what the user reported.
…i-agent-instructions # Conflicts: # pydantic_ai_slim/pydantic_ai/_agent_graph.py
|
@dsfaccini Thanks David! |
Fixes #3207
When running multiple agents sequentially with message_history, retry ModelRequest objects created by CallToolsNode were missing instructions. This caused Model._get_instructions() to search the message history and return the first agent's instructions instead of the current agent's.
The fix ensures all retry ModelRequest objects are initialized with instructions from the current agent via ctx.deps.get_instructions(run_context).
Changes:
I'll add some materials that can be useful for discussion (the code we used to test, the logs we added, the output of those logs, claude's conclussions and proposed solutions and the implemented one):