Skip to content

fix(native-chat): keep attachments on multi-image sends - #16977

Open
choihjin wants to merge 1 commit into
stablyai:mainfrom
choihjin:fix/native-chat-multi-image-fold
Open

choihjin wants to merge 1 commit into
stablyai:mainfrom
choihjin:fix/native-chat-multi-image-fold

Conversation

@choihjin

Copy link
Copy Markdown
Contributor

ELI5

Attach one image to a chat message and it shows up. Attach two and both disappear, and the raw [Image: source: /path] text Claude uses to record them shows up as if you had typed it. The fold that turns those markers into attachments only ever matched a one-image send.

What Changed

src/shared/native-chat-image-transcript-markers.ts:

  • Replaced soleText with imageSourcePaths(message), which collects a marker path from every block of a turn. It returns empty unless every block is a text block that is a marker, so a turn that also carries prose is never rewritten.
  • isImageSourceUserTurn and normalizeImageTranscriptMessages both go through it; the run-walk over consecutive marker turns now appends each turn's whole path list.
  • The standalone branch emits one image-ref per path from that turn only — index is not advanced past the run there, so following marker turns are still emitted by their own iteration.

No change to how a marker turn pairs with its prompt, to [Image #N] stripping, or to the ordering the fold expects. The only thing that moved is the block-count gate.

Why

Claude writes one transcript record per send, not per image. An N-image send arrives as a prompt record with N image parts plus a marker record carrying N [Image: source: …] text parts, and decodeClaudeTranscriptLine maps that record to one message with N blocks. The gate required exactly one:

function soleText(message: NativeChatMessage): string | null {
  return message.blocks.length === 1 && isTextBlock(message.blocks[0])
    ? message.blocks[0].text
    : null
}

So only a single-image send took the fold. Everything above it produced zero image-ref blocks and left the markers as prose.

Shapes sampled from local Claude transcripts, prompt record ↔ its marker record:

prompt content parts marker content parts occurrences
text, image text 44
text, image, image text, text 6
text, image ×3 text ×3 3
text, image ×4 text ×4 1
text, image ×24 text ×24 1

Only the first row worked.

Linked Issue

Fixes #16972

Visual Proof

No new UI. The change is that a multi-image send renders as attachments instead of as raw marker text, which is the pipeline output below rather than a new surface.

Before, on cf5e087246 — normalizeImageTranscriptMessages over a real-shaped two-image send:

user: [text<look at these>]
user: [text<[Image: source: /tmp/a.png]>, text<[Image: source: /tmp/b.png]>]

After:

user: [image-ref, image-ref, text<look at these>]

I did not capture the rendered pane; what I measured is the pipeline output the pane consumes.

Testing

Final merge base: cf5e087246

Automated — four cases added to src/shared/native-chat-image-transcript-markers.test.ts:

case expectation
multi-marker turn followed by its prompt merges to image-ref ×2 + the prompt text
multi-marker turn standing alone keeps all three paths as image-ref ×3
turn mixing a marker with prose left untouched
isImageSourceUserTurn true for marker-only, false when prose is mixed in

Confirmed RED on the merge base: the first, second and fourth failed (expected [ …(2) ] to have a length of 1 but got 2, markers still text, expected false to be true). The mixed-prose case already passed, so it is a guard rather than new behavior — it fails if the collector is loosened to "any block is a marker".

npx vitest run --config config/vitest.config.ts \
  src/shared/ src/renderer/src/components/native-chat/ src/main/native-chat/
# Test Files 685 passed | 4 skipped (689) | Tests 7201 passed | 65 skipped (7266)

tsc --noEmit clean for both config/tsconfig.tc.web.json and config/tsconfig.node.json; oxlint and oxfmt clean on the changed files.

Platforms: mobile re-exports these functions verbatim from
mobile/src/session/mobile-native-chat-image-transcript-markers.ts ("Single-sources the marker logic"), so it picks the fix up with no mobile change. I could not run the mobile suite — mobile/ fails to resolve expo/tsconfig.base.json in this environment and all 136 files fail that way on a clean merge base too. Exercised on macOS; the logic is pure and platform-independent.

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

AI Disclosure

Investigated and written with Claude (Opus 5) via Claude Code, then reviewed with Codex (codex review), which returned no findings. The transcript shapes above were measured from real local transcripts, and the RED/GREEN runs and suite numbers were actually run.

Review

Adjacent open work, no overlap that I can see: #14939 touches the mobile re-export for literal-marker stripping, and #11712 preserves attachments across transcript refresh. Neither changes the block-count gate this PR fixes.

While reporting #16972 I also noticed the doc comment says marker turns come before the prompt, while every sampled occurrence has them after — the single-image case survives only via the standalone branch, so an image renders as its own bubble rather than as an attachment on the message it was sent with. That is left alone here; happy to follow up if it is not intended.

Claude writes one transcript record per send, not per image, so a two-image
send arrives as a single turn carrying two [Image: source: …] text blocks.
The fold matched only a sole-block turn, so every send past the first image
lost its attachments and leaked the raw markers as prose. Collect the paths
from all of a turn's blocks, and only when every block is a marker so a turn
that also carries prose is left alone.

Fixes stablyai#16972
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0ad12909-9003-4af0-b1b3-62c3963af4bb

📥 Commits

Reviewing files that changed from the base of the PR and between f5fb60b and d5c31ef.

📒 Files selected for processing (2)
  • src/shared/native-chat-image-transcript-markers.test.ts
  • src/shared/native-chat-image-transcript-markers.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change adds support for multiple image-source text blocks in user turns. Validation now requires every block to contain a valid image marker and collects all image paths. Transcript normalization aggregates paths across consecutive source turns, merges them with a following prompt when applicable, and emits separate image references otherwise. Tests cover multi-image merging, standalone turns, mixed prose, and turn classification.

Merge Risk: ⚪ Minimal · up to d5c31

The change preserves multiple image attachments instead of leaving their source markers as visible text, while mixed prose remains untouched. It is merge-ready after normal checks and review, with no actionable merge-blocking risk remaining.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes preserving attachments for multi-image sends.
Description check ✅ Passed The description covers the change, rationale, linked issue, testing, AI disclosure, review notes, and behavior evidence. It is sufficiently complete for the required template.
Linked Issues check ✅ Passed The implementation directly addresses issue #16972 by recognizing marker-only turns with multiple text blocks, converting every image path to an image-ref, and preserving turns that contain prose.
Out of Scope Changes check ✅ Passed The changes are limited to native-chat image transcript normalization and focused tests for the reported multi-image attachment behavior.
  • Fix all pre-merge checks with AI

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.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Native chat drops image attachments when a send carries two or more images

1 participant