Python: mark hosted /responses incomplete on content_filter and length finish reasons - #8478
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Persist the incomplete-reason marker across workflow checkpoint recovery so filtered turns cannot be reported as completed.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates hosted Python /responses handling to report content_filter and length finishes as incomplete responses.
Changes:
- Tracks truncating finish reasons for agent and workflow streams.
- Emits incomplete metadata for streaming and non-streaming responses.
- Adds regression tests for finish-reason behavior.
File summaries
| File | Summary |
|---|---|
python/packages/foundry_hosting/tests/test_responses.py |
Adds coverage for filtering, length limits, streaming, and workflow handling. |
python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py |
Tracks finish reasons and emits incomplete responses. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
…h finish reasons
The hosted /responses path iterated each AgentResponseUpdate's contents but never read
its finish_reason, so a turn the model stopped early (Azure OpenAI content filter, token
limit) ended with status "completed" and no trace of the cut-off. Callers could only
detect a filtered turn by matching the canned refusal text.
_OutputItemTracker now records truncating finish reasons from both the plain-agent and
workflow update loops, and _handle_response ends the response with response.incomplete
plus incomplete_details.reason ("content_filter" / "max_output_tokens") instead of
response.completed, mirroring the OpenAI Responses shape. The refusal text is still
delivered as an output item.
Fixes microsoft#8475
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Recovery rebuilds _OutputItemTracker from the persisted response, so a marker held only in memory was lost if the crash landed between a filtered update and a later one. Mirror it into the stream's internal_metadata (persisted with every checkpoint, like the last checkpoint id) and restore it in the tracker constructor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…h the checkpoint an update follows _OutputItemTracker.handle_update records the update's finish reason and then handles its contents, so the plain-agent and workflow loops share one ordering rule. The workflow loop snapshotted the response against the latest checkpoint in storage at the time an update was consumed. _SignalledIterator drives the workflow one update ahead, so by then the runner could already have checkpointed past the update; a crash after that snapshot resumed the workflow with the update never replayed and its output (and any incomplete reason) lost. _SignalledIterator now takes an optional stamp coroutine that the driver awaits right after each item is produced, before advancing the wrapped iterator again, and the loop pairs snapshots with that stamped checkpoint id. After the workflow completes, its final checkpoint is paired with the full output so recovery does not replay the last superstep. The real-crash recovery integration tests that were xfailed against microsoft#7809 pass with this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1200347 to
55f009d
Compare
|
Rebased on main to resolve the conflict with #8372 (request-scoped agent factories): the workflow loop now reads the workflow name from the request-scoped |
- Drop the redundant `None, None` Generator parameters flagged by pyupgrade. - Type the `finish_reason` test parameters as `FinishReasonLiteral` instead of `str` + `type: ignore`, which `ty` does not honour. - Read the text delta with `.get()` so the `ResponseStreamEvent` TypedDict union does not need narrowing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eduard van Valkenburg (eavanvalkenburg)
left a comment
There was a problem hiding this comment.
Anish Mehta (@anishmehta24), the outstanding review concerns are addressed on the current head, and the full diff looks good to me.
Motivation & Context
When an Azure OpenAI content filter stops a turn, the underlying chat completion carries
finish_reason: "content_filter", but the hosted agent's/responsesoutput reports a fully successful turn: HTTP 200,status: "completed",error: null, and the canned refusal delivered as ordinaryoutput_text. An HTTP caller has no way to tell a filtered turn from a normal one short of matching the refusal string, which breaks on wording changes and localisation. The same applies to a turn cut off by the token limit (finish_reason: "length").Root cause:
_handle_inner_agentand_handle_inner_workflowinpython/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.pyiterateupdate.contentsand never readupdate.finish_reason, so_handle_responsealways ends withemit_completed()unless an OAuth consent request was surfaced.Description & Review Guide
_OutputItemTrackergainsrecord_finish_reason(finish_reason)and anincomplete_reasonproperty. Both update loops callrecord_finish_reason(update.finish_reason)before handling the update's contents._handle_responsenow emitsresponse.incompletewithincomplete_details.reasonwhen a truncating finish reason was seen, using theResponseIncompleteReasonenum the AgentServer SDK already defines (content_filter,max_output_tokens), so the payload matches the OpenAI Responses shape the issue asks for. Onlycontent_filterandlengthare mapped;stop/tool_callsleave the responsecompletedas before.status: "incomplete"withincomplete_details: {"reason": ...}in both streaming (response.incompleteterminal event) and non-streaming bodies. The output items themselves are unchanged: the refusal text is still delivered so a caller can show it if it wants to. The existing OAuth consentemit_incomplete()behaviour is preserved (reason staysNonein that case unless a filter also fired). The signal is sticky within a turn: a filter mid-way through a multi-step turn is not erased by a later update that finishes withstop, andcontent_filterwins overlengthif both are seen, since it is the more actionable signal._handle_response, and whether you wantlengthmapped too (it is the other member ofResponseIncompleteReason, so it felt wrong to leave it out; happy to drop it if you'd rather keep this strictly to the reported case). Tests: newTestIncompleteFinishReasonSurfacingclass next to the OAuth consent tests covers non-streaming and streamingcontent_filter, thelengthmapping, normal finish reasons still completing, stickiness across later updates, precedence, and theWorkflowAgentpath via_build_text_workflow_agent(..., finish_reason=...).Prepared with AI assistance (Claude Code) and human-verified: 6 of the 7 new tests fail on
mainand all pass with the fix;poe fmt,poe lint,poe pyright -P foundry_hosting,poe mypy -P foundry_hostingand thefoundry_hostingtest suite (324 passed, 28 skipped, 8 xfailed) were run locally.Related Issue
Fixes #8475
Contribution Checklist
🤖 Generated with Claude Code