Skip to content

Persist assistant image attachments in chat - #4859

Draft
AIdoesmyjob wants to merge 1 commit into
pingdotgg:mainfrom
AIdoesmyjob:codex/t3-assistant-image-attachments
Draft

Persist assistant image attachments in chat#4859
AIdoesmyjob wants to merge 1 commit into
pingdotgg:mainfrom
AIdoesmyjob:codex/t3-assistant-image-attachments

Conversation

@AIdoesmyjob

@AIdoesmyjob AIdoesmyjob commented Jul 29, 2026

Copy link
Copy Markdown

What

  • Persist completed provider image-view results as assistant-message image attachments.
  • Store image bytes in thread attachment storage and project them with assistant messages.
  • Render assistant image attachments in the timeline with expandable previews.

Why

Codex view_image results 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.
  • Live deployment acceptance on t3.skofo.ca: a view_image result rendered directly in the assistant message.
  • Deployed package: 0.0.31-skofo.20260729.ff321da.

Note

Persist assistant image attachments in chat and render them inline

  • When a provider emits an image_view item, ProviderRuntimeIngestion validates the file (type, size, safe extension), copies it into the server attachments directory, and appends a ChatAttachment to the assistant message via thread.message.assistant.delta.
  • decideOrchestrationCommand now forwards attachments from assistant delta commands through to the resulting thread.message-sent event payload.
  • Assistant messages in MessagesTimeline render attached images as zoomable thumbnails (or filename placeholders) above the message text, reusing a new shared MessageImageGrid component 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.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cad66b38-ca30-43e8-9579-2a73801c933e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jul 29, 2026
}
}

if (event.type === "item.completed" && event.payload.itemType === "image_view") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant