Skip to content

fix(desktop): sweep zombie optimistic rows on the reconnect seam + keep them out of the render cache - #361

Merged
Kyzcreig merged 3 commits into
mainfrom
fix/desktop-zombie-optimistic-rows
Jul 16, 2026
Merged

Kyzcreig merged 3 commits into
mainfrom
fix/desktop-zombie-optimistic-rows

Conversation

@Kyzcreig

Copy link
Copy Markdown
Collaborator

Symptom

Messages painted TWICE in the desktop app after a backend restart — persisting across app relaunches. DB clean (total==distinct), pure render corruption. Live incident 2026-07-15.

Root cause (two compounding halves)

  1. Severed stamp: a backend restart / WS reconnect can kill the message.complete frame carrying message_ids (fix(desktop): stop live-sync duplicating every message on send #352's stamping path). The client's optimistic rows (user-<ts>, assistant-stream-<ts>) keep their client-minted ids; the session-sync poll re-fetches the committed rows and the id-only dedupe appends them as duplicates beside the zombies.
  2. Cache re-infection: the render-cache write-through persisted the zombie rows to disk (transcript-<sid>.json), so a relaunch re-painted them and the next poll duplicated again — relaunching did NOT fix it. Observed: assistant-stream-1784172446416 cached beside its committed twin (row 695173).

Fix (3 narrow changes)

  • appendFetchedMessages sweeps zombies via new dropZombieOptimisticRows: optimistic, non-pending row with exact (role, text) match to an incoming committed row is dropped in favor of the committed twin. Guardrails: committed rows untouchable; pending/streaming rows exempt; empty-text rows exempt; one zombie per incoming row (genuine repeats survive).
  • pushTranscriptToRenderCache persists committed rows only (prefix-allowlist isCommittedTranscriptRow; unknown string ids fail open).
  • normalizeCachedTranscriptRows drops legacy optimistic rows already on disk before painting; all-optimistic file = cache miss; pass-through path preserves array identity (existing switch-cache-paint contract).

Tests

6 reconnect-seam cases + 4 cache-hygiene cases, driven against the REAL functions (no mocks of the unit under test). RED-proven both halves: reverting the merge-loop line fails 2; reverting the cache filters fails 4. Full desktop suite 1320/1320, tsc clean.

…ep them out of the render cache

A backend restart / WS reconnect can sever the message.complete stamping path
(#352): the turn's committed rows land in state.db but the completion frame
never reaches the client, so its optimistic rows (user-<ts> / assistant-
stream-<ts>) keep their client-minted ids. The id-only dedupe in
appendFetchedMessages then paints the polled committed rows as DUPLICATES
beside the zombies — and the render-cache write-through persisted the zombies
to disk, re-infecting every subsequent boot (observed live 2026-07-15:
assistant-stream-1784172446416 cached next to its committed twin 695173; DB
clean, pure render corruption; a relaunch did NOT clear it because the cache
re-seeded the zombie).

Three narrow changes:
1. appendFetchedMessages runs dropZombieOptimisticRows before appending: an
   optimistic, non-pending row whose (role, exact text) matches an incoming
   committed row is dropped in favor of the committed twin. Guardrails:
   committed rows untouchable, pending (streaming) rows exempt, empty-text
   rows exempt, each incoming row consumes at most one zombie.
2. pushTranscriptToRenderCache persists committed rows only (prefix-allowlist
   isCommittedTranscriptRow; unknown string ids fail open).
3. normalizeCachedTranscriptRows drops legacy optimistic rows already on disk
   before painting (all-optimistic file = cache miss), preserving array
   identity on the pass-through path.

Tests: 6 reconnect-seam cases in use-session-changes.test.ts (zombie drop for
user+assistant, pending exempt, no over-collapse, one-zombie-per-row budget,
role mismatch) + 4 cache-hygiene cases in render-cache-hydration.test.ts.
RED-proven: reverting the merge-loop line fails 2, reverting the cache filters
fails 4. Full desktop suite 1320/1320, tsc clean.
@greptile-apps

greptile-apps Bot commented Jul 16, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR fixes a live incident (2026-07-15) where messages painted twice in the desktop app after a backend restart. The root cause was two compounding problems: the message.complete frame carrying committed IDs could be severed by a WS reconnect, leaving optimistic rows (zombie user-<ts> / assistant-stream-<ts> IDs) that the id-only dedupe treated as distinct from their committed DB twins; and the render cache persisted those zombies to disk so every relaunch re-seeded the duplicates.

  • dropZombieOptimisticRows uses (role, normalized-text) as a content join key to match each zombie to its arriving committed twin, with guards for pending/empty-text/committed rows and a per-key budget that preserves genuine duplicate sends.
  • isCommittedTranscriptRow + pushTranscriptToRenderCache filter out client-minted optimistic IDs before writing to the render cache, and normalizeCachedTranscriptRows drops any legacy zombie rows already on disk before painting.

Confidence Score: 5/5

Safe to merge. The fix is narrow, well-guarded, and backed by 10 RED-proven tests run against real functions with no mocks of the units under test.

The zombie sweep and cache-hygiene changes touch only the render layer — no DB writes, no network calls. Every guard (committed-row protection, pending-row exemption, empty-text exemption, per-key budget) has a corresponding test verified to fail before the fix. The MEDIA-tag normalization edge case raised in a prior review thread was addressed correctly by making renderMediaTags idempotent on both sides of the join key.

No files require special attention. The four changed files are self-contained and the test files drive the exact functions changed.

Important Files Changed

Filename Overview
apps/desktop/src/app/chat/hooks/use-session-changes.ts Adds isOptimisticRowId and dropZombieOptimisticRows; wires the sweep into appendFetchedMessages. All guards correctly applied with MEDIA-tag normalization on both sides of the join key.
apps/desktop/src/app/render-cache-hydration.ts Adds isCommittedTranscriptRow with prefix allowlist; applied in pushTranscriptToRenderCache and normalizeCachedTranscriptRows. Array identity preserved when no filtering needed.
apps/desktop/src/app/chat/hooks/use-session-changes.test.ts Six new reconnect-seam test cases driven against real functions, covering all guards and the MEDIA-tag normalization edge case.
apps/desktop/src/app/render-cache-hydration.test.ts Four new cache-hygiene test cases covering write-through filtering, skip-when-all-optimistic, legacy zombie drop, and all-optimistic cache-miss.

Reviews (3): Last reviewed commit: "merge fork/main" | Re-trigger Greptile

Comment thread apps/desktop/src/app/chat/hooks/use-session-changes.ts Outdated
Kyzcreig added 2 commits July 15, 2026 21:25
…ey (Greptile #361 P2)

Committed assistant rows arrive media-rendered (assistantTextPart ->
renderMediaTags) while a streamed zombie may hold the raw MEDIA: line;
normalize both sides through the idempotent renderMediaTags so the
(role, text) join key is representation-stable. +1 test.
@Kyzcreig
Kyzcreig merged commit 053efc1 into main Jul 16, 2026
24 checks passed
@Kyzcreig
Kyzcreig deleted the fix/desktop-zombie-optimistic-rows branch July 16, 2026 04:40
Kyzcreig added a commit that referenced this pull request Jul 16, 2026
…pts a committed twin (#362)

The #361 zombie sweep drops an un-stamped optimistic row in favor of its
polled committed twin — but the runtime footer travels ONLY on the
message.complete frame and lives on that optimistic row (DB rows never carry
it). So the sweep silently traded a footer-bearing zombie for a footer-less
committed row: footer visible live, gone one poll later (regression of the
#357 behavior, observed live 2026-07-16).

dropZombieOptimisticRows now harvests the swept zombie's footer (keyed by the
same media-normalized content key) and adoptZombieFooters grafts it onto the
adopting committed row. Guardrails: assistant rows only, never overwrites an
existing footer, one graft per harvested key.

Tests: transplant case (RED without the graft: footer undefined after adopt)
+ no-overwrite case. Suite 1323/1323, tsc clean.

Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
Kyzcreig added a commit that referenced this pull request Jul 18, 2026
… ids onto stale zombies (re-land of #364)

Re-lands #364 (author: Kyzcreig; branch was 705 commits behind) onto current main. CDP-attach caught the mechanism live 2026-07-15: a stale optimistic assistant row (completion frame severed by a backend restart) absorbed the fresh turn user_id via the old top-down role-blind walk — the zombie wore a committed id (invisible to the #361 sweep) and painted as a permanent duplicate. stampOptimisticTranscriptRows now assigns role-aware and tail-first.

Both LIVE Greptile #364 findings adjudicated: (1) silent user-id drop when no user-role stampable row exists is DELIBERATE and now documented + test-pinned — cross-role fallback would re-create the exact mis-stamp this fixes; the poll reconciles the committed row safely. (2) 3+-id frames (array + scalar fields both populated) now documented + test-pinned: assistant-preferring tail-first walk, extras left for the poll, no crash.

RED-proven: swapping back the fork/main top-down impl fails 5/36 (the 3 zombie vectors + both edge-frame tests). Hook suite 36 green; full desktop suite delta vs clean fork/main baseline = zero new failures (17 pre-existing reds on both, 1 local-env flake passes 3/3 in isolation); tsc errors identical to baseline (7, all pre-existing).

Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
Kyzcreig added a commit that referenced this pull request Jul 18, 2026
* fix(desktop): role-aware tail-first stamping — stop stamping the turn ids onto stale zombies (re-land of #364)

Re-lands #364 (author: Kyzcreig; branch was 705 commits behind) onto current main. CDP-attach caught the mechanism live 2026-07-15: a stale optimistic assistant row (completion frame severed by a backend restart) absorbed the fresh turn user_id via the old top-down role-blind walk — the zombie wore a committed id (invisible to the #361 sweep) and painted as a permanent duplicate. stampOptimisticTranscriptRows now assigns role-aware and tail-first.

Both LIVE Greptile #364 findings adjudicated: (1) silent user-id drop when no user-role stampable row exists is DELIBERATE and now documented + test-pinned — cross-role fallback would re-create the exact mis-stamp this fixes; the poll reconciles the committed row safely. (2) 3+-id frames (array + scalar fields both populated) now documented + test-pinned: assistant-preferring tail-first walk, extras left for the poll, no crash.

RED-proven: swapping back the fork/main top-down impl fails 5/36 (the 3 zombie vectors + both edge-frame tests). Hook suite 36 green; full desktop suite delta vs clean fork/main baseline = zero new failures (17 pre-existing reds on both, 1 local-env flake passes 3/3 in isolation); tsc errors identical to baseline (7, all pre-existing).

Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>

* fix(desktop): 3+-id frames keep the positional [user, assistant] pair role-aware (Greptile #398)

---------

Co-authored-by: Apollo <apollo@kyzcreig.local>
Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
@Kyzcreig
Kyzcreig restored the fix/desktop-zombie-optimistic-rows branch September 21, 2026 10:32
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.

1 participant