fix(reactions): idempotent optimistic update to prevent double-count #33

Merged

sameh-farouk merged 2 commits from fix/reaction-double-count-race into development

2026-04-27 18:07:06 +00:00

sameh-farouk commented

2026-04-27 17:02:48 +00:00

Member

Summary

Closes #31.

toggleReaction does optimistic UI after await rpc('message.toggle_react'). The server broadcasts the authoritative reactions list to all channel members (including sender, per the multi-tab WS contract). When the WS event lands faster than the RPC response (common under any non-trivial fanout latency), onMessageReacted replaces msg.reactions with the server-authoritative list — already containing the caller's row — and then the optimistic push adds a second copy. Badge renders 2 for one reaction.

Fix

Make the 'added' branch idempotent. Before pushing, check whether (emoji, user_id) is already present:

var already = msg.reactions.some(function (r) {
    return r.emoji === emoji && r.user_id === state.currentUser.id;
});
if (!already) {
    msg.reactions.push({ emoji: emoji, user_id: state.currentUser.id });
}

The 'removed' branch's .filter is naturally idempotent (filtering an already-filtered array is a no-op), so no change there.

Why optimistic update is kept

Removing the optimistic update entirely would also fix the bug, but reintroduces a latency-sensitive feedback gap (badge would lag visibly behind the click under packet loss / slow fanout). Idempotent push is the cheap fix that preserves both perceived latency and correctness.

Test plan

React on own message: count is 1 (was 2).
React on someone else's message: count is 1 (regression check — was already correct).
React, remove, re-react: counts toggle 1 → 0 → 1 cleanly.
React in thread panel: same as main feed (cross-array findMessageAnywhere lookup unchanged).
Multi-tab: open two viewer_test tabs; react in tab A; tab B sees the reaction (no double-count regression on receivers).
node --check on chat-app.js passes.

🤖 Generated with Claude Code

## Summary Closes #31. `toggleReaction` does optimistic UI after `await rpc('message.toggle_react')`. The server broadcasts the authoritative reactions list to all channel members (including sender, per the multi-tab WS contract). When the WS event lands faster than the RPC response (common under any non-trivial fanout latency), `onMessageReacted` replaces `msg.reactions` with the server-authoritative list — already containing the caller's row — and *then* the optimistic `push` adds a second copy. Badge renders `2` for one reaction. ## Fix Make the 'added' branch idempotent. Before pushing, check whether `(emoji, user_id)` is already present: ```js var already = msg.reactions.some(function (r) { return r.emoji === emoji && r.user_id === state.currentUser.id; }); if (!already) { msg.reactions.push({ emoji: emoji, user_id: state.currentUser.id }); } ``` The 'removed' branch's `.filter` is naturally idempotent (filtering an already-filtered array is a no-op), so no change there. ## Why optimistic update is kept Removing the optimistic update entirely would also fix the bug, but reintroduces a latency-sensitive feedback gap (badge would lag visibly behind the click under packet loss / slow fanout). Idempotent push is the cheap fix that preserves both perceived latency and correctness. ## Test plan - [x] React on own message: count is 1 (was 2). - [x] React on someone else's message: count is 1 (regression check — was already correct). - [x] React, remove, re-react: counts toggle 1 → 0 → 1 cleanly. - [x] React in thread panel: same as main feed (cross-array `findMessageAnywhere` lookup unchanged). - [x] Multi-tab: open two viewer_test tabs; react in tab A; tab B sees the reaction (no double-count regression on receivers). - [x] `node --check` on chat-app.js passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

sameh-farouk added 1 commit

2026-04-27 17:02:49 +00:00

fix(reactions): idempotent optimistic update to prevent double-count f8b1485592

`toggleReaction` does an optimistic local update after `await rpc(
'message.toggle_react')` returns, then `onMessageReacted` later
replaces `msg.reactions` wholesale when the WS broadcast lands. With
`Audience::ChannelMembers` including the sender (multi-tab parity),
the WS event can arrive before the RPC `await` resolves — interleaving:

  1. RPC fires; server persists + broadcasts to channel members
     (including sender) → onMessageReacted sets msg.reactions to the
     authoritative server list (already contains caller's row).
  2. RPC await resolves on the original click handler → optimistic
     `msg.reactions.push(...)` appends a second copy of the same
     (emoji, user_id) row.
  3. The badge counts entries → renders "2" for what is in fact one
     reaction.

Symptom from dogfooding: reacting to one's own message visibly counts
2 instead of 1.

Fix: make the 'added' branch idempotent. Before pushing, check whether
an entry with the same (emoji, user_id) is already present. The
'removed' branch's `.filter` is naturally idempotent (filtering an
already-filtered array is a no-op), so no change needed there.

Removing the optimistic update entirely would also fix the bug, but
it would re-introduce the latency-sensitive feedback gap that prompted
the optimistic path in the first place — under packet loss / slow
fanout the badge would lag visibly behind the click. Idempotent push
is the cheap fix that preserves both perceived latency and correctness.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>