Persist assistant image attachments in chat - #4859
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
| } | ||
| } | ||
|
|
||
| if (event.type === "item.completed" && event.payload.itemType === "image_view") { |
There was a problem hiding this comment.
🟡 Medium Layers/ProviderRuntimeIngestion.ts:1591
When an image_view event has no turnId, getOrCreateAssistantMessageId derives a message ID from the image's itemId, but the code only registers that ID in turn state when turnId is truthy. The later item.completed event for the assistant message uses a different itemId, and turn.completed can't finalize the image's message without a turn ID. The image's message is never sent an assistant-complete command, leaving it permanently marked streaming: true.\n\nThe image-attachment dispatch at line 1614 sends a thread.message.assistant.delta with delta: \"\", but registration of assistantMessageId (via rememberAssistantMessageId) is skipped whenever turnId is absent, so no later finalization path can find it. If image events can legitimately arrive without a turn ID, consider dispatching an assistant-complete inline so the message doesn't stay in the streaming state, or document why these messages are expected to be finalized through another path.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts around line 1591:
When an `image_view` event has no `turnId`, `getOrCreateAssistantMessageId` derives a message ID from the image's `itemId`, but the code only registers that ID in turn state when `turnId` is truthy. The later `item.completed` event for the assistant message uses a different `itemId`, and `turn.completed` can't finalize the image's message without a turn ID. The image's message is never sent an `assistant-complete` command, leaving it permanently marked `streaming: true`.\n\nThe image-attachment dispatch at line 1614 sends a `thread.message.assistant.delta` with `delta: \"\"`, but registration of `assistantMessageId` (via `rememberAssistantMessageId`) is skipped whenever `turnId` is absent, so no later finalization path can find it. If image events can legitimately arrive without a turn ID, consider dispatching an `assistant-complete` inline so the message doesn't stay in the streaming state, or document why these messages are expected to be finalized through another path.
| return undefined; | ||
| } | ||
|
|
||
| yield* fileSystem.copyFile(sourcePath, destinationPath); |
There was a problem hiding this comment.
🟡 Medium Layers/ProviderRuntimeIngestion.ts:1024
persistAssistantImageAttachment copies the file into permanent attachment storage before the thread.message.assistant.delta dispatch that records the attachment. When that dispatch fails, processInputSafely logs a warning and swallows the error, but the copied file is never deleted. Each subsequent retry generates a new UUID and copies the file again, so repeated transient failures accumulate unreachable attachment files indefinitely. Consider deferring the copyFile until after the dispatch succeeds, or cleaning up the copied file when the dispatch fails.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts around line 1024:
`persistAssistantImageAttachment` copies the file into permanent attachment storage *before* the `thread.message.assistant.delta` dispatch that records the attachment. When that dispatch fails, `processInputSafely` logs a warning and swallows the error, but the copied file is never deleted. Each subsequent retry generates a new UUID and copies the file again, so repeated transient failures accumulate unreachable attachment files indefinitely. Consider deferring the `copyFile` until after the dispatch succeeds, or cleaning up the copied file when the dispatch fails.
What
Why
Codex
view_imageresults were represented only as collapsed work entries, so images did not appear in the conversation.Verification
corepack pnpm exec vitest run apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts apps/web/src/components/chat/MessagesTimeline.test.tsx --reporter=verbose— 61 tests passed.view_imageresult rendered directly in the assistant message.0.0.31-skofo.20260729.ff321da.Note
Persist assistant image attachments in chat and render them inline
image_viewitem,ProviderRuntimeIngestionvalidates the file (type, size, safe extension), copies it into the server attachments directory, and appends aChatAttachmentto the assistant message viathread.message.assistant.delta.decideOrchestrationCommandnow forwardsattachmentsfrom assistant delta commands through to the resultingthread.message-sentevent payload.MessagesTimelinerender attached images as zoomable thumbnails (or filename placeholders) above the message text, reusing a new sharedMessageImageGridcomponent also used for user messages.📊 Macroscope summarized ff321da. 4 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.