feat(canvas/chat-server): canvas consumes /chat-history + server-side row-aware reverse (RFC #2945 PR-C-2) #4
Ghost
hongming
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "feat/rfc-2945-pr-c-2-canvas-chat-history"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes the SSOT story shipped in RFC #2945 PR-C/D: canvas now consumes the typed
/chat-historyendpoint instead of/activity?type=a2a_receive, and the server emits messages in display-ready chronological order so the client doesn't reorder.Canvas (consumer migration)
loadMessagesFromDBswaps from/activityto/chat-history.type=a2a_receive,source=canvasquery params (server applies the filter centrally).INTERNAL_SELF_MESSAGE_PREFIXESconst +isInternalSelfMessagehelper. Server-sideIsInternalSelfMessageapplies the same predicate before emitting rows.historyHydration.ts. The TS parser file stays in tree becausemessage-parser.tsis still load-bearing for live A2A WebSocket messages.Server (row-aware wire-order fix)
The pre-PR-C-2 client did
[...activities].reverse()over rows, then flattened each row into[user, agent]. After PR-C/D, the server returns a flatChatMessageslice inORDER BY created_at DESCorder with[user, agent]within each row — naive flat-reverse on the client would flip each pair (agent before user at same timestamp).Fix: server now owns wire order via
reverseRowChunks:Timestampmessages into row chunks (1-2 messages per row).[user, agent]order is preserved.Single-message rows (agent reply not yet recorded, attachments-only) reverse correctly too.
Tests
Server: 3 new unit tests on
reverseRowChunks(paired across rows, single-message rows, empty input) + 1 sqlmock integration test onList()that drives the full SQL → reverse → wire path. Mutation-tested: removedmessages = reverseRowChunks(messages)fromList(), confirmed the integration test fired red with all 4 misordered indices flagged. Restored, all 25 messagestore + 9 handler tests green.Canvas: 8
lazyHistorypagination tests refactored to mock/chat-history(not/activity) and assert against the new wire shape ({messages, reached_end}). All 1389/1389 vitest tests green;tsc --noEmitclean.Three weakest spots (hostile-reviewer self-pass)
reverseRowChunksgroups byTimestampstring equality. Two rows with the same microsecond-resolutioncreated_atwould be one chunk. Mitigated by SQL granularity + microsecond-collision rarity; within-chunk order falls back to SQL return order, which is acceptable since both render at the same displayed timestamp anyway.historyHydration.tsis now dead code. Deletion parked as a follow-up after a one-week observation window confirms no live-message consumer reaches it.loadMessagesFromDBreturnsresp.messages ?? []. Server today coerces nil to[]defensively; the??is belt-and-suspenders for any future impl that omits it.Security review
messagestore. No new exposure./chat-history→wsAuth→messagestore.No security-relevant changes beyond what
/chat-historyalready exposed via PR-C. Considered, not skipped.Versioning / backwards compat
/activityunchanged./chat-historyshape unchanged ({messages, reached_end}); only the wire order within a page changed (newest-first → oldest-first). Canvas is the only consumer in tree.Phasing
historyHydration.tsafter observation window🤖 Generated with Claude Code
Closes the SSOT story shipped in PR-C/D: canvas now consumes the typed /chat-history endpoint instead of /activity?type=a2a_receive, and the server emits messages in display-ready chronological order so the client doesn't have to re-order them. ## Canvas (consumer migration) - loadMessagesFromDB swaps from /activity to /chat-history. - Drops type=a2a_receive + source=canvas params (server applies the filter centrally now). - Drops [...activities].reverse() — wire is already display-ready. - Drops the local INTERNAL_SELF_MESSAGE_PREFIXES constant + isInternalSelfMessage helper. Server-side IsInternalSelfMessage applies the same predicate before emitting rows. - Drops the activityRowToMessages + ActivityRowForHydration imports from historyHydration.ts. The TS parser stays in tree because message-parser.ts is still load-bearing for live A2A WebSocket messages (ChatTab.tsx:805, AgentCommsPanel.tsx, canvas-events.ts). ## Server (row-aware wire-order fix) The pre-PR-C-2 client did `[...activities].reverse()` over ROWS, then flattened each row into [user, agent] messages. The reversal was ROW-aware. After PR-C/D, the server returned a flat ChatMessage slice in `ORDER BY created_at DESC` order, with [user, agent] within each row. A naive client-side flat reverse would FLIP each pair (agent before user at same timestamp). Two ways to fix it: A) Server emits oldest-first within page; canvas does NOT reverse. B) Canvas does row-aware reversal (group by timestamp, reverse). Option A is cleaner — server owns the wire-order responsibility, every client trusts `for m of messages` to render chronologically. Server adds reverseRowChunks() that: 1. Groups consecutive same-Timestamp messages into row chunks (1-2 messages per row). 2. Reverses the chunk order (newest-row-first → oldest-row-first). 3. Flattens. Within-chunk [user, agent] order is preserved. Single-message rows (agent reply not yet recorded, attachments-only user upload) collapse to 1-element chunks and reverse correctly too. ## Tests Server: 3 new unit tests on reverseRowChunks (paired across rows, single-message rows, empty input) + 1 sqlmock integration test on List() that drives the full SQL → reverse → wire path. Mutation-tested: removed `messages = reverseRowChunks(messages)` from List(), confirmed the integration test fires red with all 4 misordered indices flagged. Restored, all 25 messagestore tests + 9 chat-history handler tests green. Canvas: 8 lazyHistory pagination tests refactored to mock /chat-history (not /activity) and assert against the new wire shape ({messages, reached_end} not raw activity rows). All 1389/1389 vitest tests green; tsc --noEmit clean. ## Three weakest spots (hostile-reviewer self-pass) 1. reverseRowChunks groups by Timestamp string equality. If two distinct rows had the SAME timestamp (legitimately possible at sub- millisecond granularity), the algorithm would treat them as one chunk and not reverse them relative to each other. Mitigated: activity_logs.created_at uses microsecond resolution; concurrent inserts at exact-same microsecond are vanishingly rare. If a collision happens, the within-chunk order is whatever the SQL returned — both rows render at the same timestamp, no user-visible misordering. 2. The pre-existing TS parser files (historyHydration.ts + message-parser.ts) stay in tree. historyHydration.ts is now dead code (no consumers post-migration); deletion is parked as a follow- up after a one-week observation window confirms no live-message consumer reaches it. 3. canvas's loadMessagesFromDB returns `resp.messages ?? []`. If the server were ever to return `null` instead of `[]` (it currently doesn't — handler defensively coerces nil to []), the nullish coalesce keeps the canvas from crashing. A stricter wire schema would assert the never-null invariant; for today's pragmatic safety, the ?? is enough. ## Security review - Untrusted input? Same as PR-C — agent JSON parsed defensively in the messagestore parser. No new exposure. - Trust boundary? Same. Canvas → /chat-history → wsAuth → messagestore. - Output sanitization? Plain text + opaque attachment URIs as before. No security-relevant changes beyond what /chat-history already exposes via PR-C. Considered, not skipped. ## Versioning / backwards compat - /activity endpoint unchanged. - /chat-history endpoint shape unchanged (still {messages, reached_end}); only the wire ORDER within a page changed (newest-first row → oldest- first row). Canvas is the only consumer in tree; no API consumers depend on the previous order. - canvas's loadMessagesFromDB call signature unchanged — internal refactor. 🤖 Generated with [Claude Code](https://claude.com/claude-code)Hongming-approved (chat 2026-05-07): RFC #2945 PR-C-2 canvas/chat-server canvas consumes /chat-history.