Skip to content

Messaging live path: 5 round-trips per event, lost-update race, content-based dedupe #91

Description

@TortoiseWolfe

src/app/messages/page.tsx has exactly one refresh primitive: loadMessages() (:340-397), a full refetch-and-re-decrypt of the newest 50 rows. It runs on every realtime INSERT (:203), every UPDATE (:253), every reconnect (:207), and every 10s poll tick (:223-234).

Each run costs five sequential network round-trips, one crypto.subtle.importKey, one ECDH P-256 derivation, and up to 50 AES-GCM decryptions — producing byte-identical output to the previous run. Meanwhile the realtime payload that triggered it already contains the ciphertext: realtime.ts:68 passes payload.new as Message, and the handler at page.tsx:203 is declared () => {…}, discarding it.

This issue collects the resulting defects, cheapest fix first. None of it requires reviving the dead hook (see the deletion issue) — the fixes belong in page.tsx and message-service.ts.

Correctness

1. Lost-update race destroys on-screen messages. loadMessages guards only !conversationId (:341); loading is set at :344 and never read at entry, and there is no request sequencing. Full replace + concurrent invocation = last-resolver-wins. Against a backend whose read-replica lag this same file documents (:434-436, :451-456), a stale response can overwrite a fresher one and visibly remove a message that is already rendered.
→ Generation counter around loadMessages, ~5 lines. Cheapest meaningful fix in the file.

2. Optimistic dedupe matches on content, not identity (:358-366). serverOwnContent is a Set of message strings; any optimistic row whose content appears in it is dropped. Send "ok" twice before the first confirms and the second echo disappears from the UI until a later poll heals it. For an offline-queued message the heal does not come until the queue flushes, because the row is not on the server yet.
→ Mint the UUID client-side at compose time and let the server echo it, making dedupe id-equality. The schema already permits it (messages.id UUID PRIMARY KEY DEFAULT gen_random_uuid(), migration:835); both insert paths simply decline to supply one (message-service.ts:331-340, message-adapter.ts:238-251). The offline queue already mints a UUID at message-service.ts:147 and then throws it away.

3. Your own sent message is labelled with the other person's name. page.tsx:417 sets senderName: participantName on the optimistic bubble while :416 sets isOwn: true.

4. loadMore prepends with no dedupe (:355) while cursor is clobbered by a concurrent refetch (:392) → duplicate React keys at MessageThread.tsx:274.

Cost and UX

5. The UPDATE handler refetches everything on a status flip (:251-257). A delivered_at or read_at change triggers the full 5-RTT re-decryption cycle. With markAsDelivered and markAsRead both firing per message per side, a single message can drive three full refetches.
→ Make it incremental using payload.new, already supplied at realtime.ts:132: if encrypted_content is unchanged, patch the timestamps and decrypt nothing. Biggest single win available here.

6. The poll truncates scrollback every 10 seconds. loadMessages(false) replaces state with the newest 50 and resets the cursor (:356-368, :389), and the poll calls it with no argument (:226). A user who paged back through 200 messages is silently collapsed to 50.

7. The poll paints a loading spinner every 10 seconds. It sets loading (:344), which drives "Loading older messages…" at MessageThread.tsx:339 and intermittently swallows the load-more gesture (:213).
→ Split the pagination flag from background refreshes.

8. Idle cost is ~30 REST calls/min/tab, forever. lastRealtimeEventRef initialises to 0 (:197), so the t=10s tick always fires and duplicates the mount load. The INSERT filter has no sender exclusion (realtime.ts:65), so the sender refetches on their own insert.
→ Convert the poll to a gap query (WHERE sequence_number > my_max) with exponential backoff, triggered on reconnect and visibilitychange rather than on a fixed interval. Bytes then scale with what was actually missed, usually zero.

9. getUser() on read paths is an unconditional network call (message-service.ts:564, :931, :987) — one of the five RTTs per refetch. The repo's own comment at realtime.ts:218-221 documents why getSession() is correct here.

10. A missing peer key adds up to 6s of latency per event (message-service.ts:733-744), never memoized.

Suggested order

15276/89. Items 1 and 3 are small enough to land together as a first PR.

Note on architecture

Full refetch is the right repair mechanism and the wrong primary update mechanism. Its structural properties are real and worth keeping: state is a pure function of one SELECT ordered by a trigger-assigned UNIQUE column, so a dropped, duplicated, or reordered realtime event cannot corrupt it; key rotation and revocation self-heal within one cycle with no invalidation machinery. The fix is not to replace it wholesale but to stop using it as the response to every event.

Related: #69 (item 2 is a live candidate mechanism for its "silent message loss" symptom), #90, and the dead-island deletion issue.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions