[BREAKING] Python: Trim final FRC to match service storage#4803
[BREAKING] Python: Trim final FRC to match service storage#4803TaoChenOSU wants to merge 1 commit intomicrosoft:mainfrom
Conversation
github-actions
bot
changed the title
TaoChenOSU
changed the title
Python Test Coverage Report •
Python Unit Test Overview
|
|||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
Aligns Python agent session history storage with service-side behavior by trimming trailing function-result content that is produced locally at termination (e.g., middleware/tool termination), mirroring the .NET behavior described in #4792 and addressing #4609.
Changes:
- Add
store_final_function_result_contentrun option to control whether trailing tool/function-result messages are stored in history. - Implement trimming logic in
RawAgentto filter trailing tool-role messages that contain onlyfunction_resultcontent before running after-run providers. - Add unit + integration coverage for filtering behavior across non-streaming and streaming runs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| python/packages/core/agent_framework/_agents.py | Adds the filtering implementation and threads the new option through the agent run lifecycle before persisting history. |
| python/packages/core/agent_framework/_types.py | Extends ChatOptions typing with the new store_final_function_result_content flag. |
| python/packages/core/tests/core/test_store_final_function_result_content.py | Adds tests validating default filtering behavior, opt-out, and streaming behavior. |
| opts = dict(options) if options else {} | ||
| existing_additional_args: dict[str, Any] = opts.pop("additional_function_arguments", None) or {} | ||
| store_final_frc: bool = opts.pop("store_final_function_result_content", False) | ||
|
|
There was a problem hiding this comment.
store_final_function_result_content is only read from per-run options (via opts.pop(...)) and does not consider Agent.default_options. If a user sets this flag as a default option, it will (1) be ignored for filtering (ctx value remains False) and (2) be forwarded down to client.get_response() via co, potentially breaking clients that reject unknown options. Consider deriving the effective flag from the merged options (run overrides defaults), then removing it from the options dict passed to the client.
| # System/instructions | ||
| instructions: str | ||
|
|
||
| # History storage | ||
| store_final_function_result_content: bool | ||
|
|
There was a problem hiding this comment.
store_final_function_result_content is included in ChatOptions, which is documented as “Common request settings for AI services”. This key is agent-side history-storage behavior (not a provider request parameter), so its presence in ChatOptions can mislead users/custom clients into passing it to client.get_response(). Consider clarifying in the comment/docs that this option is Agent-only and not forwarded to providers (or splitting non-provider run options into a separate TypedDict).
| with patch("agent_framework._tools.DEFAULT_MAX_ITERATIONS", 2): | ||
| agent = Agent(client=client, name="test-agent") | ||
| session = agent.create_session() |
There was a problem hiding this comment.
Tests cover per-run options={"store_final_function_result_content": ...} but don’t cover configuring this behavior via Agent(default_options=...). Given default_options is a common way to set run defaults, add a test to ensure the default is honored and that the flag is not forwarded to the underlying chat client options.
There was a problem hiding this comment.
this can just be part of the test_function_invocation_logic.py test suite
| instructions: str | ||
|
|
||
| # History storage | ||
| store_final_function_result_content: bool |
There was a problem hiding this comment.
is this common enough to have to be part of the core chat options? not sure I like that, it also is not really a service side setting, which is what is the target for the ChatOptions, so we might want to use client_kwargs for this
Motivation and Context
Closes #4609
Description
Python implementation of .Net PR #4792
Contribution Checklist