fix(desktop): role-aware tail-first stamping — stop stamping the turn's ids onto stale zombies

[Kyzcreig]

Symptom (last unexplained double-paint mechanism)

In ONE long, restart-scarred session: the newest assistant reply painted twice, DB and render cache both verifiably clean, fresh sessions unaffected. Survived #352/#357/#361/#362.

Root cause — caught live via CDP-attach to the running app

The session held a stale optimistic assistant row (completion frame severed by an earlier backend restart). On the next turn's message.complete, the stamp walked non-committed rows top-down and role-blind: the frame's user_id landed on the stale ASSISTANT zombie, assistant_id on the fresh USER row. The zombie now wore a committed id — invisible to the #361 sweep, which only targets optimistic ids — so its stale text painted as a permanent duplicate and the fresh rows stayed un-stamped. Deterministically 'the last message'.

Fix

Role-aware, tail-first assignment in stampOptimisticTranscriptRows: with the [user_id, assistant_id] contract each id claims the last unclaimed stampable row of its role; a lone id prefers the assistant streamed row. Stale zombies keep optimistic ids → remain sweepable by #361 when their committed twins arrive via the poll (covered end-to-end in the new tests).

Tests

4 new cases; RED-proof: restoring the old top-down walk fails 3 of them. Full desktop suite 1334/1334, tsc clean.

[greptile-apps]

Greptile Summary

This PR replaces the old top-down, role-blind stamping walk in stampOptimisticTranscriptRows with a role-aware, tail-first assignment strategy to prevent stale optimistic assistant rows (zombies from severed backend-restart sessions) from being stamped with the wrong committed IDs and becoming unsweepable duplicates.

• Core fix: The new claim() helper scans stampable[] from tail to head and filters by role, so [user_id, assistant_id] each go to the last unclaimed row of their respective role; zombies that don't match keep their optimistic IDs and remain sweepable by the #361 poll.
• Lone-id path: When exactly one committed ID arrives, the code prefers the last assistant-role row (the dup-prone streamed bubble) and falls back to any row — mirroring the historical intent.
• Tests: Four new RED-proof cases cover the mis-stamp scenario, the end-to-end sweep after stamping, the normal send path, and lone-id preference.

Confidence Score: 4/5

Safe to merge for the targeted bug; the two edge cases flagged are unlikely in normal operation and do not affect the happy path or the documented zombie scenario.

The core role-aware stamping logic is correct and well-tested against the specific live regression. Two edge cases in the else branch deserve attention: when extractCommittedMessageIds produces more than two IDs (e.g., both an array field and a scalar field are non-empty in the same payload), the loop applies assistant-first preference to every ID including the positional user ID, which can reproduce the very cross-role mis-stamp this PR fixes. The two-id path also silently drops the user ID rather than falling back if no user-role stampable row is found.

The else branch in stampOptimisticTranscriptRows in use-session-changes.ts handles the length-not-2 case in a way that conflates lone-ID and 3+-ID payloads — worth a second look before extending the backend contract.

Important Files Changed

