Skip to content

Handle item/reasoning/textDelta notifications#165

Merged
friuns2 merged 1 commit into
friuns2:mainfrom
lxistired:fix/reasoning-text-delta
May 12, 2026
Merged

Handle item/reasoning/textDelta notifications#165
friuns2 merged 1 commit into
friuns2:mainfrom
lxistired:fix/reasoning-text-delta

Conversation

@lxistired
Copy link
Copy Markdown

Problem

codex emits the full reasoning-chain stream as item/reasoning/textDelta in addition to the item/reasoning/summaryTextDelta summary stream. useDesktopState.ts only recognises the summary variant, so reasoning text streamed via the textDelta channel is dropped — the UI shows only the summary, which makes long thinking phases look like a stall ("Thinking…" with no streaming text for tens of seconds).

Change

Add the item/reasoning/textDelta variant to:

  • readReasoningDelta() — same params shape as the existing summary handler ({ itemId, delta }), routed to liveReasoningTextByThreadId via liveReasoningMessageId(itemId)
  • the Thinking activity recognition (so the activity label still flips correctly)

Both additions mirror the existing summaryTextDelta handling exactly — no new code paths, just one more recognised method name in each spot.

Notes

  • 13 insertions / 1 deletion, all in src/composables/useDesktopState.ts.
  • Verified against codex 0.130 (item/reasoning/textDelta is in the app-server protocol; gpt-5.x with model_reasoning_summary = "detailed" is the common trigger for the channel carrying content).
  • No behaviour change for existing summary-only sessions — the extra branch only fires on the new method name.

codex emits the full reasoning-chain stream as item/reasoning/textDelta
in addition to the item/reasoning/summaryTextDelta summary stream. The
notification handler only recognised the summary variant, so any reasoning
text streamed via the textDelta channel was dropped — the UI showed only
the summary, which makes long thinking phases look like a stall.

Add the textDelta variant to both readReasoningDelta() and the Thinking
activity recognition, mirroring the existing summaryTextDelta handling
exactly (same params shape: { itemId, delta }).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Handle item/reasoning/textDelta notifications for full reasoning streams

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Handle item/reasoning/textDelta notifications alongside existing summary stream
• Add textDelta variant to reasoning delta reader for full reasoning chain
• Include textDelta in Thinking activity recognition logic
• Prevent reasoning text loss during long model thinking phases
Diagram
flowchart LR
  A["item/reasoning/textDelta<br/>notification"] -->|"recognized in<br/>readReasoningDelta()"| B["liveReasoningTextByThreadId<br/>updated with delta"]
  A -->|"recognized in<br/>activity detection"| C["Thinking activity<br/>label maintained"]
  B --> D["UI displays full<br/>reasoning stream"]
  C --> D
Loading

Grey Divider

File Changes

1. src/composables/useDesktopState.ts 🐞 Bug fix +13/-1

Add textDelta variant to reasoning stream handlers

• Added item/reasoning/textDelta method check to Thinking activity recognition alongside existing
 summaryTextDelta and summaryPartAdded checks
• Implemented new handler block in readReasoningDelta() to process item/reasoning/textDelta
 notifications with same parameter shape as summary variant
• Both additions mirror existing summaryTextDelta handling to route full reasoning chain stream to
 liveReasoningTextByThreadId
• Prevents reasoning text loss during long thinking phases by capturing both summary and full
 reasoning streams

src/composables/useDesktopState.ts


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 12, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1)

Grey Divider


Action required

1. Missing tests.md for textDelta 📘 Rule violation ⚙ Maintainability
Description
This PR adds a new UI-facing feature by handling item/reasoning/textDelta, but it does not add
corresponding manual test instructions in the repository-root tests.md. Without updated test
steps, reviewers/maintainers lack a reproducible way to validate the new streaming behavior.
Code

src/composables/useDesktopState.ts[R3270-3279]

