fix(agent): handle content_filter in Codex incomplete response handling - #55682
fix(agent): handle content_filter in Codex incomplete response handling#55682AlexFucuson9 wants to merge 1 commit into
Conversation
Codex Responses can return status='incomplete' with incomplete_details.reason='content_filter' and no output items. The existing code only handled 'max_output_tokens'/'length' as incomplete reasons — content_filter fell through to the else branch and was treated as finish_reason='stop', causing the response to be processed as a normal completion instead of a content-policy refusal. Add explicit check for incomplete_reason == 'content_filter' to set finish_reason='content_filter', which routes through the existing content-policy refusal handling (failover, retry with fallback model). Fixes NousResearch#55637
Code Review Summary for PR #55682PR: #55682 Verdict: Changes requested Blocking
Checks run
|
Competing fix for #55637. This PR ( |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the missing Codex reason mapping. The loop-only change does not handle the exact empty-output response in the report.
Problems
agent/conversation_loop.py:1373-1420validates Codex output before the finish-reason block and marks an emptyoutputinvalid, so this branch is not reached for the reported shape.- If that gate is relaxed, the existing refusal path calls
normalize_response()atagent/conversation_loop.py:1669-1677;_normalize_codex_response()raises on empty output atagent/codex_responses_adapter.py:1121-1138before it can surface a refusal. - No regression test covers
status="incomplete",incomplete_details.reason="content_filter", andoutput=[].
Suggested changes
- Handle the no-output content-filter response in validation and normalization as well as in the loop, and add focused transport/adapter/loop coverage. The broader approach in #55639 covers those layers.
Automated hermes-sweeper review.
| incomplete_reason = getattr(incomplete_details, "reason", None) | ||
| if status == "incomplete" and incomplete_reason in {"max_output_tokens", "length"}: | ||
| finish_reason = "length" | ||
| elif status == "incomplete" and incomplete_reason == "content_filter": |
There was a problem hiding this comment.
This mapping alone does not reach the reported empty-output case: current main validates Codex output before finish-reason handling (agent/conversation_loop.py:1373-1420) and rejects output=[]. If that validation is relaxed, the refusal path also needs _normalize_codex_response() to accept this response instead of raising on empty output.
|
Closing as a duplicate of #55639 by @sk-holmes, which was submitted first (about 100 minutes earlier) and has been merged via salvage PR #65061. Your conversation-loop branch matched the same classification idea, but the merged fix also covers the transport validation gap — the |
Summary
Handle
content_filteras an incomplete reason in Codex Responses mode so content-policy refusals are properly classified instead of being treated as normal completions.Problem (P2 #55637)
Codex Responses can return
status="incomplete"withincomplete_details.reason="content_filter"and no output items. The existing code only handledmax_output_tokens/lengthas incomplete reasons —content_filterfell through to theelsebranch and was treated asfinish_reason="stop", causing the response to be processed as a normal completion instead of a content-policy refusal.This means:
Fix
Add explicit check for
incomplete_reason == "content_filter"to setfinish_reason = "content_filter", which routes through the existing content-policy refusal handling (failover, retry with fallback model).Changes
agent/conversation_loop.py: Addelifbranch for content_filter (2 lines added)Fixes #55637