fix(gateway): forward Codex commentary to API Server streaming clients (#67580)

[PRATHAMESH75]

What does this PR do?

When Hermes uses the openai-codex backend behind API Server, Codex emits user-facing progress preambles as assistant items with phase="commentary". The core already separates these — routing them to on_commentary_message → interim_assistant_callback, while phase="analysis" stays on the reasoning path and final text uses the normal text callback (added in #66115).

But APIServerAdapter forwarded stream_delta_callback, tool-progress, tool-start, and tool-complete callbacks to AIAgent and never interim_assistant_callback. So on the streaming session endpoint (POST /api/sessions/{id}/chat/stream), long tool-heavy turns stayed silent until the final answer — the commentary was generated and stored in codex_message_items but lost at the API Server boundary, making active work indistinguishable from a stalled run.

This wires the existing callback through the two adapter methods and surfaces commentary on the session stream as a distinct, typed assistant.commentary event — never concatenated into the final answer.

Related Issue

Fixes #67580

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• gateway/platforms/api_server.py
  • _create_agent() — accept interim_assistant_callback and forward it to the AIAgent(...) constructor (stored on the agent via agent_init, same attribute the native gateway sets).
  • _run_agent() — accept interim_assistant_callback and forward it to _create_agent().
  • _handle_session_chat_stream() — add a _commentary callback that enqueues a typed assistant.commentary SSE event, and pass it as interim_assistant_callback to _run_agent(). It drops already_streamed=True interims (no duplicate of text already sent via assistant.delta) and only emits non-empty text.
• tests/gateway/test_api_server.py — test_create_agent_forwards_interim_assistant_callback: asserts _create_agent forwards the callback to AIAgent.
• tests/gateway/test_session_api.py — test_session_chat_stream_emits_codex_commentary_as_typed_event: asserts a commentary event is emitted with its text, an already_streamed interim is dropped, and commentary is not concatenated into the final assistant.completed content.

Design notes / non-goals

• Only commentary is exposed. phase="analysis", raw chain-of-thought, and reasoning summaries never reach this callback (the core keeps them on the reasoning path), so no CoT leaks through the new event.
• The final answer stays clean — commentary is a separate event, never merged into final content.
• The existing display.show_commentary: false gate still suppresses commentary entirely (the core doesn't fire the interim callback when it's off), so quiet behavior is preserved.
• Non-streaming endpoints are unchanged.

How to Test

scripts/run_tests.sh -j 4 tests/gateway/test_session_api.py tests/gateway/test_api_server.py
# 220 tests passed, 0 failed

The two new tests fail without the wiring: the stream handler must pass interim_assistant_callback=_commentary (otherwise kwargs["interim_assistant_callback"] KeyErrors in the fake run and no assistant.commentary event is produced), and _create_agent must forward it to AIAgent.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run the affected suites and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS (Darwin 25.5.0)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A (no new config keys)
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

[teknium1]

Thanks for tracing the callback boundary and keeping commentary separate from final content. The session-SSE wiring is consistent with the existing Codex callback contract.

Problems

• The linked issue covers API Server streaming more broadly, but this PR only installs the callback on /api/sessions/{id}/chat/stream. Current main's /v1/responses stream starts _run_agent at gateway/platforms/api_server.py:3940-3952 without an interim callback, so that supported streaming surface remains silent for Codex commentary.
• assistant.commentary is a new public SSE event, while website/docs/user-guide/features/api-server.md:346 lists the session-stream event contract and is not updated by this PR.

Suggested changes

• Add a Responses-compatible commentary output item and regression test for /v1/responses, or explicitly narrow this PR's scope to the session SSE endpoint.
• Document the new assistant.commentary event and its distinct-final-content behavior.

Automated hermes-sweeper review.

[PRATHAMESH75]

Thanks for the review — addressed both points in fcff829e7.

1. /v1/responses streaming surface. Rather than narrow scope, I extended the fix to the second streaming surface so #67580 is fully closed. The Responses SSE handler (gateway/platforms/api_server.py) now wires interim_assistant_callback into _run_agent, and _write_sse_responses emits commentary as a distinct assistant message output item carrying "phase": "commentary" — exactly the "Responses-compatible assistant message item retaining phase=commentary" the issue proposed. It's a separate item from the final answer, never feeds final_text_parts, and so is never concatenated into final content. already_streamed=True interims are dropped to avoid duplicates, and analysis/reasoning stays on the reasoning path (no CoT leak) — same contract as the session-stream path.

• Regression test: test_responses_stream_emits_codex_commentary_as_distinct_item (asserts the distinct item + phase marker, the duplicate drop, and that the final answer item stays clean).

2. Documentation. Updated website/docs/user-guide/features/api-server.md — the session-stream event row now lists assistant.commentary, plus a paragraph documenting both surfaces: the typed assistant.commentary session event ({message_id, content}) and the Responses phase="commentary" message item, their separation from the final answer, and that the display.show_commentary: false gate suppresses commentary on both.

/v1/chat/completions is intentionally left out per the issue's guidance (commentary must not land in choices[].delta.content where naive clients would concatenate it).

Tests: 221 passed across tests/gateway/test_api_server.py + tests/gateway/test_session_api.py (was 220).

[alt-glitch]

This was generated by AI during triage.

Related to #67580: this is its focused API-server streaming implementation.

[PRATHAMESH75]

Thanks @teknium1 — both points addressed in fcff829.

• /v1/responses streaming surface was silent for Codex commentary: the Responses SSE handler now wires interim_assistant_callback and emits phase="commentary" preambles as a distinct assistant message output item ("phase": "commentary"), kept out of final_text_parts so it never concatenates into the final answer; an already-streamed interim is dropped to avoid duplicates, and analysis/reasoning stays on the reasoning path (no CoT leak) — matching the session-stream behavior. A /v1/responses regression test in tests/gateway/test_api_server.py covers the distinct item, the phase marker, the duplicate drop, and a clean final answer.
• Undocumented public SSE event: website/docs/user-guide/features/api-server.md now documents the assistant.commentary session event and the Responses commentary item, including the distinct-final-content behavior.

[PRATHAMESH75]

Thanks for the review — both points are now addressed in fcff829e7:

1. /v1/responses streaming gap. The commentary callback is now also installed on the Responses stream. phase="commentary" preambles surface there as a distinct assistant message output item tagged "phase": "commentary", kept out of the final answer item — mirroring the session-SSE behavior. Regression test: tests/gateway/test_api_server.py::…::test_responses_stream_emits_codex_commentary_as_distinct_item asserts exactly one commentary item and that it is never concatenated into final content.
2. Undocumented assistant.commentary event. website/docs/user-guide/features/api-server.md now lists assistant.commentary in the session-stream event contract (line ~346) and documents its shape ({message_id, content}), its distinct-from-final-content behavior, the display.show_commentary gate, and the equivalent Responses-stream phase="commentary" message item (line ~350).

So the PR no longer narrows to the session endpoint — both supported streaming surfaces now forward commentary, with tests and docs for each.

[alt-glitch]

This was generated by AI during triage.

Related to #67705: the current Responses-stream implementation emits commentary as a completed typed item without output_text.delta. This is an event-contract decision for maintainers, not a duplicate relationship.

[GottZ]

This was generated by AI during triage.

Summary

Two open PRs address #67580 by wiring Codex commentary into API Server streaming. #67593 covers both session SSE and /v1/responses, suppresses already-streamed duplicates, separates commentary from final content, and adds tests and documentation; #67613 covers only session SSE and forwards already-streamed commentary.

Related pull requests

• fix(gateway): forward Codex commentary to API Server streaming clients (#67580) #67593 best fix — (+738/-1) — n/a: The diff wires interim_assistant_callback through the agent path for both session SSE and /v1/responses, emits distinct commentary events/items, drops already_streamed=True, keeps final content clean, and adds regression tests plus documentation. Consistent with the maintainer-bot keep_open verdict, the diff addresses its previously cited Responses-stream and documentation concerns.
• fix(api-server): forward Codex commentary in streaming SSE endpoint #67613 duplicate — (+21/-0) — n/a: The diff wires commentary into session SSE, but _commentary still enqueues already_streamed=True, while /v1/responses, regression tests, and event-contract documentation are absent. Despite the contributor keep_open COMMENTED review on fix(api-server): forward Codex commentary in streaming SSE endpoint #67613, its own cited salvage gaps remain visible in the diff and are covered by fix(gateway): forward Codex commentary to API Server streaming clients (#67580) #67593.

Duplicates

#67593 and #67613 duplicate the same callback wiring and session-SSE commentary event; #67613 is the narrower subset of #67593.

Suggested consolidation

Keep open with a salvage path for #67593: retain its issue-specific callback wiring, duplicate suppression, two-surface coverage, tests, and documentation, while reviewing or splitting the unrelated test additions within its large diff. Close #67613 as duplicate of #67593 because its narrower implementation retains the duplicate-emission defect and omits the Responses surface, tests, and documentation.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I67580(["issue #67580 (open)"])
    subgraph Dup67593 ["PRs duplicating each other"]
        P67593["PR #67593 (open)"]
        P67613["PR #67613 (open)"]
    end
    P67593 -->|best fix| I67580
    class I67580 open
    class P67593 open
    class P67613 open
    class P67593 best
    class P67593 target
    click I67580 "https://github.com/NousResearch/hermes-agent/issues/67580"
    click P67593 "https://github.com/NousResearch/hermes-agent/pull/67593"
    click P67613 "https://github.com/NousResearch/hermes-agent/pull/67613"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 46 kB of PR diffs, 13 kB of issue/PR text, 7 kB of discussion (9 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(gateway): forward Codex commentary to API Server streaming clients (#67580)

1. The callback is plumbed through three hops and any omission silently kills commentary — gateway/platforms/api_server.py: _create_agent (~L2626) → _run_agent (~L6358) → the agent constructor (~L6424), plus the SSE and Responses paths. A future call site that forgets interim_assistant_callback gets no error — commentary just never arrives. Consider defaulting the param to a no-op or asserting its presence when the resolved provider is openai-codex.
2. already_streamed dedup relies entirely on the producer — the drop happens in the callback, but there is no test that the same text emitted via output_text.delta and via the callback with already_streamed=True is byte-deduplicated; the tests only exercise the flag path in isolation. The contract ("who sets already_streamed and when") lives only in comments; worth documenting on the AIAgent side that emits it.
3. Commentary items are emitted status: "completed" with no delta streaming — _emit_commentary (~L4903): the whole text arrives in one output_item.added/done pair, unlike the incremental output_text.delta path. Acceptable for short preambles, but if commentary can be long (tool-heavy turns), streaming it via output_text.delta then finalizing the item would also make most of the already_streamed complexity unnecessary.
4. Docs (website/docs/user-guide/features/api-server.md) are updated and the tests cover both streaming surfaces well.