feat(platform): improve chat UX with persisted drafts and error handling#502
Merged
Conversation
…nhancements, and error handling
Greptile SummaryThis PR significantly improves chat UX by implementing persistent drafts and attachments, fixing the stuck loading state issue, and enhancing file display capabilities. Key improvements:
Implementation quality:
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| services/platform/app/features/chat/hooks/use-persisted-attachments.ts | New hook to persist chat attachments across thread switches using localStorage with URL fetching logic |
| services/platform/app/features/chat/components/chat-interface.tsx | Integrates persisted draft and attachment hooks, removes key prop that was preventing draft persistence |
| services/platform/convex/lib/agent_response/generate_response.ts | Adds failed message saving in error handler to break UI loading state |
| services/platform/convex/lib/agent_chat/internal_actions.ts | Adds failed message saving for empty responses to prevent stuck loading state |
| services/platform/app/features/chat/hooks/use-message-processing.ts | Strips internal file reference markers from displayed messages using regex patterns |
| services/platform/app/features/chat/components/message-bubble/image-preview-dialog.tsx | Adds pan/drag support for zoomed images with pointer events and dark mode fixes |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User enters chat message] --> B{Has attachments?}
B -->|Yes| C[Save to localStorage]
B -->|No| D[Save draft to localStorage]
C --> D
D --> E[User sends message]
E --> F[Clear draft & attachments]
F --> G[Create optimistic pending message]
G --> H[Call generateAgentResponse]
H --> I{Agent execution}
I -->|Success| J[Save assistant message with content]
I -->|Error thrown| K[Save failed message with status: 'failed']
I -->|Empty response| L[Save failed message for empty response]
K --> M[UI exits loading state]
L --> M
J --> M
N[User switches threads] --> O[Save current attachments to localStorage]
O --> P[Load new thread's attachments from localStorage]
P --> Q{Attachments have image preview URLs?}
Q -->|No| R[Fetch URLs via useFileUrls batch query]
Q -->|Yes| S[Render immediately]
R --> S
Last reviewed commit: 28565da
📝 WalkthroughWalkthroughThis PR implements file attachment persistence and lift attachment state management from the ChatInput component to its parent ChatInterface component. It introduces new hooks for persisting attachments to localStorage per chat thread and batch retrieving file URLs. The changes include enhanced error handling in the agent response generation to save failed messages, improved image preview functionality with pan/zoom capabilities, and refined markdown image rendering with better styling defaults. Message bubble components now handle missing file display URLs gracefully with placeholder rendering. Backend APIs are extended to support batch file URL retrieval. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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)
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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/chat/components/message-bubble/markdown-renderer.tsx`:
- Line 46: The CSS selector '[&_image-group]:mt-2' is targeting a non-existent
element named image-group; change it to target the class applied on the div by
replacing the selector with '[&_.image-group]:mt-2' so the rule applies to
descendants with className="image-group" in markdown-renderer.tsx (look for the
array entry string '[&_image-group]:mt-2' and update it to
'[&_.image-group]:mt-2').
In
`@services/platform/app/features/chat/hooks/__tests__/use-persisted-attachments.test.ts`:
- Around line 50-190: Tests currently use non-null assertions (stored!) when
reading localStorage and parsing JSON in use-persisted-attachments.test (e.g.,
the calls around JSON.parse(stored!)). Replace those assertions with explicit
guards: check that localStorage.getItem('chat-attachments-thread-1') returns a
non-null value (throw or fail the test with a clear message or use expect to
assert non-null) before calling JSON.parse, and similarly guard any other stored
variable usages (the three occurrences flagged) so no `!` is used; update the
assertions to operate on the parsed value only after the guard.
In `@services/platform/app/features/chat/hooks/use-message-processing.ts`:
- Around line 134-138: The code passes a possibly undefined m.text to
stripInternalFileReferences when building the message object; update the
construction in the use-message-processing hook so you guard m.text (e.g., use a
default empty string or conditional) before calling stripInternalFileReferences
for the message's content to avoid a runtime error—identify the object creation
where id/key/content are returned and change the content assignment involving
stripInternalFileReferences(m.text) to call stripInternalFileReferences(m.text
?? '') or only call it when m.text is defined.
In `@services/platform/app/features/chat/hooks/use-persisted-attachments.ts`:
- Around line 93-114: The restore/save logic is running during render and calls
setAttachments/savePersisted which causes React warnings; move both the initial
restore (the hasRestoredRef branch using loadPersisted, isRestoringRef,
setAttachments, toFileAttachment) and the thread-switch logic (checking
prevThreadIdRef, calling savePersisted with attachmentsRef, updating
prevThreadIdRef, then loading via loadPersisted and setAttachments) into
useEffect hooks that depend on threadId (and run once on mount for the initial
restore), so state updates happen after render; ensure the effects update
hasRestoredRef and prevThreadIdRef and preserve the existing conversions with
toFileAttachment and the isRestoringRef flag.
Fix [&_image-group] selector to [&_.image-group] to correctly target elements with className="image-group". Run oxfmt on all branch files.
Previous format pass used oxfmt@0.34.0 which produced different output than the project's pinned oxfmt@0.32.0 used in CI.
Avoid indented if-conditions in FileAttachmentDisplay. Extract MAX_BATCH_FILE_IDS to lib/shared/file-types so both backend and frontend enforce the same batch limit.
The displayUrl variable could be string | null | undefined since serverFileUrl returns string | null. Adding || undefined coerces null to undefined, satisfying the img src prop type.
This was referenced Mar 9, 2026
4 tasks
4 tasks
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
This was referenced Apr 9, 2026
8 tasks
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.
Summary
[ATTACHED FILES...],fileId: ...) is stripped from displayed messagesStatItemcomponentTest plan
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style