feat(desktop): opt-in live Working lane for Codex commentary - #66318
Closed
null-runner wants to merge 7 commits into
Closed
null-runner wants to merge 7 commits into
null-runner wants to merge 7 commits into
Conversation
On the Desktop app the Codex commentary/analysis channel (mid-turn narration) has nowhere to go: tui_gateway never sets interim_assistant_callback, so codex_runtime folds commentary into the reasoning channel — it shows up inside the "Thinking" disclosure rather than as user-facing progress. Every other surface (messaging, CLI) already delivers commentary via the interim path. This wires the desktop to that same engine behind an opt-in setting: - tui_gateway sets interim_assistant_callback (emitting a commentary.delta WS event) only when display.commentary_lane is enabled. Default off, so nothing changes unless you turn it on; when off the callback stays unset and commentary falls back to reasoning exactly as before. The gate is backend- side on purpose: once the callback is set codex_runtime routes commentary away from reasoning, so a renderer-only gate would drop it entirely. - The renderer consumes commentary.delta into a dedicated data-commentary part with its own grey "Working" disclosure, separate from Thinking and from the final answer, flushed in semantic order (reasoning -> commentary -> answer). It is a data part, not a text part, so completion's replaceTextPart can't discard it. - Settings > Appearance gets a "Live Working Lane" toggle, config-backed via the config.get/config.set key `commentary_lane`. Live-only: reload does not reconstruct the lane (commentary is not persisted into reasoning/content), so a reloaded turn shows just the answer. Tests: tui_gateway commentary event + gate; renderer commentary lane (event -> part, ordering, coalescing) plus a render case (Working vs Thinking); the store. Relates to NousResearch#62396, which proposes a full end-to-end dedicated display lane; this is a narrower, opt-in, desktop-only wiring that reuses the commentary engine already on main. If the full lane lands, happy to close or rebase this.
alt-glitch
added
type/feature
Contributor
Related: #62396 implements a broader end-to-end dedicated commentary lane, while this PR is an opt-in Desktop/TUI-gateway design. The overlapping user-facing goal needs a maintainer choice rather than a duplicate closure. |
commentaryPart built {type:'data-commentary'}, but assistant-ui routes data
parts to components.data.by_name by the part's `name`, not by a `data-<name>`
type — so the commentary part never matched the registry and the "Working"
lane rendered nothing even though commentary.delta was delivered.
Build the part as {type:'data', name:'commentary', data} and match it by name
in the stream coalescer. The render tests now build the fixture from the real
commentaryPart() factory instead of a hand-written shape, so a shape drift can
no longer pass while the runtime renders nothing.
Two small presentation tweaks for the opt-in Working lane: - It now rests OPEN by default (a new `defaultOpen` on the shared disclosure, set only for the commentary slot). The lane is opt-in, so once enabled the narration should be visible immediately rather than collapsed — a completed turn was showing an empty "Working" header until clicked open. The user's explicit toggle still wins. - Panel restyled from flat grey to a soft grey-blue with a thin left accent so it reads as its own surface without shouting; rgba tints blend over both light and dark themes. The render test now asserts the narration text is visible with no interaction.
tonydwb
reviewed
Jul 17, 2026
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Clean, well-scoped change with good test coverage. No security concerns.
Reviewed by Hermes Agent
teknium1
reviewed
Jul 18, 2026
teknium1
left a comment
teknium1
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the narrowly scoped Desktop implementation. The current-main premise is valid: tui_gateway/server.py:4155 supplies no interim callback, while agent/codex_runtime.py:1032-1040 routes commentary to reasoning when no dedicated callback is supplied.
Problems
- Proposed
tui_gateway/server.py:4230samples the config only when_agent_cbs()builds an agent. Proposedconfig.setat :10862-10865 only persists YAML; current-main agent construction happens earlier via**_agent_cbs(sid)attui_gateway/server.py:4969. The Settings control therefore cannot change an existing session's callback. commentary.deltais a shared tui_gateway event, but the Ink TUI has neither a union entry inui-tui/src/gatewayTypes.ts:617-716nor a handler alongside itsreasoning.deltaandmessage.deltacases inui-tui/src/app/createGatewayEventHandler.ts:679-681,945-948. Enabling this profile setting would hide commentary on that surface rather than preserve its current reasoning fallback.
Suggested changes
- Apply the callback change to live agents or make next-session activation explicit and tested.
- Gate the event by Desktop capability, or preserve the Ink TUI fallback and cover it.
This is an automated hermes-sweeper review.
| # already_streamed means the text is the answer already relayed via | ||
| # message.delta — skip it. Each call is one completed commentary message; the | ||
| # trailing blank line separates messages that flush in the same window. | ||
| if _load_commentary_lane(): |
Collaborator
There was a problem hiding this comment.
config.set commentary_lane only writes configuration; it does not mutate agents that were already constructed with _agent_cbs(). Enabling or disabling this control in an existing Desktop chat therefore has no effect on that chat. Update the live session callback here/from the setter, or make next-session activation explicit and cover both transitions.
teknium1
added
sweeper:risk-message-delivery
The display.commentary_lane setting was only sampled inside _agent_cbs, which runs once when _make_agent builds the agent. Flipping the toggle therefore only affected sessions created afterwards; running sessions kept whatever state they were born with. codex_runtime re-reads agent.interim_assistant_callback on every LLM call, so propagating the toggle is just a matter of flipping that attribute on the live agents. config.set now calls _apply_commentary_lane_to_live_sessions, which wires the commentary callback when enabling and clears it (back to None) when disabling so the off path stays byte-identical to upstream, with commentary falling back onto the reasoning channel. The opt-in default is unchanged.
commentary.delta is emitted by the shared tui_gateway when the desktop's opt-in Working lane is on, but ui-tui had no entry for it in the GatewayEvent union nor a handler, so a TUI client attached to a gateway with the lane enabled silently dropped every commentary message. Add the event to the type union and route it into the reasoning channel, which is exactly where upstream puts Codex commentary when no lane callback is installed (see agent/codex_runtime's on_reasoning_delta fallback). This keeps the TUI behaviour equivalent to upstream instead of dropping the event, and empty payloads are ignored without throwing.
…mentary-lane # Conflicts: # tui_gateway/server.py
…ages Upstream merged display.interim_assistant_messages (default on), which also installs interim_assistant_callback. The lane-disabled test assumed the callback was absent whenever commentary_lane was off; disable both settings so it tests only the commentary lane's own contribution.
Contributor
Author
|
Closing this one: we ended up preferring the current upstream inline-commentary behavior in daily use, so the opt-in lane no longer earns its complexity. #62396 also covers a broader take on the same surface. Thanks for the triage pointers. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
On the Desktop app, the Codex commentary/analysis channel — mid-turn narration like "reading the screenshot first", "checking the logs" — has nowhere to go. Every other surface delivers it via the interim path (messaging gates sending on
display.interim_assistant_messages; the CLI prints it), buttui_gatewaynever setsinterim_assistant_callback. With the callback unset,codex_runtimefolds commentary into the reasoning channel, so on Desktop it surfaces inside the "Thinking" disclosure instead of as user-facing progress — and during long tool-heavy runs the user watches Thinking/tool rows with no narrative context.Fix
Wire the desktop to the commentary engine that already exists on
main, behind an opt-in setting (default off → zero behavior change):tui_gatewaysetsinterim_assistant_callback(emitting acommentary.deltaWS event) only whendisplay.commentary_laneis enabled. When off, the callback stays unset and commentary falls back to the reasoning channel exactly as today. The gate is backend-side on purpose: once the callback is set,codex_runtimeroutes commentary away from reasoning (on_commentary_messageinstead of the reasoning fallback), so a renderer-only gate would drop commentary entirely when off. Exposed as theconfig.get/config.setkeycommentary_lane.commentary.deltainto a dedicateddata-commentarymessage part with its own grey "Working" disclosure, separate from Thinking and from the final answer, flushed in semantic order (reasoning → commentary → answer). It is a data part, not a text part, socompleteAssistantMessage'sreplaceTextPartcan't discard it on completion.commentary_lane.Scope
Relationship to #62396
#62396 proposes a full, end-to-end dedicated commentary lane (backend adapter changes, reload derivation, CLI/messaging recap). This PR is deliberately narrower: an opt-in, desktop-only wiring that reuses the commentary engine already merged on
main(_fire_streamed_codex_commentary/interim_assistant_callback), with no changes tocodex_runtimeor the adapters. If the full lane in #62396 lands, I'm happy to close or rebase this — referencing it here so maintainers can weigh both.Tests
tests/tui_gateway/test_protocol.py— emitscommentary.deltawhen enabled; skipsalready_streamed/empty; omits the callback entirely when the lane is off (upstream-identical).commentary.test.tsx—commentary.delta→ commentary part (never reasoning); reasoning → commentary → tool → commentary → answer ordering; commentary stays above the answer in a single flush window; consecutive deltas coalesce.streaming.test.tsx— renders a separate "Working" disclosure, not under Thinking.commentary-lane.test.ts— the config-backed store round-tripscommentary_lane.tsc(renderer + electron) clean;vitest --project electronunaffected.Checklist