feat(mobile): preview images and markdown read by the model - #4835
Merged
Conversation
… crash, transient model catalog gap
Add read-tool markdown parser, inline ChatMarkdownText preview, full-screen reader modal, and ReadToolCard branch for .md/.mdx.
…S cache Thread an optional image-attachment sink through the chat processor and all three processor creation sites (live session, child hydration, history applyPage). The store strip is unchanged — zero added store RAM and no web behaviour change (web never passes the sink). Mobile caches image/* data URLs under Paths.cache/tool-card-images, deduped by part id, for Slice 4's file:// tool-card rendering.
…vening Records the planner-intervention incident as a learning and states the negative space in Planner Monitor Mode: never kill a live dispatch on a judgment call, never edit the worktree, never forge a dispatch-log marker; verify from the orchestrator's scrollback, scratch and git log first.
Move message-image-preview-modal to components/image-viewer-modal, add RNGH pinch/pan/double-tap zoom (own GestureHandlerRootView inside the Modal), and make onShare optional so tool cards can adopt the viewer. kilo-chat behaviour is unchanged: same share flow, same a11y labels, same inline share-error block.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe only product-code change since the previous review ( Files Reviewed (1 file)
Previous Review Summary (commit deda804)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit deda804)Status: 4 Issues Found | Recommendation: Address before merge Executive SummaryThe highest-risk finding is a latent multi-image collapse bug ( Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (21 files)
Reviewed by claude-sonnet-5 · Input: 38 · Output: 8.1K · Cached: 1.3M Review guidance: REVIEW.md from base branch |
…escalation proven)
jeanduplessis
approved these changes
Jul 29, 2026
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What: Mobile agent tool cards now preview the files the model read — rendered markdown for
.mdreads (with a full-screen reader) and inline images for image reads and MCP screenshots (with a pinch-zoom full-screen viewer).Why: A
.mdread rendered as raw line-numbered mono text, and an image read rendered nothing at all: the client SDK blanks attachment bytes (stripPartContentIfFile) before the store ever sees them, on both the live and history paths.How (four slices):
2f10f9dbc) —readtool cards for.md/.mdxparsestate.metadata.display(falling back to the legacystate.outputwrapper for parts that predate the field), render a capped inline markdown preview with a partial-read footer (lines X–Y of N/(truncated)), and open a full-screen markdown reader modal. Non-markdown reads are byte-identical tomain.onImageAttachmentsink + FS cache (91d2ab1b7) —createChatProcessoraccepts an optional sink called with(partId, mime, dataUrl)for eachimage/*attachment just before the existing strip runs; threaded through all three processor sites (livesession.ts, hydration + history replay insession-manager.ts). The strip itself is byte-identical tomain— the store retains nothing new. Mobile's sink writes the bytes once to theexpo-file-systemcache (in-memory dedupe + existence check) and exposes afile://URI via jotai; expo-image then gets native caching/downsampling. Web never passes the sink and is byte-for-byte unaffected.497207f34) — kilo-chat's image preview modal moved toImageViewerModalwith pinch/pan/double-tap-reset gestures (ownGestureHandlerRootViewinside the RNModal). kilo-chat keeps its share action unchanged; the tool-card viewer omits share (data URLs have no presigned URL/attachmentId).b18b2fb8c) —ToolCardShelltakes an optionalpart;ReadToolCardandGenericToolCard(coversmaestro_take_screenshotand any future MCP image tool) render cached images inline at a clamped aspect ratio with a skeleton, a terminal "unavailable" row when no cached bytes exist (container sessions strip server-side), and a static "Image unavailable" row on decode failure. Image reads suppress the mono output line.Scope note: the
packages/cloud-agent-sdkchange is a sanctioned scope break — the requesting human approved it with a performance constraint ("yes, but performant") and specified this sink + FS-cache design himself. The originally planned in-store retention cap was not shipped;MAX_RETAINED_IMAGE_URL_CHARSdoes not exist.Verification
packages/cloud-agent-sdk:pnpm typecheck+pnpm test(28 suites, 1059 tests) — sink fires per site, stored part still stripped, non-image/blank-url/sink-absent cases covered; the pre-existing strip test passes unmodifiedapps/mobile:pnpm format && pnpm typecheck && pnpm lint && pnpm check:unused && pnpm test(280 files, 2386 tests) — markdown parser golden fixtures, attachment filter, aspect-ratio clamp, FS-cache dedupe/failure pathspnpm typecheck;git diff --checkclean;scripts/lint-all.shcleanreadrenders inline in the expanded card, opens the full-screen viewer on tap, no mono output line (D10), no share button (D7)maestro_take_screenshotrenders the same way through the generic tool card.mdrenders markdown (heading/list/fence), no footer.mdrenders markdown + footerlines 1–200 of 1,450, unterminated fence closedmetadata.display) renders markdown via thestate.outputfallbackurlimage renders the terminalImage preview unavailable in this session.row, no broken image.tsread unchanged frommain(line-numbered mono in the wrapper)application/pdfattachment renders nothing new (and no spurious unavailable row)Attach filebutton is gated onbotStatus.capabilities.includes('attachments')(conversation-screen.tsx:72); adev:seedki_fake_*sandbox never advertises it; and a forced, correctly authenticatedbot-statusPOST is overwritten within ~15s of the conversation opening, because the app's ownrequest-bot-statuspoll escalates through kiloclaw'sdeliverChatWebhook, which throwshas no sandboxIdfor fake sandboxes → kilo-chat republishesonline:falseand the monotonic DO upsert NULLs capabilities (mechanism proven on device; learning committed inb2578064fasmobile-kilochat-attachments-capability-gate-blocks-e2e.md). A fix needs a product-side dev seed/fake-bot harness, which is out of scope. What covers it: slice 3's fresh impl-reviewer round (impl-reviewer-slice3-r1) returnedNo findings., and kilo-chat still suppliesonShareatmessage-attachment.tsx:135— the share path is unchanged by code inspection (slice 3's risk to kilo-chat is limited to an import move + tag rename + in-modal gestures;message-attachment.tsxis otherwise unchanged). Per the approved plan this is reported as unverified — never proxied with the tool-card viewer, never marked passed — and the requesting human has explicitly accepted shipping with criterion 10 unverified on device.Visual Changes
E2E screenshots (13) were captured by the verifier and are described here per card; they are
not attached because images cannot be uploaded via the CLI. They are preserved on the requesting
human's machine at
/private/tmp/claude-501/-Users-igor-Projects-cloud/cc2ef1f9-7c13-4287-80f8-f33266da28f8/scratchpad/e2e-shots/(
01-shot-png-expanded.png…13-docpdf-expanded.png). The most informative:07-big-expanded.png(partial markdown with the
lines 1–200 of 1,450footer and the mid-block code fenceauto-balanced),
01/02(image inline, then the full-screen viewer),04-maestro-screenshot-expanded.png(maestro_take_screenshotrendering identically toread,proving the shared
ToolCardShellplacement), and11-blank-unavailable.png(blankurldegrading to a muted message).
readof a PNG (monoImage read successfully→ inline preview)maestro_take_screenshot(mono output → inline preview)readof.md(line-numbered mono → rendered markdown).mdpart withoutmetadata.displaystate.outputfallbackurlimage (nothing → terminal state)Image preview unavailable in this session.row, no broken imagereadof.tsmainImageViewerModalwith pinch-zoom; share button retained (not reachable in local E2E — see Verification)Reviewer Notes
packages/cloud-agent-sdk/src/session-manager.ts+session.ts(three call sites, sink errors contained) and the FS-cache contract inapps/mobile/src/components/agents/tool-card-image-cache.ts(dedupe Set cleared on failure,file.existsskip)..kilo_workflow/(learnings + monitor-behavior fixes:04df12f32,fed917841,b2578064f) — no product code.services/session-ingestintegration tests are broken repo-wide (pgimported eagerly under miniflare; no CI runs them).apps/web/src/components/cloud-agent-next/types.tsstill duplicates the SDK strip logic (live on the app-builder path). With the sink design the two copies no longer diverge in behavior, but the duplication itself is untouched.main; the FS cache is OS-evictable and does not add to it).