fix: dedupe realtime messages before render#616
fix: dedupe realtime messages before render#616CoderLuii wants to merge 2 commits intositeboon:mainfrom
Conversation
📝 WalkthroughWalkthroughUpdates message deduplication in the session store from simple ID equality to a sophisticated equivalence comparison that considers multiple identifier fields (id, rowid, sequence, requestId, toolId) with special handling for user messages and session creation events. Changes
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stores/useSessionStore.ts (1)
342-346:⚠️ Potential issue | 🟡 MinorSkip server-equivalent messages before storing them.
These paths still retain messages that
computeMerged()will immediately hide because an equivalent server message already exists. Those hidden entries still count towardMAX_REALTIME_MESSAGES, so a burst of duplicates can evict genuinely pending realtime items from the buffer. Since this store already treatsserverMessagesas the source of truth, short-circuiting here would keep the capped realtime buffer reserved for not-yet-persisted events.Also applies to: 357-363
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/useSessionStore.ts` around lines 342 - 346, Before adding to slot.realtimeMessages, check whether the incoming msg is effectively already present in the authoritative server view and skip storing it—i.e., call the same equivalence check used by computeMerged() or compare against slot.serverMessages and, if an equivalent server message exists, do not call upsertRealtimeMessage. Update the two places that currently call upsertRealtimeMessage (the block using upsertRealtimeMessage(slot.realtimeMessages, msg) and the similar block later at 357-363) to short-circuit when a server-equivalent message is found so the realtime buffer only counts truly pending, not-already-persisted events; keep MAX_REALTIME_MESSAGES trimming logic unchanged for actual inserts. Ensure you reference computeMerged, serverMessages, realtimeMessages, upsertRealtimeMessage and MAX_REALTIME_MESSAGES when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/stores/useSessionStore.ts`:
- Around line 139-145: The current equality logic in useSessionStore (the
comparisons using a.requestId, b.requestId, a.toolId, b.toolId and a.kind)
considers requestId before toolId which collapses multiple same-kind messages
from the same request; change the comparator to disambiguate by tool identity
first (check toolId equality with a.kind) or require both toolId and requestId
to match before returning true, and if applicable include the same identity
fields used in src/components/chat/utils/messageKeys.ts (toolId, rowid,
sequence) when determining message identity so that repeated
tool_use/tool_result events are not merged incorrectly.
---
Outside diff comments:
In `@src/stores/useSessionStore.ts`:
- Around line 342-346: Before adding to slot.realtimeMessages, check whether the
incoming msg is effectively already present in the authoritative server view and
skip storing it—i.e., call the same equivalence check used by computeMerged() or
compare against slot.serverMessages and, if an equivalent server message exists,
do not call upsertRealtimeMessage. Update the two places that currently call
upsertRealtimeMessage (the block using
upsertRealtimeMessage(slot.realtimeMessages, msg) and the similar block later at
357-363) to short-circuit when a server-equivalent message is found so the
realtime buffer only counts truly pending, not-already-persisted events; keep
MAX_REALTIME_MESSAGES trimming logic unchanged for actual inserts. Ensure you
reference computeMerged, serverMessages, realtimeMessages, upsertRealtimeMessage
and MAX_REALTIME_MESSAGES when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 31ebe943-a048-40c7-b32f-98bb83831048
📒 Files selected for processing (1)
src/stores/useSessionStore.ts
| if (a.requestId && b.requestId && a.requestId === b.requestId && a.kind === b.kind) { | ||
| return true; | ||
| } | ||
|
|
||
| if (a.toolId && b.toolId && a.toolId === b.toolId && a.kind === b.kind) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
requestId is too broad to outrank toolId here.
Line 139 matches on requestId before Line 143 gets a chance to disambiguate by toolId. That can collapse multiple same-kind messages from one request—especially repeated tool_use / tool_result events—into a single entry. src/components/chat/utils/messageKeys.ts:10-24 points the same way: it treats toolId, rowid, and sequence as intrinsic identity, but never uses requestId.
🔧 Narrow fix
- if (a.requestId && b.requestId && a.requestId === b.requestId && a.kind === b.kind) {
- return true;
- }
-
if (a.toolId && b.toolId && a.toolId === b.toolId && a.kind === b.kind) {
return true;
}
+
+ if (
+ a.requestId &&
+ b.requestId &&
+ a.requestId === b.requestId &&
+ a.kind === b.kind &&
+ (a.toolId === undefined || b.toolId === undefined || a.toolId === b.toolId)
+ ) {
+ return true;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/useSessionStore.ts` around lines 139 - 145, The current equality
logic in useSessionStore (the comparisons using a.requestId, b.requestId,
a.toolId, b.toolId and a.kind) considers requestId before toolId which collapses
multiple same-kind messages from the same request; change the comparator to
disambiguate by tool identity first (check toolId equality with a.kind) or
require both toolId and requestId to match before returning true, and if
applicable include the same identity fields used in
src/components/chat/utils/messageKeys.ts (toolId, rowid, sequence) when
determining message identity so that repeated tool_use/tool_result events are
not merged incorrectly.
1 participant
Summary
Testing
Summary by CodeRabbit