Python: update FoundryAgent for hosted agent sessions#5447
Python: update FoundryAgent for hosted agent sessions#5447eavanvalkenburg wants to merge 3 commits intomicrosoft:mainfrom
Conversation
Co-authored-by: Copilot <copilot@github.com>
Python Test Coverage Report •
Python Unit Test Overview
|
|||||||||||||||||||||||||||||||||||||||||||||
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated Code Review
Reviewers: 3 | Confidence: 92%
✗ Correctness
The
_handle_inner_agentmethod in_responses.pyhas an invertedisinstance(self._agent, RawAgent)check: it warns and skips options forRawAgent(which supportsoptionsin.run()— see_agents.py:896) and passes options to non-RawAgentagents (whoseSupportsAgentRun.run()at_agents.py:271-278has nooptionsparameter). This inverts the correct old behavior. The corresponding tests were updated to match the inverted logic (usingraw_agent=False), masking the bug. The rest of the changes (FoundryAgentOptions, service session management, extra_body merging, allow_preview plumbing) look correct.
✓ Test Coverage
The PR adds substantial new functionality (FoundryAgentOptions, service session management, extra_body merging, conversation_id suppression) with generally good test coverage for the happy paths. However, there are notable test gaps: the streaming
_parse_chunk_from_openaioverride has zero test coverage despite being a mirror of the tested_parse_response_from_openai; theallow_preview=Truebranch that preserves tools/tool_choice in_prepare_optionsis untested; andget_agent_version()error paths (TypeError raises) lack coverage. The_responses.pyrefactoring inverts the RawAgent/non-RawAgent options-forwarding logic, and the test was updated to match the new non-RawAgent path, but no test verifies the new RawAgent warning behavior.
✗ Design Approach
I found two design-level problems in the new hosted-session path. The first is a real identity mix-up: the code now allows the local wrapper
nameto diverge from the remote Foundryagent_name, but the new session-management methods call the service withself.name, so a custom display name will target the wrong hosted agent. The second is an abstraction mismatch around preview enablement: the docs say a caller can pass an already preview-configuredproject_client, but the implementation gates hosted-session behavior off the wrapper'sallow_previewflag instead of the supplied client capability, so that path is disabled unless callers redundantly set both.
Suggestions
- Add a streaming-path test for
_parse_chunk_from_openaianalogous totest_raw_foundry_agent_chat_client_parse_response_suppresses_conversation_id_for_agent_sessions— the conversation_id suppression logic is identical but has zero test coverage. - Model preview enablement as a single source of truth — either require the wrapper flag consistently and document it, or derive capability from the supplied
project_client— so calers don't need to configure both.
Automated review by eavanvalkenburg's agents
Remove the public hosted-agent service session CRUD helpers from FoundryAgent and drop the related feature-stage inventory entry. Update the hosted-agent sample to create and delete service sessions directly through the preview AIProjectClient APIs, and tighten a few test harnesses surfaced by full workspace validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| openai_client_kwargs["default_headers"] = dict(default_headers) | ||
| if allow_preview: | ||
| openai_client_kwargs["agent_name"] = self.agent_name | ||
| async_client = self.project_client.get_openai_client(**openai_client_kwargs) |
| raise ValueError("isolation_key is required. Pass it explicitly or set default_options['isolation_key'].") | ||
| return resolved_isolation_key | ||
|
|
||
| async def _create_service_session_id( |
There was a problem hiding this comment.
Do we need to do this? I think if the request contains a new session ID, the service will automatically create one for the client.
There was a problem hiding this comment.
Is this a left over file from testing the project?
There was a problem hiding this comment.
Let's not change this for now.
There was a problem hiding this comment.
Let's not modify this sample for now. I will take a pass on the samples once the once in the Foundry get more stable.
Motivation and Context
Foundry hosted agents now rely on preview session APIs, and the current Python Foundry agent and sample flow did not handle lazy service session creation or the new hosted request payload shape. This change aligns the Foundry agent, foundry_hosting server, and hosted-agent samples with the new hosted-agent flow.
Description
Contribution Checklist