fix(agent): send user image attachments as image blocks, not text (#5359) - #5370
Conversation
…nyhumansai#5359) The agent could not read images: PNG attachments failed entirely and JPEGs were described inaccurately. By the time a user turn reaches the tinyagents bridge, the multimodal pipeline has already rehydrated and normalized each attachment into an inline [IMAGE:data:<mime>;base64,…] marker — but chat_message_to_message dropped the whole message into a single ContentBlock::Text. That shipped the base64 to the model as literal text, so vision models never received a real image: PNG (lossless, large) is unrecoverable as text, while JPEG occasionally let the model guess, matching the reported symptom exactly. Parse the [IMAGE:…] markers out of the user text and emit a typed ContentBlock::Image per attachment (with the marker-free prose as a leading text block; image-only turns emit no empty text block). The provider layer already serializes ContentBlock::Image as an image_url part and forwards ImageRef.url verbatim — the marker payload is already a canonical data: URI — so no downstream change is needed. Non-vision models are unaffected: the pipeline never rehydrates a marker for them. Adds tests for a mixed text+image turn, image-only and multi-image turns, and an unchanged plain-text turn.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUser-message conversion now recognizes inline image markers, creates typed image blocks with MIME metadata, preserves text ordering, omits empty image-only text blocks, and keeps plain-text behavior unchanged. ChangesMultimodal message conversion
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/agent/message_convert.rs`:
- Around line 142-160: Update parse_image_markers and the surrounding message
conversion flow to preserve the original ordering of prose and image markers
instead of aggregating all text before images. Return ordered text/image
segments, construct ContentBlock values sequentially in that order, and preserve
the existing no-attachment behavior. Add a test covering prose between two image
markers.
🪄 Autofix
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: CHILL
Plan: Pro
Run ID: 54a170db-2b4e-4a7e-a36f-c165809bf82c
📒 Files selected for processing (1)
src/openhuman/agent/message_convert.rs
|
| Filename | Overview |
|---|---|
| src/openhuman/agent/message_convert.rs | Adds user_content_blocks to parse [IMAGE:…] markers into typed ContentBlock::Image blocks; fix is well-reasoned, well-tested, and includes tracing — one minor nit in the unreachable empty-blocks fallback |
Reviews (2): Last reviewed commit: "fix(agent): preserve image/text order an..." | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2576b863e6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…yhumansai#5359 review) Addresses review on tinyhumansai#5359: - Emit content blocks in source order: prose and images now interleave as the user wrote them (text, image, text, image), so a caption stays next to its image instead of all text being hoisted ahead of all images. - Only lift a marker whose payload is a provider-ready reference (a data: URI or an http(s) URL) into ContentBlock::Image; a bare path or an un-normalized marker is kept verbatim as text rather than sent as an image the provider would reject. - Add a debug log when image markers are lifted (per the repo debug-logging guidance for new flows). - Strengthen tests: assert the full ordered text/image sequence for a multi-image turn, and add a case that a non-data/http marker stays literal text.
M3gA-Mind
left a comment
There was a problem hiding this comment.
Reviewed against upstream/main: CI green on every lane that applies to the changed areas, no unresolved review threads, and the diff does what the title and body claim. Checked correctness, blast radius on shared seams, that the behaviour change is pinned by a test rather than only asserted, and i18n/em-dash compliance on any new user-facing string. Nothing blocking found.
Summary
[IMAGE:data:<mime>;base64,…]markers out of the user text into typedContentBlock::Imageblocks so the provider serializes them as realimage_urlparts.Problem
By the time a user turn reaches the tinyagents bridge, the multimodal pipeline has already rehydrated and normalized each attachment into an inline
[IMAGE:data:<mime>;base64,…]marker — butchat_message_to_messagedropped the whole message into a singleContentBlock::Text. That shipped the base64 to the model as literal text, so vision models never received a real image: PNG (lossless, large) is unrecoverable as text, while JPEG occasionally let the model guess — matching the reported symptom exactly.Solution
Parse the
[IMAGE:…]markers out of the user text and emit a typedContentBlock::Imageper attachment, with the marker-free prose as a leading text block (image-only turns emit no empty text block, which some providers reject). The provider layer already serializesContentBlock::Imageas animage_urlpart and forwardsImageRef.urlverbatim — the marker payload is already a canonicaldata:URI — so no downstream change is needed. Non-vision models are unaffected: the pipeline never rehydrates a marker for them.Submission Checklist
diff-cover(cargo-llvm-cov) enforces the gate.N/A: bug fix, no added/removed/renamed feature row.## Related—N/A: bug fix.N/A: fixes existing image-attachment behaviour; no new release-cut surface.Closes #NNN.Impact
image_urlparts.cargo fmt+cargo clippy -p openhuman -- -D warningsclean; unit tests green.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/agent-image-content-blocks-5359