Repository navigation
fix: improve missing stream completion diagnostics - #5457
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed the full change at 3911321. No substantive correctness or maintainability findings.
The diagnostics use the existing stream events and request-ID propagation. They preserve the exception type and prefix, failure timing, retries, and explicit terminal handling. Empty streams still fail and report unavailable IDs. The added message includes correlation metadata without copying prompts or model output.
The regression tests exercise the real SSE parser and public runner, including partial and empty streams, missing request IDs, cleanup, and completed/failed/incomplete events. Hosted CI at this head reports 23 successful checks and two skipped checks. This was a source-only review with an independent source pass; I did not execute candidate code or tests. The local baseline failures described in the PR remain author-reported.
Summary
When a model stream ends without
response.completed, the runner currently raises a generic error that provides no request correlation information. Include the observed request ID, response ID, event count, and last event type in that error so reports can be investigated without logging prompts or model output.Preserves the
ModelBehaviorErrorclass, original message prefix, failure location, and retry behavior. An entirely empty stream reports unavailable IDs because no response events reached the runner. This is a diagnostic improvement, not a fix for the underlying stream interruption.Test plan
httpx2.MockTransportfor partial and empty streams, absent request IDs, payload exclusion, no additional retries, and completed/failed/incomplete terminal behavior.userin the local/home/dev-usercheckout path, and seven sandbox tests expect local snapshots but receive no-op snapshots in this environment. No unrelated fixes are included.Checks
.agents/skills/code-change-verification/scripts/run.sh