+    // codex also emits the full reasoning-chain stream as item/reasoning/textDelta
+    // (alongside the summary stream). Without handling it, reasoning text the
+    // model streams via this channel is dropped and the UI shows only the
+    // summary, making long thinking phases look like a stall.
+    if (notification.method === 'item/reasoning/textDelta') {
+      const itemId = readString(params.itemId)
+      const delta = readString(params.delta)
+      if (!itemId || !delta) return null
+      return { messageId: liveReasoningMessageId(itemId), delta }
+    }
Evidence
PR Compliance ID 1 requires updating the root tests.md after implementing a feature. The diff adds
new handling for item/reasoning/textDelta (a user-visible streaming change), while the current
tests.md content does not include any manual test section covering this new notification/streaming
behavior.

AGENTS.md
src/composables/useDesktopState.ts[3084-3096]
src/composables/useDesktopState.ts[3258-3282]
tests.md[5023-5081]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A feature was implemented (handling `item/reasoning/textDelta` for the Thinking activity + reasoning delta streaming) but `tests.md` was not updated with manual verification steps.

## Issue Context
The change affects UI streaming behavior during long thinking phases; manual verification should explicitly cover both light and dark themes.

## Fix Focus Areas
- tests.md[5023-5081]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Reasoning delta stream mixing 🐞 Bug ≡ Correctness
Description
readReasoningDelta() now accepts both item/reasoning/summaryTextDelta and item/reasoning/textDelta,
and applyRealtimeUpdates() appends any returned delta into the same liveReasoningTextByThreadId
string. If both streams are emitted for the same reasoning phase (as implied by the new comment),
the UI overlay will show interleaved/duplicated content rather than a single coherent stream.
Code

src/composables/useDesktopState.ts[R3270-3279]

+    // codex also emits the full reasoning-chain stream as item/reasoning/textDelta
+    // (alongside the summary stream). Without handling it, reasoning text the
+    // model streams via this channel is dropped and the UI shows only the
+    // summary, making long thinking phases look like a stall.
+    if (notification.method === 'item/reasoning/textDelta') {
+      const itemId = readString(params.itemId)
+      const delta = readString(params.delta)
+      if (!itemId || !delta) return null
+      return { messageId: liveReasoningMessageId(itemId), delta }
+    }
Evidence
The PR adds a new item/reasoning/textDelta branch returning the same {delta} shape as
summaryTextDelta; downstream, all returned deltas are appended to the same per-thread reasoning
text buffer with no stream selection, so supporting both methods can combine their outputs into one
string.

src/composables/useDesktopState.ts[3258-3282]
src/composables/useDesktopState.ts[3757-3760]
src/composables/useDesktopState.ts[2452-2456]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`readReasoningDelta()` can now return deltas for both `item/reasoning/summaryTextDelta` and `item/reasoning/textDelta`, and callers append every returned `delta` into the same per-thread `liveReasoningTextByThreadId` buffer. This makes the live overlay content ambiguous and can interleave/duplicate text when both delta streams are present.

## Issue Context
- `applyRealtimeUpdates()` appends `readReasoningDelta(notification).delta` directly into `liveReasoningTextByThreadId`.
- The new code comment explicitly states `textDelta` can be emitted “alongside the summary stream”, so the code should define precedence/selection.

## Fix Focus Areas
- src/composables/useDesktopState.ts[3258-3282]
- src/composables/useDesktopState.ts[3757-3760]
- src/composables/useDesktopState.ts[2452-2456]

## Suggested fix approach
- Decide a single source-of-truth stream per active reasoning item/thread:
 - Prefer `item/reasoning/textDelta` when observed, and ignore `summaryTextDelta` thereafter for that reasoning item/thread, OR
 - Store both streams separately (e.g., `liveReasoningFullTextByThreadId` vs `liveReasoningSummaryByThreadId`) and choose which to display.