Filename	Overview
apps/desktop/src/app/chat/hooks/use-session-changes.ts	Replaces the top-down role-blind stamp walk with a role-aware tail-first assignment strategy; the two-id path has no fallback when a role has no matching stampable row, and the else branch silently handles length > 2 with assistant-first logic that may mis-assign positional user IDs.
apps/desktop/src/app/chat/hooks/use-session-changes.test.ts	Adds 4 targeted new test cases covering the stale-zombie mis-stamp scenario, end-to-end sweep, normal send path, and lone-id preference; all cases are well-structured and RED-proof per the PR description.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["message.complete fires\n[user_id, assistant_id]"] --> B["stampOptimisticTranscriptRows"]
    B --> C{"committedIds.length?"}
    C -- "0" --> D["return unchanged (early exit)"]
    C -- "2" --> E["Build stampable[] from optimistic/pending rows"]
    C -- "1 or >2" --> F["else branch\n(lone-id loop)"]
    E --> G["claim(user_id, role='user')\ntail-first scan"]
    E --> H["claim(assistant_id, role='assistant')\ntail-first scan"]
    G -- "found" --> I["assign user_id to last user-role row"]
    G -- "not found" --> J["user_id dropped\n(no fallback)"]
    H -- "found" --> K["assign assistant_id to last assistant-role row"]
    H -- "not found" --> L["assistant_id dropped\n(no fallback)"]
    F --> M["for each id:\ntry claim(id, 'assistant')"]
    M -- "success" --> N["assigned to last assistant row"]
    M -- "failure" --> O["fallback: claim(id, null)\nassign to last any-role row"]
    I & K --> P["Stale zombies keep\noptimistic ids → sweepable"]
    P --> Q["Next poll: dropZombieOptimisticRows\nreplaces zombie with committed twin"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["message.complete fires\n[user_id, assistant_id]"] --> B["stampOptimisticTranscriptRows"]
    B --> C{"committedIds.length?"}
    C -- "0" --> D["return unchanged (early exit)"]
    C -- "2" --> E["Build stampable[] from optimistic/pending rows"]
    C -- "1 or >2" --> F["else branch\n(lone-id loop)"]
    E --> G["claim(user_id, role='user')\ntail-first scan"]
    E --> H["claim(assistant_id, role='assistant')\ntail-first scan"]
    G -- "found" --> I["assign user_id to last user-role row"]
    G -- "not found" --> J["user_id dropped\n(no fallback)"]
    H -- "found" --> K["assign assistant_id to last assistant-role row"]
    H -- "not found" --> L["assistant_id dropped\n(no fallback)"]
    F --> M["for each id:\ntry claim(id, 'assistant')"]
    M -- "success" --> N["assigned to last assistant row"]
    M -- "failure" --> O["fallback: claim(id, null)\nassign to last any-role row"]
    I & K --> P["Stale zombies keep\noptimistic ids → sweepable"]
    P --> Q["Next poll: dropZombieOptimisticRows\nreplaces zombie with committed twin"]

Loading

_{Reviews (1): Last reviewed commit: "fix(desktop): role-aware tail-first stam..." | Re-trigger Greptile}

[greptile-apps]

The else branch handles every committedIds.length !== 2 case — including lengths of 3 or more — but its logic and comment only make sense for exactly one ID. extractCommittedMessageIds can return 3+ IDs when both an array field (message_ids) and a scalar field (message_id) are non-empty in the same payload. In that situation the for loop calls claim(id, 'assistant') for every ID, including committedIds[0] which is the positional user ID — repeating the exact cross-role mis-stamp this PR fixes, just for the >2 case. Tightening the condition to === 1 keeps the assistant-preference logic scoped to its documented intent.

Suggested change

	if (committedIds.length === 2) {
	claim(committedIds[0], 'user')
	claim(committedIds[1], 'assistant')
	} else {
	// Lone id: the [user, assistant] positional contract can't disambiguate a
	// single survivor. Prefer the streamed assistant row (the dup-prone one —
	// the poll always re-fetches the final text bubble), fall back to any.
	for (const id of committedIds) {
	if (!claim(id, 'assistant')) {
	claim(id, null)
	}
	}
	}
	if (committedIds.length === 2) {
	claim(committedIds[0], 'user')
	claim(committedIds[1], 'assistant')
	} else if (committedIds.length === 1) {
	// Lone id: the [user, assistant] positional contract can't disambiguate a
	// single survivor. Prefer the streamed assistant row (the dup-prone one —
	// the poll always re-fetches the final text bubble), fall back to any.
	if (!claim(committedIds[0], 'assistant')) {
	claim(committedIds[0], null)
	}
	} else {
	// Unexpected count (>2 or 0 after the early-exit guard): fall back to
	// role-blind tail-first assignment so at least the tail rows get stamped.
	for (const id of committedIds) {
	claim(id, null)
	}
	}

[greptile-apps]

Silent drop when no user-role stampable row exists

In the two-id branch, claim(committedIds[0], 'user') returns false when there are no unclaimed user-role rows in stampable — and the return value is silently discarded. Unlike the lone-id path (which falls back to claim(id, null)), the user ID is simply lost. This is fine if the user row was already committed before this stamp fires, but it could silently drop a valid user ID in any edge case where the optimistic user row was cleared without getting a committed integer ID. Adding a fallback claim(committedIds[0], null) when the role-specific claim fails would mirror the lone-id path's defensive behaviour.

[Kyzcreig]

Superseded by #397 — a re-land onto current main (this branch is 705 commits behind and conflicts with the livesync Phase-1 hook changes). Both LIVE Greptile findings are adjudicated there: the no-user-row silent drop is deliberate (documented + test-pinned — cross-role fallback would re-create the mis-stamp), and 3+-id frames are handled + test-pinned. RED-proven against the old impl. Authorship credited via Co-authored-by.

[Kyzcreig]

Correction: the re-land PR is #398 (not #397).