Skip to content

fix(desktop): keep fuller pending assistant over empty inflight shell - #75967

Closed
arimu1 wants to merge 1 commit into
NousResearch:mainfrom
arimu1:fix/75825-preserve-complete-pending-assistant
Closed

fix(desktop): keep fuller pending assistant over empty inflight shell#75967
arimu1 wants to merge 1 commit into
NousResearch:mainfrom
arimu1:fix/75825-preserve-complete-pending-assistant

Conversation

@arimu1

@arimu1 arimu1 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a Desktop transcript bug where switching sessions while a turn streams (or right as it completes) can leave the assistant reply missing from the UI until restart, even though the streamed content was still held in the local slice.

When resuming, reconciliation merges stored history + the backend inflight projection (fixed id assistant-stream-${sessionId}, often empty or lagging) with local pending messages. preserveLocalPendingTurnMessages previously dropped the local pending assistant whenever any authoritative assistant existed at the same role-ordinal — including an empty projection shell — so the fuller streamed row was discarded and nothing repopulated it after stream events had stopped.

Related Issue

Fixes #75825

Related but not duplicated:

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • reconcileResumeMessages: prefer a richer local pending assistant when the authoritative/projected text is empty or a strict prefix of the local stream text
  • preserveLocalPendingTurnMessages: when a same-ordinal authoritative assistant is less complete, replace that shell with the local pending row (do not append a second bubble); same-id shells get the same completeness preference
  • Regression tests for empty shell, lagging mid-stream shell, same-id shell, and “authoritative already further ahead”

How to Test

cd apps/desktop
../../node_modules/.bin/vitest run --project ui \
  src/app/session/hooks/use-session-actions/utils.test.ts

Expected: 50 passed

Manual:

  1. Start a turn in session A; switch to B while the assistant is still streaming (or right as it completes).
  2. Switch back to A (repeat A↔B a few times).
  3. Transcript should keep the assistant reply for the in-flight/recent turn — not user-only until restart.

Checklist

Code

Documentation & Housekeeping

  • Docs / config / AGENTS — N/A (no user-facing API or config change)
  • Cross-platform: renderer reconciliation only

When reconciling a session after mid-turn switch, an empty or lagging
inflight projection at the same role-ordinal no longer discards the local
pending assistant that still holds streamed content (NousResearch#75825).

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the focused transcript-reconciliation fix. The reported gap remains present on current main: apps/desktop/src/app/session/hooks/use-session-actions/utils.ts:411-415 drops a local pending assistant whenever a same-ordinal authoritative row exists, while utils.ts:515-524 can construct an empty assistant-stream-${sessionId} projection.

Problems

  • apps/desktop/src/app/session/hooks/use-session-actions/utils.ts:460 chooses the local assistant using only text length. A longer but unrelated local message can replace an authoritative assistant at the same ordinal. This conflicts with the stricter identity guard already used by this PR in reconcileResumeMessages at utils.ts:265-268.
  • The new tests at utils.test.ts:656-699 cover empty/prefix projections but not a shorter non-prefix authoritative body.

Suggested changes

  • Require the local text to be a strict extension of the authoritative text (or let an empty authoritative shell match) in both replacement branches, including the same-id branch at utils.ts:431.
  • Add non-prefix regressions for both same-id and different-id rows, asserting the authoritative row remains intact.

Automated hermes-sweeper review.

const localText = chatMessageText(message).trim()
const authoritativeText = chatMessageText(authoritative).trim()

if (authoritativeText.length >= localText.length) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This length-only comparison can replace a different authoritative assistant that merely happens to be shorter. Please mirror the strict-extension condition used above in reconcileResumeMessages (empty shell or localText.startsWith(authoritativeText)) and add a non-prefix regression.

@OutThisLife

Copy link
Copy Markdown
Collaborator

Confirmed the premise and the branch you targeted — thank you, this was the right diagnosis. Superseded by #77644, which keeps your commit as its base.

Two things pushed it to a salvage rather than a merge. The length-only comparison teknium1 flagged is still live in both replacement paths, so a longer unrelated local row can take an authoritative reply's ordinal or reuse its stream id. And the reporter's own first symptom — reply streaming, but no inference traces or tool calls — is a local row holding reasoning and tool calls with no text yet, where both bodies are empty text and length can't separate them.

Two more cases turned up while I was in there: an inflight.error snapshot is projected with empty text, so "empty shell wins" erased a failed turn and repainted it healthy; and returning the local row wholesale carried pending: true over a settled shell, leaving a finished reply spinning. #77644 routes all three sites through one guard pair and adds regressions for each — nine of them fail on bare main, nine on this branch as submitted.

@OutThisLife OutThisLife closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop: switching sessions while a turn streams can leave transcript showing only user messages until restart

4 participants