Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThis PR implements server-authoritative chat persistence with SSE-driven canonical message ID handshakes. It replaces frontend-side row persistence with idempotent database upserts keyed on partial unique indexes, introduces server-side content building to keep streamed state coherent with persistence, and adds SSE events to communicate canonical message IDs back to the frontend for optimistic-to-real ID renaming. ChangesServer-Authoritative Persistence & SSE Message ID Handshake
Debug Warnings Configuration
Sequence DiagramsequenceDiagram
participant FE as Frontend
participant Route as Backend Route
participant Persist as Persistence Layer
participant DB as Database
participant Stream as Stream Orchestrator
participant SSE as SSE Service
FE->>Route: POST /new_chat (optimistic userMsgId, assistantMsgId)
Route->>Persist: persist_user_turn()
Persist->>DB: INSERT user row (ON CONFLICT DO NOTHING)
DB-->>Persist: user message_id
Route->>Persist: persist_assistant_shell()
Persist->>DB: INSERT assistant shell (ON CONFLICT DO NOTHING)
DB-->>Persist: assistant message_id
Route->>Stream: stream_new_chat(...)
Stream->>SSE: format_data("data-user-message-id", id)
SSE-->>FE: SSE event
FE->>FE: Rename optimistic userMsgId to canonical ID
Stream->>SSE: format_data("data-assistant-message-id", id)
SSE-->>FE: SSE event
FE->>FE: Rename optimistic assistantMsgId to canonical ID
par Streaming Content
Stream->>Stream: on_text_start/delta/end
Stream->>SSE: format_data("text-delta", ...)
SSE-->>FE: Stream text content
and Content Building
Stream->>Stream: content_builder.on_text_start/delta/end
Stream->>Stream: Accumulate ContentPart[]
end
Note over Stream,DB: On disconnect or completion
Stream->>Stream: mark_interrupted() if needed
Stream->>Persist: finalize_assistant_turn(snapshot)
Persist->>DB: UPDATE assistant row + INSERT token_usage
DB-->>Persist: Done
Stream-->>FE: SSE close
FE->>FE: Render canonical persisted content
Estimated code review effort🎯 5 (Critical) | ⏱️ ~105 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
FIX #
Screenshots
API Changes
Change Type
Testing Performed
Checklist
High-level PR Summary
This PR moves chat message persistence from client-side round-trips to server-side writes within the streaming chat flow. The backend now persists both user and assistant messages immediately during the stream via new
persist_user_turn,persist_assistant_shell, andfinalize_assistant_turnhelpers, emitting SSE events (data-user-message-idanddata-assistant-message-id) so the frontend can rename optimistic placeholder IDs to canonical database IDs in real-time. Two database migrations add partial unique indexes on(thread_id, turn_id, role)and(message_id)to make concurrent writes idempotent and prevent duplicate rows when legacy frontend code races the new server-side persistence. A newAssistantContentBuildermirrors the frontend's rich content projection server-side so the persisted JSONB matches the live-rendered UI. The legacyappendMessageroute is converted to a recovery-only no-op that returns existing rows when the unique index fires, and comprehensive integration tests verify the cross-writer contract holds under race conditions.⏱️ Estimated Review Time: 3+ hours
💡 Review Order Suggestion
surfsense_backend/alembic/versions/141_unique_chat_message_turn_role.pysurfsense_backend/alembic/versions/142_token_usage_message_id_unique.pysurfsense_backend/app/db.pysurfsense_backend/app/tasks/chat/persistence.pysurfsense_backend/app/tasks/chat/content_builder.pysurfsense_backend/app/tasks/chat/stream_new_chat.pysurfsense_backend/app/routes/new_chat_routes.pysurfsense_backend/app/schemas/new_chat.pysurfsense_web/lib/chat/stream-side-effects.tssurfsense_web/lib/chat/streaming-state.tssurfsense_web/app/dashboard/[search_space_id]/new-chat/[[...chat_id]]/page.tsxsurfsense_web/lib/chat/thread-persistence.tssurfsense_backend/tests/integration/chat/test_persistence.pysurfsense_backend/tests/integration/chat/test_message_id_sse.pysurfsense_backend/tests/integration/chat/test_append_message_recovery.pysurfsense_backend/tests/unit/tasks/chat/test_content_builder.pysurfsense_backend/tests/unit/test_stream_new_chat_contract.pysurfsense_backend/tests/integration/chat/__init__.py.vscode/launch.jsonSummary by CodeRabbit
New Features
Deprecations
Tests