Skip to content

fix(desktop): preserve mid-turn message order - #73882

Closed
carbongotfound wants to merge 1 commit into
NousResearch:mainfrom
carbongotfound:agent/preserve-midturn-message-order
Closed

fix(desktop): preserve mid-turn message order#73882
carbongotfound wants to merge 1 commit into
NousResearch:mainfrom
carbongotfound:agent/preserve-midturn-message-order

Conversation

@carbongotfound

Copy link
Copy Markdown
Contributor

What changed

Reordered the live-session projection so mid-turn correction messages are appended after the assistant output that was already streaming when the correction arrived.

Why

The live renderer updates the assistant bubble in place, then appends a mid-turn user message. On resume or reload, appendLiveSessionProjection() rebuilt the same data as prompt, correction, assistant. That moved the correction above all partial assistant output, including content the user had already read.

The projection now matches the visible arrival order: prompt, current assistant stream, corrections, then any queued next-turn prompt. Existing persisted corrections are still deduplicated.

Closes #73793.

Validation

  • npm run --workspace apps/desktop test:ui -- src/app/session/hooks/use-session-actions/utils.test.ts (42 tests)
  • npm exec --workspace apps/desktop -- eslint src/app/session/hooks/use-session-actions/utils.ts src/app/session/hooks/use-session-actions/utils.test.ts
  • npm run --workspace apps/desktop typecheck
  • git diff --check

@carbongotfound
carbongotfound marked this pull request as ready for review July 29, 2026 05:58
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) area/sessions Session lifecycle, resume, persistence, history P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 29, 2026
@carbongotfound
carbongotfound force-pushed the agent/preserve-midturn-message-order branch from fe0f63e to 167c5fa Compare July 30, 2026 01:09

@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 regression test. The proposed reorder conflicts with the current live transcript contract.

Problems

  • The primary redirect path explicitly inserts the correction before the active reply before awaiting the RPC (apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts:686-690); its helper inserts at the active stream or latest-assistant index (:267-280). Session tiles use the same placement (apps/desktop/src/app/chat/session-tile-actions.ts:287-304).
  • appendLiveSessionProjection already matches that live ordering—prompt, corrections, then assistant-stream—at apps/desktop/src/app/session/hooks/use-session-actions/utils.ts:510-535. This was intentionally established by 8a21df18acbe73c63d06747d0ab359288bf84276 (fix(desktop): place steer messages before redirected replies).

Suggested changes

  • Retain the existing projection order and restore the prior test expectation. An arrival-order redesign would need to update the live redirect paths and recovery contract together, rather than changing only resume projection.

Automated hermes-sweeper review.

@@ -494,6 +478,23 @@ export function appendLiveSessionProjection(
})

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 reverses the order used by both live redirect renderers: the primary path passes insertBeforeActiveReply: true (use-prompt-actions/index.ts:686-690) and session tiles insert before streamId/the last assistant row (session-tile-actions.ts:287-304). Keeping this block before the assistant projection is necessary for resume to match the live transcript.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
@carbongotfound

Copy link
Copy Markdown
Contributor Author

Verified the review independently against current upstream/main — you're right, and I'm closing this rather than pushing a fix on a broken premise.

Both live redirect paths insert the correction before the active reply, not after:

  • apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts:690appendSessionTextMessage(..., { insertBeforeActiveReply: true }), whose helper (:267-280) inserts at streamId or the last assistant index.
  • apps/desktop/src/app/chat/session-tile-actions.ts:287-304 — same insertion logic, explicitly commented as matching the primary composer.

appendLiveSessionProjection (prompt → corrections → assistant-stream) already matches that, per 8a21df18a (#69739, "place steer messages before redirected replies"). This PR's reorder makes the projection agree with the reported symptom but disagree with the actual live contract — trading a resume-time bug for a live-vs-resume mismatch in the other direction.

The real bug in #73793 is real, though: a mid-turn correction is inserted once, before a single assistant bubble that keeps mutating in place for the rest of the turn — so by the time the turn ends, content generated before the correction (which the user already watched appear above it) ends up rendered below it too. Fixing that properly means splitting the in-flight assistant bubble at each correction boundary — both live paths and the resume/recovery projection need to agree on that split, which likely also needs the backend inflight payload to carry boundary info it doesn't currently expose. That's a larger, cross-cutting change than this PR's scope.

Closing this one rather than merging a projection-only reorder that's confirmed to conflict with the established contract. Happy to see a follow-up PR that tackles the bubble-splitting approach end-to-end if someone wants to take that on — #73793 stays open for it.

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

Labels

area/sessions Session lifecycle, resume, persistence, history 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: mid-turn messages render above assistant output that predates them

3 participants