fix(agent): stream Codex commentary separately from reasoning - #65653
fix(agent): stream Codex commentary separately from reasoning#65653100yenadmin wants to merge 4 commits into
Conversation
|
Independently verified this PR on a live deployment (WSL2, systemd gateway + desktop/dashboard backends, brain Repro before the patch: bundled prompts like "write just 'ciao', then count the files in /tmp" returned only the tool result — the greeting landed in the hidden reasoning channel ( After cherry-picking these 4 commits onto current main:
Compared with the sibling PRs (#59831 streaming-only, #60453 superseded here with authorship preserved, #62396 much larger UI surface), this one covers both the streaming and non-streaming paths with dedup and secret redaction, which is what made it adoptable as-is. Would be great to see it land. |
|
Merged via PR #66115 with your commits cherry-picked onto current main — authorship preserved in git log (136ade2, a15397d, plus David Robertson's two originals from #60453). Thanks for finding #60453, fixing it up, and getting it tested end-to-end for codex models — and thanks to @null-runner for the independent live verification. We added a |
What does this PR do?
This PR gives Codex
phase="commentary"a first-class, user-visible streaming path without exposing private reasoning.It supersedes #60453 while preserving David Robertson's two original commits and authorship. On top of that work, it adds the lifecycle, deduplication, fallback, and security fixes found during adversarial review against current
main.The user-visible contract is intentionally narrow:
phase="commentary"interim_assistant_callbackresponse.reasoning_text.*/ reasoning summariesshow_reasoningphase="analysis"phase="final_answer"This means gateways can keep
show_reasoning: falseand still show polished Codex progress updates, matching the commentary/final channel split used by Codex clients.Related issue and PRs
Type of change
Why this supersedes #60453
#60453 found the correct user-facing seam and added the first safe extraction path. This PR retains those commits, including think-block stripping and secret redaction, then fixes cases that could otherwise duplicate, delay, omit, or misclassify commentary:
final_answercontent when a response contains both commentary and final text;analysisand reasoning events off the visible commentary rail;Related design work:
WorkingUI protocol. That can still be evaluated independently; this PR deliberately uses Hermes' existing interim-assistant contract to keep the change backward-compatible and scoped.Routing design
flowchart LR SSE["Codex Responses SSE"] --> Kind{"Event / output phase"} Kind -->|"phase=commentary"| Item["Assemble one output item"] Item --> Safe["Strip think blocks<br/>Redact sensitive text"] Safe --> Interim["interim_assistant_callback"] Kind -->|"reasoning events<br/>or phase=analysis"| Reasoning["reasoning_callback"] Kind -->|"phase=final_answer"| Final["normal stream_delta_callback"] Interim --> Gateway["Gateway progress message"] Reasoning --> Thinking["Shown only when reasoning is enabled"] Final --> Reply["Assistant final response"]Commentary is emitted at the earliest authoritative lifecycle boundary:
sequenceDiagram participant C as Codex SSE participant H as Hermes runtime participant G as Gateway C->>H: commentary deltas C->>H: commentary output_item.done H->>H: strip think blocks + redact + deduplicate H-->>G: interim commentary C->>H: function call / tool transition H-->>G: existing tool progress C->>H: final_answer deltas H-->>G: normal final response streamImplementation notes
agent/codex_runtime.pyexposes a commentary callback alongside the existing reasoning and final-stream callbacks.agent/conversation_loop.pyandagent/agent_init.pywire that callback through the existing interim assistant delivery path.run_agent.pyowns per-turn delivery state so retries and response fallbacks remain idempotent.response.reasoning_text.delta.Safety and compatibility
<think>content is removed andredact_sensitive_text()is applied.phase="analysis"is never promoted to the visible path.show_reasoning: falsecontinues to hide reasoning; it no longer hides commentary.Verification
Rebased onto
mainat53adb3fd9750376b520100b0f90b737da802d1e1; reviewed head:48a48b85ca9a8a935d75590136fbd4cae51c1af6.Focused local proof: 521 passed, 0 failed.
Also passed:
noqawarning atrun_agent.py:107)compileallon all changed Python filesgit diff --checkNot claimed here: a live external Codex provider or messaging-platform smoke. The deterministic stream fixtures exercise the same event lifecycle without consuming credentials or sending messages.
Review guide
agent/codex_runtime.py.agent/agent_init.pyandagent/conversation_loop.py.run_agent.py.tests/run_agent/test_run_agent_codex_responses.pyto verify each lifecycle edge case.Checklist
Code
pytest tests/ -qlocally — the 39k-test repository suite is left to canonical CI; 521 focused tests pass locallyDocumentation and housekeeping
cli-config.yaml.exampleis N/A; no config keys changedCONTRIBUTING.md/AGENTS.mdupdates are N/A; no contributor workflow changedBehavioral acceptance
mainrebase and focused validation completeCredit: David Robertson (@davidrobertson) for the original #60453 diagnosis and safe extraction path.