Render generated images inline in chat#3984
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| function generatedImageReference(value: string): string | null { | ||
| const reference = value.trim(); | ||
| const segments = reference.split("/"); |
There was a problem hiding this comment.
🟡 Medium components/ChatMarkdown.tsx:896
generatedImageReference splits the reference only on /, so a Windows-style path like images\1.jpg produces a single segment and is rejected even though the server accepts and normalizes that same artifact path. This causes inline generated-image rendering to fail for Windows-style references. Consider normalizing \ to / before splitting and validating, matching the server's behavior.
| function generatedImageReference(value: string): string | null { | |
| const reference = value.trim(); | |
| const segments = reference.split("/"); | |
| function generatedImageReference(value: string): string | null { | |
| const reference = value.trim().replace(/\\/g, "/"); | |
| const segments = reference.split("/"); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatMarkdown.tsx around lines 896-898:
`generatedImageReference` splits the reference only on `/`, so a Windows-style path like `images\1.jpg` produces a single segment and is rejected even though the server accepts and normalizes that same artifact path. This causes inline generated-image rendering to fail for Windows-style references. Consider normalizing `\` to `/` before splitting and validating, matching the server's behavior.
There was a problem hiding this comment.
Addressed in 391fa4da8: normalize generated-image path separators before validation.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces a new user-facing feature (inline image rendering) with new components and server-side logic. Additionally, there are multiple unresolved review comments identifying bugs including Windows path handling issues, broken fenced code block rendering, and incorrect artifact correlation within turns. You can customize Macroscope's approvability policy. Learn more. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a679e683b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const rawOutput = asUnknownRecord(data?.rawOutput); | ||
| const artifactPath = rawOutput?.path; |
There was a problem hiding this comment.
When the matching activity comes from Codex app-server image generation, CodexAdapter stores the raw item payload as payload.data (see apps/server/src/provider/Layers/CodexAdapter.ts:476-484), and the generated file is exposed as data.item.savedPath for imageGeneration (or data.item.path for imageView), not as data.rawOutput.path. With this lookup limited to rawOutput.path, a valid assistant code span such as images/1.jpg never matches Codex's generated-image activity, so assets.createUrl returns not found and the primary Codex generated images stay rendered as plain code.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 2c613968b: resolve Codex imageGeneration.savedPath and imageView.path values.
| code({ node, className: codeClassName, children, ...props }) { | ||
| const artifactReference = codeClassName | ||
| ? null | ||
| : generatedImageReference(plainHastText(node) ?? ""); |
There was a problem hiding this comment.
With this code component registered, fenced-code children passed to the pre renderer are React elements of this function rather than the literal "code" element that extractCodeBlock requires (onlyChild.type !== "code"). For any fenced code block, the pre renderer falls back to raw <pre> and skips the existing MarkdownCodeBlock/Shiki path, removing code-block chrome and highlighting; if a fence contains an image-looking path it can also render an artifact preview inside the block.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in bd0756a80: add regression coverage for enhanced fenced-code rendering.
| const reference = value.trim(); | ||
| const segments = reference.split("/"); |
There was a problem hiding this comment.
For Windows-style relative artifact references such as images\\1.jpg, this client matcher splits only on /, so it returns null before the server-side backslash normalization can run and the image remains plain code. The server accepts backslashes in normalizeThreadArtifactReference, so normalize them here too to keep generated images working when providers format relative paths with Windows separators.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 0dcfdaf6d: add focused Windows-path normalization coverage.
| for (const activity of [...thread.activities].toReversed()) { | ||
| if (activity.turnId !== turnId) continue; |
There was a problem hiding this comment.
When a single turn generates the same relative artifact name more than once (for example two image-generation calls that both report images/1.jpg from different session directories), this reverse scan always picks the last activity in that turn for every assistant message carrying that turnId. Earlier messages in the turn then resolve to a later artifact, so the UI can show the wrong generated image; the lookup needs a message/activity ordering discriminator rather than turnId alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in c6a0a2a70: correlate artifact lookup to the assistant message's activity window.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fdf4006. Configure here.
| for (const activity of thread.activities) { | ||
| if (activity.turnId !== turnId) continue; | ||
| const createdAt = Date.parse(activity.createdAt); | ||
| if (Number.isNaN(createdAt) || createdAt <= window.after || createdAt > window.before) continue; |
There was a problem hiding this comment.
Activity window excludes mid-stream tools
Medium Severity
The artifactMessageWindow function uses an assistant message's createdAt as the upper bound for matching activities. For streaming messages, this createdAt is from the first chunk. Activities, like image generation, that occur later in the same streaming message have a createdAt after this bound, causing findThreadArtifactPath to incorrectly exclude them and preventing inline previews from resolving.
Reviewed by Cursor Bugbot for commit fdf4006. Configure here.
1 participant
Summary
Why
Generated images currently appear as code-formatted paths such as
images/1.jpg, so users must leave the conversation to inspect them. These files live in provider session storage rather than the project workspace or managed attachment store.The new thread-artifact request carries the thread, turn, and relative image path needed to authorize the matching projected tool output. After that authorization, the server delegates signing and serving to the existing exact workspace-file asset implementation. Turn correlation prevents an older message from resolving a newer artifact with the same relative path.
Validation
pnpm exec vp checkpnpm exec vp run typecheckpnpm exec vp test apps/server/src/assets/AssetAccess.test.ts apps/web/src/components/chat/MessagesTimeline.test.tsx(19 tests passed)Note
Render generated images inline in chat as expandable thumbnails
thread-artifactvariant toAssetResourceand a newassetsCreateUrlRPC handler that resolves artifact paths from thread activity history within the assistant message time window.normalizeGeneratedImageReference(client) andnormalizeThreadArtifactReference(server) to validate and normalize inline code image references.ChatMarkdownto detect inline code containing valid image references and render them as lazy-loaded thumbnails via a newMarkdownThreadArtifactImagecomponent; clicking opens the expanded image view.AssistantTimelineRowpasses turn/message context toChatMarkdownto scope artifact resolution.<code>rendering.Macroscope summarized fdf4006.
Note
Medium Risk
Adds authorization logic that maps user-supplied relative paths to filesystem locations via projected activities; ambiguous or out-of-window matches fail closed, but incorrect correlation could still expose wrong files within session storage.
Overview
Assistant messages can show generated image paths (e.g.
`images/1.jpg`) as inline previews instead of plain code, with click-to-expand in the existing lightbox.A new
thread-artifactAssetResourcecarriesthreadId,turnId,messageId, and a relative image path.assetsCreateUrlresolves it by loading thread detail, matching the path to tool activity output viaThreadArtifactResolver(message time window, unique suffix match, fail-closed on ambiguity), then issuing a signed URL through the existingworkspace-fileflow against the resolved session directory.issueAssetUrlno longer acceptsthread-artifactdirectly.ChatMarkdown detects un-fenced inline code that normalizes to a safe image reference and, when turn/message context is passed from MessagesTimeline, fetches a URL and renders
MarkdownThreadArtifactImage(~25% width). Client and server both reject absolute paths, traversal, and non-image extensions.Reviewed by Cursor Bugbot for commit fdf4006. Bugbot is set up for automated code reviews on this repo. Configure here.