From 934ba02718f05ab47a2b0852864ee3bb7b646ffc Mon Sep 17 00:00:00 2001 From: brother7 <7brother7@gmail.com> Date: Sun, 23 Aug 2026 22:39:34 +0800 Subject: [PATCH] fix(coding): keep optimistic identity across snapshots --- .../20260823-pi-renderer-store-b7e2c4a1.md | 25 +++++++++---- src/stores/coding-conversations.ts | 24 +++++++++++- .../unit/coding-conversations-store.test.tsx | 37 +++++++++++++++++++ 3 files changed, 77 insertions(+), 9 deletions(-) diff --git a/.project-docs/30-worklog/tasks/20260823-pi-renderer-store-b7e2c4a1.md b/.project-docs/30-worklog/tasks/20260823-pi-renderer-store-b7e2c4a1.md index 9d6bcf8..6d5de97 100644 --- a/.project-docs/30-worklog/tasks/20260823-pi-renderer-store-b7e2c4a1.md +++ b/.project-docs/30-worklog/tasks/20260823-pi-renderer-store-b7e2c4a1.md @@ -107,7 +107,10 @@ and replayed in generation/sequence order after a valid Snapshot; unresolved pending/accepted/uncertain prompt requests re-overlay their local optimistic nodes after snapshot-first reconnect until a durable `clientRequestId` - reconciliation arrives. Neither mechanism replays a mutation. + reconciliation arrives. If the reconnect Snapshot already contains that + durable message, its message id is first reconciled to the local request's + stable UI node id before the request is removed. Neither mechanism replays a + mutation. - Added optimistic user nodes keyed by `clientRequestId`. Durable upsert keeps the optimistic UI node id; definite rejection restores an untouched draft and marks the node failed; uncertain delivery restores the draft while @@ -129,11 +132,16 @@ - `pnpm exec vitest run tests/unit/coding-conversations-facade.test.ts tests/unit/coding-conversations-store.test.tsx`: initial implementation 2 files / 12 tests passed; after review fixes, 2 files - / 13 tests passed, including delayed gap GET + concurrent patch replay and - reconnect Snapshot optimistic-node continuity. + / 14 tests passed, including delayed gap GET + concurrent patch replay, + reconnect Snapshot optimistic-node continuity, and a reconnect Snapshot that + already contains the durable user message while preserving `node-1`. - `pnpm exec vitest run` for the two PI-110 tests plus PI-010 contracts, coding core routes, event projector, Conversation runtime, product tools, - and subagent suites: 8 files / 75 tests passed. + and subagent suites: initial 8 files / 75 tests; final review-fix range 8 + files / 77 tests passed. One intervening parallel run hit a non-reproducible + `CODING_STORAGE_WRITE_FAILED` in the unrelated Conversation runtime temp + persistence test; that file passed alone immediately afterward and the same + 8-file command then passed in full. - `pnpm run typecheck`: passed after the shared product seam removed the clean Renderer -> composite Electron declaration dependency. - `pnpm run lint:check`: passed with zero errors and the same six unrelated @@ -150,9 +158,12 @@ PI-150. Neither is recorded as Pass. - Planner review of `a072257...36626c8`: Standards PASS / 0 findings; Spec Needs Fix with two P1 findings for recovery-window patch loss and reconnect - Snapshot optimistic-node loss. Both independent reproductions were accepted - and fixed locally with the focused regressions above; fixed-range re-review - remains required before PI-110 can be marked Done. + Snapshot optimistic-node loss. The first fixed-range re-review accepted gap + replay and missing-durable-node overlay, then found one remaining P1 where a + Snapshot already containing the durable node adopted the server id. That + reproduction is now fixed by `clientRequestId` id reconciliation before + request cleanup, with the focused regression above. Final fixed-range + re-review remains required before PI-110 can be marked Done. ## Follow-ups diff --git a/src/stores/coding-conversations.ts b/src/stores/coding-conversations.ts index 7046c5e..1c725da 100644 --- a/src/stores/coding-conversations.ts +++ b/src/stores/coding-conversations.ts @@ -160,6 +160,22 @@ function withoutReconciledRequests( ); } +function withReconciledOptimisticNodeIds( + snapshot: ConversationSnapshot, + requests: Record | undefined, +): ConversationSnapshot { + if (!requests) return snapshot; + let changed = false; + const nodes = snapshot.nodes.map((node) => { + if (node.kind !== 'message' || !node.clientRequestId) return node; + const nodeId = requests[node.clientRequestId]?.nodeId; + if (!nodeId || node.id === nodeId) return node; + changed = true; + return { ...node, id: nodeId }; + }); + return changed ? { ...snapshot, nodes } : snapshot; +} + function optimisticNode( nodeId: string, clientRequestId: string, @@ -601,12 +617,16 @@ export function createCodingConversationStore( if (!current.reducer.invalidation && snapshotIsOlder(current.reducer.snapshot, snapshot)) { return state; } + const reconciledSnapshot = withReconciledOptimisticNodeIds( + snapshot, + state.requestsByConversationId[event.conversationId], + ); const requests = withoutReconciledRequests( state.requestsByConversationId[event.conversationId], - snapshot, + reconciledSnapshot, ); const reducer = withUnreconciledOptimisticNodes( - replaceConversationSnapshot(current.reducer, snapshot), + replaceConversationSnapshot(current.reducer, reconciledSnapshot), requests, ); const entry: CodingConversationEntry = { diff --git a/tests/unit/coding-conversations-store.test.tsx b/tests/unit/coding-conversations-store.test.tsx index a593d64..091fc5b 100644 --- a/tests/unit/coding-conversations-store.test.tsx +++ b/tests/unit/coding-conversations-store.test.tsx @@ -337,6 +337,43 @@ describe('coding Conversation store', () => { expect(store.getState().requestsByConversationId['conversation-a']).toEqual({}); }); + it('preserves the optimistic UI id when a reconnect snapshot is already durable', async () => { + const store = createCodingConversationStore({ + getSnapshot: vi.fn(), + openEvents: vi.fn(), + submitPrompt: vi.fn(async (input) => acceptance( + input.conversationId, + input.clientRequestId, + )), + createId: ids(), + }); + store.getState().applySnapshotEvent(snapshotEvent(snapshot('conversation-a'))); + store.getState().setDraft('conversation-a', 'Already durable'); + await store.getState().submitPrompt({ conversationId: 'conversation-a', mode: 'prompt' }); + const durable = snapshot('conversation-a', 1, 1); + store.getState().applySnapshotEvent(snapshotEvent({ + ...durable, + nodes: [{ + kind: 'message', + id: 'durable-user-1', + sourceEntryId: 'entry-user-1', + clientRequestId: 'request-1', + role: 'user', + status: 'complete', + blocks: [{ kind: 'text', id: 'durable-text-1', text: 'Already durable', status: 'complete' }], + }], + })); + + expect(selectCodingConversationSnapshot('conversation-a')(store.getState())?.nodes[0]) + .toMatchObject({ + id: 'node-1', + sourceEntryId: 'entry-user-1', + clientRequestId: 'request-1', + status: 'complete', + }); + expect(store.getState().requestsByConversationId['conversation-a']).toEqual({}); + }); + it('restores draft attachments and marks the optimistic node on definite rejection', async () => { const submitPrompt = vi.fn(async () => { throw new AppError('RUNTIME', 'Model unavailable', undefined, {