fix(platform): chat file attachment flicker on stream completion#1602
Merged
Conversation
A generated file (e.g. PDF from pdf_tool) is saved as a separate file-only assistant message that lands in the query before the post-tool text message (M2) does. With the previous forward-merge, the file rendered in a standalone bubble until M2 arrived, then jumped into M2 — visible as the PDF card disappearing and M2's TypewriterText animating from scratch. Hide file-only messages whose order matches an active streaming/pending assistant message; the existing forward merge attaches them as soon as M2 gains content. activeTurnOrder is read from the same uiMessages snapshot the merge iterates to keep the pass consistent. Image-gen and failed turns are unaffected — neither has a matching active streaming message.
📝 WalkthroughWalkthroughThis PR introduces logic to improve handling of file-only assistant messages in the chat message processing pipeline. The changes add detection of an active turn with streaming or pending assistant messages, then conditionally hide file-only assistant messages (messages with empty text but containing file parts) when they exist in the same turn as an active streaming assistant. The test suite comprehensively covers scenarios where file-only messages are merged into text-bearing messages, hidden when a streaming assistant is active, or rendered independently when no active streaming is present. Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 2
🤖 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/hooks/__tests__/use-message-processing.test.ts`:
- Around line 1056-1073: The test fixtures pdfPart, pdfPart2, and imagePart use
"as const" which violates the no-as rule; replace those assertions by removing
"as const" and instead apply a TypeScript "satisfies" check against the
appropriate part/type interface used in the codebase (e.g., MessagePart or
FilePart) so the objects remain fully typed at compile time without using "as";
update the object literals (pdfPart, pdfPart2, imagePart) to end with "satisfies
<CorrectType>" to validate their shapes while keeping runtime values unchanged.
In `@services/platform/app/features/chat/hooks/use-message-processing.ts`:
- Around line 198-202: The current code uses uiMessages.find(...) to derive a
single activeTurnOrder, which misses additional in-flight assistant turns;
change logic in use-message-processing (the activeTurnOrder computation) to
collect all assistant message orders with status 'streaming' or 'pending' (e.g.,
filter/map to a Set or array of orders) and then use that collection when
deciding to hide file-only messages so all in-flight turns are handled; apply
the same change to the duplicate logic around the block referenced at lines
~375-381 so both places use the multi-order set instead of a single find result.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5e0282b1-3313-43cd-ba58-9494fcc204d7
📒 Files selected for processing (2)
services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.tsservices/platform/app/features/chat/hooks/use-message-processing.ts
Comment on lines
+1056
to
+1073
| const pdfPart = { | ||
| type: 'file' as const, | ||
| mediaType: 'application/pdf', | ||
| filename: 'report.pdf', | ||
| url: 'https://example.com/report.pdf', | ||
| }; | ||
| const pdfPart2 = { | ||
| type: 'file' as const, | ||
| mediaType: 'application/pdf', | ||
| filename: 'appendix.pdf', | ||
| url: 'https://example.com/appendix.pdf', | ||
| }; | ||
| const imagePart = { | ||
| type: 'file' as const, | ||
| mediaType: 'image/png', | ||
| filename: 'generated.png', | ||
| url: 'https://example.com/generated.png', | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify remaining `as const` assertions in this test file.
rg -n --type=ts '\bas const\b' services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.tsRepository: tale-project/tale
Length of output: 162
🏁 Script executed:
#!/bin/bash
# Check how the fixture objects are used after definition
sed -n '1056,1080p' services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.tsRepository: tale-project/tale
Length of output: 804
🏁 Script executed:
#!/bin/bash
# Find all usages of pdfPart, pdfPart2, and imagePart in the test file
rg -n 'pdfPart|imagePart' services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.ts | head -20Repository: tale-project/tale
Length of output: 771
🏁 Script executed:
#!/bin/bash
# Check the broader context - what are these parts being assigned to or used with?
sed -n '1056,1150p' services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.tsRepository: tale-project/tale
Length of output: 2961
🏁 Script executed:
#!/bin/bash
# Find the createUIMessage function and its type definition
rg -A 10 'function createUIMessage|const createUIMessage' services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.ts | head -30Repository: tale-project/tale
Length of output: 324
🏁 Script executed:
#!/bin/bash
# Find the UIMessage type or parts type definition
rg -B 2 -A 5 'type.*UIMessage|interface.*UIMessage' services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.tsRepository: tale-project/tale
Length of output: 11707
🏁 Script executed:
#!/bin/bash
# Search for imports to understand where UIMessage comes from
head -50 services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.tsRepository: tale-project/tale
Length of output: 1194
🏁 Script executed:
#!/bin/bash
# Verify the structure of the object and whether `satisfies` would work
# Check if there's a specific part type we can use with `satisfies`
rg -B 5 -A 15 'parts\s*[:=]' services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.ts | head -40Repository: tale-project/tale
Length of output: 1033
🏁 Script executed:
#!/bin/bash
# Check if there's a specific Part type exported or used
rg 'type.*Part|interface.*Part' services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.tsRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Look at what `@convex-dev/agent/react` might provide for parts
rg -l '@convex-dev/agent/react' services/platform/app/features/chat/hooks/ | head -5Repository: tale-project/tale
Length of output: 264
🏁 Script executed:
#!/bin/bash
# Check the main file for part type definitions or imports
head -100 services/platform/app/features/chat/hooks/use-message-processing.tsRepository: tale-project/tale
Length of output: 3504
🏁 Script executed:
#!/bin/bash
# Look for 'part' or 'Part' in the main implementation file
rg -i 'type.*part|interface.*part|parts:' services/platform/app/features/chat/hooks/use-message-processing.ts | head -20Repository: tale-project/tale
Length of output: 298
🏁 Script executed:
#!/bin/bash
# Check if FilePart is exported from use-message-processing.ts
rg 'export.*FilePart|export type FilePart' services/platform/app/features/chat/hooks/use-message-processing.tsRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check how FilePart is actually used in the test - search for the parts type in UIMessage
rg -B 5 -A 5 'parts.*\[' services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.ts | grep -A 5 -B 5 'pdfPart\|imagePart' | head -30Repository: tale-project/tale
Length of output: 820
🏁 Script executed:
#!/bin/bash
# See if FilePart is defined elsewhere or imported in the test file
rg 'FilePart|import.*from.*use-message-processing' services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.tsRepository: tale-project/tale
Length of output: 43
Use satisfies instead of as const assertions in test fixtures.
Lines 1057, 1063, and 1069 use as const, which violates the no-as rule. Test fixtures should use satisfies for compile-time validation instead.
💡 Suggested refactor
- const pdfPart = {
- type: 'file' as const,
+ const pdfPart = {
+ type: 'file',
mediaType: 'application/pdf',
filename: 'report.pdf',
url: 'https://example.com/report.pdf',
- };
+ } satisfies { type: 'file'; mediaType: string; filename: string; url: string };
- const pdfPart2 = {
- type: 'file' as const,
+ const pdfPart2 = {
+ type: 'file',
mediaType: 'application/pdf',
filename: 'appendix.pdf',
url: 'https://example.com/appendix.pdf',
- };
+ } satisfies { type: 'file'; mediaType: string; filename: string; url: string };
- const imagePart = {
- type: 'file' as const,
+ const imagePart = {
+ type: 'file',
mediaType: 'image/png',
filename: 'generated.png',
url: 'https://example.com/generated.png',
- };
+ } satisfies { type: 'file'; mediaType: string; filename: string; url: string };Per coding guidelines: Never as, never any, never unknown in TypeScript. For test data, use satisfies for compile-time validation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const pdfPart = { | |
| type: 'file' as const, | |
| mediaType: 'application/pdf', | |
| filename: 'report.pdf', | |
| url: 'https://example.com/report.pdf', | |
| }; | |
| const pdfPart2 = { | |
| type: 'file' as const, | |
| mediaType: 'application/pdf', | |
| filename: 'appendix.pdf', | |
| url: 'https://example.com/appendix.pdf', | |
| }; | |
| const imagePart = { | |
| type: 'file' as const, | |
| mediaType: 'image/png', | |
| filename: 'generated.png', | |
| url: 'https://example.com/generated.png', | |
| }; | |
| const pdfPart = { | |
| type: 'file', | |
| mediaType: 'application/pdf', | |
| filename: 'report.pdf', | |
| url: 'https://example.com/report.pdf', | |
| } satisfies { type: 'file'; mediaType: string; filename: string; url: string }; | |
| const pdfPart2 = { | |
| type: 'file', | |
| mediaType: 'application/pdf', | |
| filename: 'appendix.pdf', | |
| url: 'https://example.com/appendix.pdf', | |
| } satisfies { type: 'file'; mediaType: string; filename: string; url: string }; | |
| const imagePart = { | |
| type: 'file', | |
| mediaType: 'image/png', | |
| filename: 'generated.png', | |
| url: 'https://example.com/generated.png', | |
| } satisfies { type: 'file'; mediaType: string; filename: string; url: string }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/chat/hooks/__tests__/use-message-processing.test.ts`
around lines 1056 - 1073, The test fixtures pdfPart, pdfPart2, and imagePart use
"as const" which violates the no-as rule; replace those assertions by removing
"as const" and instead apply a TypeScript "satisfies" check against the
appropriate part/type interface used in the codebase (e.g., MessagePart or
FilePart) so the objects remain fully typed at compile time without using "as";
update the object literals (pdfPart, pdfPart2, imagePart) to end with "satisfies
<CorrectType>" to validate their shapes while keeping runtime values unchanged.
Comment on lines
+198
to
+202
| const activeTurnOrder = uiMessages.find( | ||
| (m) => | ||
| m.role === 'assistant' && | ||
| (m.status === 'streaming' || m.status === 'pending'), | ||
| )?.order; |
There was a problem hiding this comment.
Handle multiple in-flight assistant turns, not only the first one.
At Line 198, find(...) captures a single activeTurnOrder. If the snapshot contains both pending and streaming assistants in different orders, only one order is hidden in Pass 2, so file-only messages in the other active turn can still render transiently.
💡 Suggested fix
- const activeTurnOrder = uiMessages.find(
- (m) =>
- m.role === 'assistant' &&
- (m.status === 'streaming' || m.status === 'pending'),
- )?.order;
+ const activeTurnOrders = new Set(
+ uiMessages
+ .filter(
+ (m) =>
+ m.role === 'assistant' &&
+ (m.status === 'streaming' || m.status === 'pending') &&
+ m.order != null,
+ )
+ .map((m) => m.order),
+ );
...
- if (
- activeTurnOrder != null &&
+ if (
+ msg.order != null &&
+ activeTurnOrders.has(msg.order) &&
msg.role === 'assistant' &&
!msg.content &&
!msg.isAborted &&
- msg.fileParts?.length &&
- msg.order === activeTurnOrder
+ msg.fileParts?.length
) {
return false;
}Also applies to: 375-381
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/hooks/use-message-processing.ts` around
lines 198 - 202, The current code uses uiMessages.find(...) to derive a single
activeTurnOrder, which misses additional in-flight assistant turns; change logic
in use-message-processing (the activeTurnOrder computation) to collect all
assistant message orders with status 'streaming' or 'pending' (e.g., filter/map
to a Set or array of orders) and then use that collection when deciding to hide
file-only messages so all in-flight turns are handled; apply the same change to
the duplicate logic around the block referenced at lines ~375-381 so both places
use the multi-order set instead of a single find result.
The prior guard scanned uiMessages for a streaming/pending assistant to detect an active turn, but both statuses are undefined in the gap between a pre-tool message marked `success` and the post-tool message being created — letting the orphan file-only row flash. Switch to threadMetadata.generationStatus (via threads.queries.isThreadGenerating), which stays true across the entire multi-step turn, and scope to the max assistant order in the current snapshot so earlier completed turns are unaffected.
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
panda-research-report.pdf), the PDF card briefly appeared in a standalone bubble, then disappeared as the post-tool text message mounted fresh and re-animated its content.appendGeneratedFilePartwrites a separate file-only assistant message (F) via its own mutation; F lands in the subscription result before the post-tool text message (M2) does. With the previous forward-only merge, F rendered standalone during that window, then was filtered and reattached to M2 once M2 gained content — a visible mount/unmount plus freshTypewriterTextanimation.activeTurnOrderfrom the sameuiMessagessnapshot the merge pass iterates, and hide file-only messages whose order matches it. The existing forward merge attaches F as soon as M2 gains content, so the PDF appears once, inside M2's bubble, without the intermediate bubble.saveMessagewithoutpromptMessageId, so their file-only message sits at a differentorderthan any active streaming message. Failed turns also fall back to standalone rendering (no active streaming → guard inert).Test plan
npx tsc --noEmitfromservices/platform/— clean.npm run lint --workspace=@tale/platform— clean.dji.tale.dev— PDF card should appear only once, inside the post-tool bubble, no standalone flicker.Summary by CodeRabbit
Tests
Improvements