- Implement a small piece of state (e.g., `reasoningDeltaModeByThreadId` or by `activeReasoningItemId`) to prevent mixing once one stream is selected.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

3. Duplicate delta parsing 🐞 Bug ⚙ Maintainability
Description
The new item/reasoning/textDelta handler duplicates the summaryTextDelta parsing/validation logic
verbatim, increasing the likelihood of future fixes being applied to only one branch and causing
inconsistent behavior.
Code

src/composables/useDesktopState.ts[R3274-3279]

+    if (notification.method === 'item/reasoning/textDelta') {
+      const itemId = readString(params.itemId)
+      const delta = readString(params.delta)
+      if (!itemId || !delta) return null
+      return { messageId: liveReasoningMessageId(itemId), delta }
+    }
Evidence
Both the existing and newly-added branches perform the same itemId/delta extraction, identical
validation, and identical return shape, differing only by notification.method string.

src/composables/useDesktopState.ts[3262-3279]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `summaryTextDelta` and `textDelta` branches in `readReasoningDelta()` repeat identical parsing/validation logic (`itemId`, `delta`, null checks, return shape). This duplication increases future maintenance risk.

## Issue Context
Both branches extract `params.itemId` and `params.delta`, validate non-empty, and return `{ messageId: liveReasoningMessageId(itemId), delta }`.

## Fix Focus Areas
- src/composables/useDesktopState.ts[3258-3282]

## Suggested fix approach
- Extract a small helper inside `readReasoningDelta()` (or nearby) such as `parseReasoningDelta(params): {messageId, delta} | null`.
- Combine the method checks:
 - `if (notification.method === 'item/reasoning/summaryTextDelta' || notification.method === 'item/reasoning/textDelta') { ... }`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment on lines +3270 to +3279
// codex also emits the full reasoning-chain stream as item/reasoning/textDelta
// (alongside the summary stream). Without handling it, reasoning text the
// model streams via this channel is dropped and the UI shows only the
// summary, making long thinking phases look like a stall.
if (notification.method === 'item/reasoning/textDelta') {
const itemId = readString(params.itemId)
const delta = readString(params.delta)
if (!itemId || !delta) return null
return { messageId: liveReasoningMessageId(itemId), delta }
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Missing tests.md for textdelta 📘 Rule violation ⚙ Maintainability

This PR adds a new UI-facing feature by handling item/reasoning/textDelta, but it does not add
corresponding manual test instructions in the repository-root tests.md. Without updated test
steps, reviewers/maintainers lack a reproducible way to validate the new streaming behavior.
Agent Prompt
## Issue description
A feature was implemented (handling `item/reasoning/textDelta` for the Thinking activity + reasoning delta streaming) but `tests.md` was not updated with manual verification steps.

## Issue Context
The change affects UI streaming behavior during long thinking phases; manual verification should explicitly cover both light and dark themes.

## Fix Focus Areas
- tests.md[5023-5081]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@lxistired
Copy link
Copy Markdown
Author

By the way — we maintain a downstream fork of codexUI for a product we're building, and we've been doing some work on top of it that might be useful upstream. This PR is one piece; we've also done perf work on the SSE delta path (batching delta-driven re-renders per animation frame) and a richer /goal status display on top of codex 0.130's goals feature (turn counter, live elapsed time, streaming reasoning preview in the banner). Happy to send those as separate PRs if you'd want them.

More broadly: if you'd ever be interested in being involved on the product side, we'd be glad to talk — you know this codebase better than anyone. No pressure at all; even "just keep sending PRs" is a fine answer. If it's easier to chat off GitHub, let me know how to reach you.

@friuns2 friuns2 merged commit 48fb701 into friuns2:main May 12, 2026
@friuns2
Copy link
Copy Markdown
Owner

friuns2 commented May 12, 2026

sure thanks, what is the fork? i couldnt find i dont see any changes there https://github.com/lxistired/codexui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants