improvement(avatar): use selection-update as the source of truth for presence, ignore other socket ops#2866
Conversation
…presence, ignore other socket ops
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryChanged Key changes:
Rationale: Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Client Socket
participant Provider as SocketProvider
participant Server as Socket Server
participant Room as RoomManager
Note over Client,Room: User Joins Workflow
Client->>Server: join-workflow(workflowId)
Server->>Room: createUserPresence(userId, userName, avatarUrl)
Room->>Room: room.users.set(socketId, userPresence)
Server->>Room: broadcastPresenceUpdate(workflowId)
Room->>Client: presence-update([...allUsers])
Client->>Provider: setPresenceUsers(users)
Note over Provider: User presence list initialized
Note over Client,Room: Cursor Movement (After PR)
Client->>Server: cursor-update({x, y})
Server->>Client: cursor-update(data)
Client->>Provider: Check if user exists
alt User exists in presenceUsers
Provider->>Provider: Update cursor for existing user
else User NOT in presenceUsers
Provider->>Provider: Return early, ignore update
end
Note over Client,Room: Selection Change (After PR)
Client->>Server: selection-update({type, id})
Server->>Client: selection-update(data)
Client->>Provider: Check if user exists
alt User exists in presenceUsers
Provider->>Provider: Update selection for existing user
else User NOT in presenceUsers
Provider->>Provider: Return early, ignore update
end
|
| setPresenceUsers((prev) => { | ||
| const existingIndex = prev.findIndex((user) => user.socketId === data.socketId) | ||
| if (existingIndex !== -1) { | ||
| return prev.map((user) => | ||
| user.socketId === data.socketId ? { ...user, cursor: data.cursor } : user | ||
| ) | ||
| if (existingIndex === -1) { | ||
| return prev | ||
| } |
There was a problem hiding this comment.
style: If a cursor-update arrives before presence-update (timing issue during connection), cursor position will be silently dropped. Consider logging when this occurs for debugging.
| setPresenceUsers((prev) => { | |
| const existingIndex = prev.findIndex((user) => user.socketId === data.socketId) | |
| if (existingIndex !== -1) { | |
| return prev.map((user) => | |
| user.socketId === data.socketId ? { ...user, cursor: data.cursor } : user | |
| ) | |
| if (existingIndex === -1) { | |
| return prev | |
| } | |
| setPresenceUsers((prev) => { | |
| const existingIndex = prev.findIndex((user) => user.socketId === data.socketId) | |
| if (existingIndex === -1) { | |
| logger.debug('Received cursor-update for unknown user', { socketId: data.socketId }) | |
| return prev | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/providers/socket-provider.tsx
Line: 407:411
Comment:
**style:** If a `cursor-update` arrives before `presence-update` (timing issue during connection), cursor position will be silently dropped. Consider logging when this occurs for debugging.
```suggestion
setPresenceUsers((prev) => {
const existingIndex = prev.findIndex((user) => user.socketId === data.socketId)
if (existingIndex === -1) {
logger.debug('Received cursor-update for unknown user', { socketId: data.socketId })
return prev
}
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| setPresenceUsers((prev) => { | ||
| const existingIndex = prev.findIndex((user) => user.socketId === data.socketId) | ||
| if (existingIndex !== -1) { | ||
| return prev.map((user) => | ||
| user.socketId === data.socketId ? { ...user, selection: data.selection } : user | ||
| ) | ||
| if (existingIndex === -1) { | ||
| return prev | ||
| } |
There was a problem hiding this comment.
style: Same as the cursor-update handler - consider adding debug logging for dropped selection updates.
| setPresenceUsers((prev) => { | |
| const existingIndex = prev.findIndex((user) => user.socketId === data.socketId) | |
| if (existingIndex !== -1) { | |
| return prev.map((user) => | |
| user.socketId === data.socketId ? { ...user, selection: data.selection } : user | |
| ) | |
| if (existingIndex === -1) { | |
| return prev | |
| } | |
| setPresenceUsers((prev) => { | |
| const existingIndex = prev.findIndex((user) => user.socketId === data.socketId) | |
| if (existingIndex === -1) { | |
| logger.debug('Received selection-update for unknown user', { socketId: data.socketId }) | |
| return prev | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/providers/socket-provider.tsx
Line: 420:424
Comment:
**style:** Same as the cursor-update handler - consider adding debug logging for dropped selection updates.
```suggestion
setPresenceUsers((prev) => {
const existingIndex = prev.findIndex((user) => user.socketId === data.socketId)
if (existingIndex === -1) {
logger.debug('Received selection-update for unknown user', { socketId: data.socketId })
return prev
}
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Summary
Type of Change
Testing
Tested manually
Checklist