Skip to content

fix(codex): preserve commentary across interleaved output items - #69260

Open
ruizanthony wants to merge 3 commits into
NousResearch:mainfrom
ruizanthony:fix/codex-commentary-interleaved-followup
Open

fix(codex): preserve commentary across interleaved output items#69260
ruizanthony wants to merge 3 commits into
NousResearch:mainfrom
ruizanthony:fix/codex-commentary-interleaved-followup

Conversation

@ruizanthony

Copy link
Copy Markdown
Contributor

Summary

  • preserve commentary buffers when Codex Responses output items interleave
  • keep unmatched aliased deltas private while a private item is active
  • preserve standalone unphased final output streams instead of dropping visible text

Context

PR #59831 was superseded by #66115, which landed the core commentary-to-interim routing and display.show_commentary control. This is a narrow follow-up for residual stream-shape cases reproduced against current main; it does not reintroduce the superseded implementation.

Tests

  • HERMES_PYTHON=/usr/local/lib/hermes-agent/venv/bin/python ./scripts/run_tests.sh tests/run_agent/test_run_agent_codex_responses.py -q — 112 passed
  • python -m py_compile agent/codex_runtime.py
  • git diff origin/main..HEAD --check

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/openai OpenAI / Codex Responses API codex labels Jul 22, 2026
@ruizanthony

Copy link
Copy Markdown
Contributor Author

Maintainer action needed: the CI workflows on this PR are awaiting approval (first-time contributor gating) — no checks have run yet on head 780aefe72a. Could you approve the workflow run so the gate can certify this head? Local gate on the contributor side: codex responses/streaming regression suites pass (42/42 on the rebased patch). Thanks!

@ruizanthony

Copy link
Copy Markdown
Contributor Author

Rebased onto current master (4d9541b9c) — the PR is mergeable again. Semantic content unchanged: the 3 commits are preserved (interleaved commentary items, unmatched aliased deltas privacy, unphased final streams); the only delta vs the previous head is upstream context adaptation (event_streamconcrete_response rename in the concrete-response compatibility block). Combined diff verified line-by-line against the pre-rebase patch: no behavior loss. tests/run_agent/test_run_agent_codex_responses.py: 41/41 passed. New head: 100799377. Still needed from a maintainer: approval of the pending workflow runs (first-time contributor gating) so CI can certify this head.

@ruizanthony
ruizanthony force-pushed the fix/codex-commentary-interleaved-followup branch from 780aefe to 1007993 Compare July 30, 2026 00:14
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused follow-up. The premise remains valid on current main: agent/codex_runtime.py:1069-1078 stores only one active message phase, and agent/codex_runtime.py:1083-1101 routes deltas solely through that mutable value. That cannot preserve a commentary item when another output item interleaves before its remaining deltas arrive.

The proposed item-id/output-index association addresses that state-model gap, and the added regression coverage exercises interleaved commentary/final items, function-call interleaving, alias mismatches, and the Relay concrete-response path. I found no blocking correctness or design-fit issue in the diff.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/openai OpenAI / Codex Responses API sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants