Skip to content

feat(mobile): show chat visuals inline in the phone's native chat - #26071

Merged
brennanb2025 merged 34 commits into
mainfrom
brennanb2025/inline-visuals-mobile
Oct 7, 2026
Merged

brennanb2025 merged 34 commits into
mainfrom
brennanb2025/inline-visuals-mobile

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 11 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​759 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​759
Prod 28 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1274 $\color{#cf222e}{\Huge{\mathbf{−}}}$​89 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1185

ELI5

When an agent in a native chat wants to show a chart, it writes a small web page into the chat's own visuals folder and puts one line in its reply, like ::orca-visual{file="usage-chart.html" title="Usage by day"}. The desktop learns to draw that page inside the reply in the base PR. On the phone, that line still showed up as raw text. This PR makes the phone draw the chart inside the reply too, lets you open it full screen, and keeps the agent's page locked in a box it cannot get out of.

What Changed

Before: in a structured chat on the phone, the visual line was shown as literal text (::orca-visual{file="…"}), and the chart never appeared.

After (iOS and Android app):

  • A finished assistant reply shows the visual inline, sized to the page's own height (80–2000 px), on the chat's dark theme with the chart colors the desktop uses.
  • A top-right expand button opens it full screen; Close or the Android back button returns.
  • A link inside the visual opens in the phone's browser, only after a real tap on the visual, at most once per tap.
  • While loading, a quiet reserved box shows. If the host refuses the file (missing, too large, outside the folder) or cannot be reached, one muted line reads "Visualization unavailable"; tapping it tries again.
  • While the agent is still writing, a visual line it is in the middle of typing at the very end of its reply is held back instead of flashing raw syntax. Once the line is finished, or the agent moves on to a tool call or asks a question, the visual shows.
  • A drag that starts on a visual scrolls the chat. A visual wider or taller than its inline box is seen in full via full screen.
  • With a desktop that does not support visuals yet, the line shows "Visualization unavailable" right away.

Mechanism:

  • Finding the line: the phone's chat markdown now recognizes the visual line using the shared grammar from the base PR (src/shared/native-chat-visual-directive.ts). Only top-level lines outside code fences count (not quotes, lists, inline code or indented code), at most 8 per message. The line is lifted out before the phone's HTML clean-up step so a title is never rewritten. Only assistant rows of structured chats get this; every other markdown surface is unchanged.
  • Reading the file: through the base PR's agentSession.readVisual on the host that owns the chat (now on the mobile method allowlist). The phone sends only the chat id and a bare file name; no path ever leaves the host. A small in-memory cache (16 entries, 4 MB of text) revalidates on every mount by content revision, shares concurrent reads, and drops a visual the host now refuses. Lost contact retries twice automatically, then waits for a tap; nothing latches.
  • Isolation: a phone WebView's top document cannot be sandboxed, so the agent's page never runs there. The WebView loads a trusted host page we write. The agent's page runs in a sandbox="allow-scripts" frame inside it, with an opaque origin: no storage, cookies, same-origin access, popups, forms or top navigation. It uses the base PR's shared shell (src/shared/native-chat-visual-shell.ts): content policy first, theme variables, height and link reporting. The host page relays only messages from that frame, stamps them with a random per-frame token the agent's page never sees, and asks only two things of the app: a height, which the shared height governor (moved to src/shared so the phone can bundle it) clamps, rate-limits and stops from running away, and an http(s) link, which needs frame focus and user activation.
  • Locking the message channel: react-native-webview exposes its message channel to every frame on both platforms, including the sandboxed one. On Android a non-string message from there would crash the app. The repo's existing patch to that library now drops any message that is not a string from the main frame, on iOS and Android. That is the same rule the app's own shell bridge already enforces. The token check stays as a second fence.
  • Navigation lockdown: only about:blank, about:srcdoc and in-page anchors may load. The WebView whitelist is * on purpose: anything outside the library's whitelist is opened in the system browser by the library itself, with no gesture check. If the agent's frame ever loads a second document, the host page removes it and the visual shows as unavailable.
  • Inline frames do not scroll: iOS gives a scrollable frame its own scroll view, which swallowed drags meant for the transcript (found in simulator QA). The inline frame is sized to its content and set not to scroll; full screen still scrolls.
  • Live replies: structured replies grow in place, so the hold-back applies only to the last block of the newest assistant row of a working turn with no question or approval open, computed in the turn hook that already orders the rows.
  • Crash recovery: if the WebView's web process dies, the visual reloads once. If it dies again, it shows as unavailable, and a tap starts over.
  • Hybrid web page build: the experimental web page build (off in every default build) is served under a policy that forbids inline scripts in any frame, so a visual renders there sealed (no scripts) in a fixed-height frame. That is the same approach the page already uses for HTML file previews. The page's policy is not loosened.

Why

  • Trusted host page + opaque sandboxed frame, not loading the agent's HTML as the WebView's page. A top-level native WebView document gets no sandbox, and a <meta> policy cannot sandbox it. The common pattern serves the page over HTTP with a sandbox policy in a response header. Orca's phone has no per-file HTTP endpoint to the host (it reads over the runtime connection), so the frame sandbox is the equivalent isolation.
  • Patching the library rather than switching bridges. I considered signalling by navigation instead of messages. Android's navigation hook falls back to allowing the load when the app is slow to answer (250 ms), which would make visuals flaky and less safe on a busy phone. Filtering at the native receiver removes the whole class (subframe senders, non-string payloads) for every WebView in the app. The terminal, Mermaid and editor WebViews only post from their main frame, so they are unaffected.
  • One shared shell and one height governor with the desktop, so the security boundary cannot drift between clients.

Differences from the common pattern

  • Live height fit and full-screen open: intended. The common mobile pattern sizes the row from server-measured heights. Here the page reports its height and the shared governor applies it, with the same caps as desktop.
  • Isolation by a sandboxed frame inside a trusted page, instead of a document served with a sandbox response header: intended. Same isolation result (opaque origin, scripts only), different delivery, because the phone has no HTTP route to the visual file. The extra native-channel filter is what makes the frame approach safe on this WebView library.
  • Theme is fixed at load, no live updates: intended. The phone has one dark theme today. The visual gets it before its content runs, so there is nothing to follow.
  • Hybrid web page build renders visuals sealed (no scripts): temporary. Follow-up: have the shell serve the visual from its own scheme with a sandbox response header, behind a negotiated capability. That needs a native shell change, so it is out of scope here. The build is off by default.
  • Network: scripts, styles, fonts and images may load from a pinned CDN allowlist; fetch, XHR, WebSocket, frames and workers may not. This is the disclosed, accepted product decision from the base PR: a page can still encode data in a request to an allowed CDN (and WebRTC is not covered by the content policy), so this is not a guarantee that no data leaves.

Known residual risks (accepted, not fixed here)

  • Cost before the native filter: a hostile visual can still call the native message handler in a tight loop. The patched filter drops each message, but only after it reaches the app process. Not measured on a device.
  • Android file picker: a tap on a file input in a visual can open the picker. The sandbox cannot block that, and what the user picks could leave through an allowed CDN. The frame's permissions policy denies camera, microphone, geolocation, clipboard and display capture.
  • Clipboard on a tap: clipboard-write 'none' covers the async clipboard API, not execCommand('copy') during a tap.
  • Very old Android WebViews: the library's fallback bridge reaches every frame; the per-frame token still rejects forged messages, so this is a flood risk only.
  • Shared Android renderer: a visual that hangs or crashes its renderer affects the app's other WebViews (terminal, Mermaid, editor). Each visual reloads once, then shows as unavailable. Not exercised on a device.
  • Cap of 8 visuals per text block, not per reply; offscreen rows rely on the list's own virtualization rather than a separate lazy mount.

Linked Issue

Stacked on the base PR (branch brennanb2025/inline-visuals-render), which adds the grammar, the host read, the shared shell and the desktop rendering. A sibling PR teaches the agents to write visuals.

Visual Proof

iOS Simulator (iPhone 17 Pro, iOS 26.5), a dev client built from this branch, paired to the repo's mobile mock host serving a structured chat whose replies contain visual lines.

Before: the line is raw text After: inline, fitted, themed Full screen
ios-00-before.png ios-02-chart.png ios-04-fullscreen-chart.png
Sandbox probe: origin null, storage blocked Probe tried the native channel directly; app received nothing Link tap opened the browser
ios-05-probe-inline.png ios-07-bridge-probe.png ios-08-link-opened-safari.png

After killing the visual's web processes, both visuals reloaded:

ios-12-after-webcontent-kill.png

Testing

  • I manually tested these changes locally (iOS Simulator; see the review comment for the full list and what was not verified)
  • Automated tests added/updated

New tests:

  • markdown placement: contexts, CRLF, titles kept verbatim, the cap, placeholder aliasing
  • read cache: revalidation, refusal eviction, concurrent sharing, host/session isolation, bounds
  • the hook's bounded retry and no-latch behavior
  • bridge validation (token, channel, schemes, sizes, non-strings)
  • host-page construction (sandbox flags, script-literal escaping)
  • row wiring (which rows render visuals, hold-back only on the growing block)
  • the turn hook's growing-row flag (newest assistant row only, never while a prompt is open)

Gates run locally:

  • mobile tsc and the tests-typecheck ratchet
  • tc:node and the web typecheck
  • oxlint
  • the changed-code quality gate
  • an importer sweep of 46 mobile test files plus the root tests touched

Not tested on Android: this machine has no JDK to build an Android dev client. The Android half of the native patch was checked against the androidx.webkit 1.14.0 API (getType(), TYPE_STRING) but not compiled.

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Security: see Isolation and Locking the message channel above. Two review rounds included a dedicated security pass.
  • Cross-platform: iOS and Android code paths are the same except the native patch, which covers both.
  • SSH/remote: the visual is read from the chat's owning host; nothing local-only.
  • Backwards compatibility:
    • An older host answers method_not_found, and the visual shows as unavailable.
    • An older phone shows the line as text.
    • The react-native-webview patch ships with the app binary.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…s visuals folder

A shared grammar for the ::orca-visual{file="..." title="..."} reply line,
the per-chat visuals folder location on the owning host, and the
agentSession.readVisual runtime method that reads one visual with lexical and
canonical containment, a 512 KiB bounded read and UTF-8 refusal.
One string builder every client wraps a visual's HTML with: the policy
(CDN assets only, no fetch, frames, workers, forms or base rewrites),
the theme variables, and a prelude that reports height, routes links to
the parent and refuses navigation. Also the validated frame-to-parent
message reader and the live theme message.
A finished assistant reply's ::orca-visual line now shows the visual
inline, read from the chat's owning host through agentSession.readVisual.
The visual runs in an opaque sandboxed child of a trusted host document
inside the WebView; the app accepts only a token-checked height and an
http(s) link opened under user activation. Navigation away from the
visual's own document is refused, a dead web process reloads once, and
the visual opens full screen. The hybrid shell's page renders it sealed.
Native-chat assistant replies render a ::orca-visual{...} line as the chat's
HTML visual in an opaque, scripts-only sandboxed frame: CSP first, the host
frame navigation guard registered before content runs, live theme without a
reload, fitted height, links opened in the viewer's browser only from a real
gesture, lazy mount, and one muted line when the visual cannot be shown.
Open in sidebar shows the same frame in the right sidebar, widened while it
is open and restored after.
…y message the app

Review round 1:
- use PR 1's shared visual shell and height governor instead of a second
  builder; the app decides heights and pushes them to the host page
- react-native-webview patch: the message channel accepts only string
  messages from the main frame on iOS and Android, so a visual in its
  sandboxed child cannot reach it (or crash Android with a non-string)
- structured replies grow in place: while a turn works, a row holds back a
  directive still being typed at its tail; finished lines mount
- links need child focus + activation, one per activation window
- a refused read takes the visual down and drops its cached bytes
- in-page anchors load; text spelling a placeholder renders no visuals
The phone bundles only src/shared, so the governor the desktop frame uses
moves there unchanged and the mobile frame shares it.
- host page relays only messages on the visual's own channel, as its own
  copy, so a visual cannot push oversized fields through it; relay rate
  halved; a link is validated before it uses up the link window
- the frame denies camera, microphone, geolocation, clipboard and display
  capture; iOS media capture requests are denied
- react-native-webview patch: the iOS history-shim handler also accepts
  only main-frame string messages
- only the newest assistant row of a working turn holds back an unfinished
  directive; earlier finished rows show their visuals
- an error reply or older host is not a verdict: it keeps a visual on
  screen and its cache, and retries; only a host refusal takes it down
iOS gives a scrollable frame its own scroll view, which took the drag;
the inline frame is sized to its content, so it no longer scrolls.
Full screen still does.
- hold back only the last block of the newest assistant row, and not
  while a question or approval is open, so a visual followed by a tool
  call or a pending question shows at once
- an older host (method_not_found) reads as unavailable without retries
- drop a stray @pnpm/exe lockfile block; only the patch hash changes
@brennanb2025

brennanb2025 commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Review summary

Head: 32c3480b601. Base: main, merged at 7f43d9b2c26 after PR #26103 landed.

Review rounds

Round 1 (correctness + a separate security pass):

  • Found:
    • This PR had forked the base PR's shared visual shell.
    • The streaming guard never fired, because structured replies grow in place and have no streaming row.
    • The sandboxed visual could reach react-native-webview's native message channel directly. On Android, a non-string message there would crash the app. On both platforms it could be flooded.
    • A height runaway loop, multiple link opens per tap, a stale cache after a refusal, and in-page anchors being refused.
  • Fixed:
    • Adopted the shared shell, and moved the shared height governor to src/shared so the phone can bundle it.
    • Patched react-native-webview on iOS and Android so the channel accepts only string messages from the main frame.
    • Hold back an unfinished directive only while the turn works.
    • Links need frame focus and user activation, one per activation window.
    • A refused read drops the cache.
    • Anchors allowed.

Round 2:

  • Found:
    • The host page relayed the visual's channel field without a length limit (P1).
    • The iOS history-shim handler had no frame check (P2).
    • Every row of a working turn was held back, not just the growing one (P2).
    • An error reply took down a visual already on screen (P3).
  • Fixed:
    • Only an exact channel match is relayed, rebuilt from the host's own copy.
    • History-shim guard added to the patch.
    • Only the newest assistant row holds back.
    • An error reply counts as no verdict.
    • Deny-all allow policy on the frame; iOS media capture denied.

QA, between rounds:

  • Found: on iOS, a drag that starts on a visual did not scroll the chat.
  • Fixed: the inline frame no longer scrolls (scrolling="no"); full screen still does.

Round 3:

  • Found:
    • A visual followed by a tool call or a pending question in the same turn stayed hidden (P2).
    • A stray @pnpm/exe lockfile block (P3).
    • An older host took about 6.5 s before showing "unavailable" (P3).
  • Fixed:
    • Hold back only the last block of the newest row, and never while a prompt is open.
    • Lockfile now changes only the patch hash, which equals the patch file's SHA-256.
  • Partly fixed: the older-host case first caught only method_not_found; round 4 completed it.

Round 4:

  • Found: no P1 or P2. A phone gets forbidden from an older desktop's mobile allowlist, not method_not_found.
  • Fixed: now handled with the existing mobile method-unavailable predicate.

Live QA

iOS Simulator (iPhone 17 Pro, iOS 26.5). The dev client was built from this branch, including the native patch, and rebuilt after each native change. The app was paired with the repo's mobile mock host serving a structured chat with three visual lines; that mock scenario is QA tooling and is not committed.

Verified:

  • Rendering:
    • The chart renders inline, themed, with a fitted height.
    • Full screen opens and closes.
    • A missing file shows "Visualization unavailable", and a tap retries.
  • Isolation (sandbox probe):
    • The visual's origin is null, at about:srcdoc; localStorage throws SecurityError; cookies are empty.
    • fetch is blocked.
    • window.open does nothing.
  • Native channel:
    • The visual called window.webkit.messageHandlers.ReactNativeWebView.postMessage directly with a forged JSON string and with an object.
    • The app received only the host page's token-stamped size messages, confirmed by a temporary log since removed.
  • Navigation:
    • The visual's location.href = 'https://…' never reached the app's navigation hook: the host page's frame-src 'none' blocked it inside WebKit.
    • The escape detector then removed the frame and showed "unavailable". Nothing remote loaded.
  • Links: tapping a link in the visual opened Safari, under the focus + activation rule.
  • Process recovery:
    • After the simulator's web content processes were killed (only that simulator's recorded PIDs), both visuals reloaded.
    • A second kill gave "unavailable", and a tap brought the visual back.
  • Scrolling: after the fix, a drag that starts on a visual scrolls the chat.
  • Before state: with no visual renderer, the line shows as raw text.

Gates run locally

  • Mobile tsc and the tests-typecheck ratchet.
  • Desktop typechecks: tc:node and web (tsconfig.tc.web.json).
  • oxlint and the changed-code quality gate: pass.
  • Tests:
    • an importer sweep of 45–46 mobile test files plus the session/overlay importers;
    • the root shell, governor, frame and allowlist tests;
    • the mobile web overrides census.

CI, classified (head 32c3480b601: base main)

PR #26103 was squash-merged to main, leaving this branch's original stack commits in its comparison. Merged main at 7f43d9b2c26 into 32c3480b601 without conflicts. The diff now contains only this PR's 39 files; the desktop renderer and shared visual code have no diff against main. This retarget adds no change to how visuals appear or behave on the phone.

All checks finished: 22 pass, 15 skipped, 0 fail, 0 pending.

  • Desktop CI passes static analysis/typecheck, all ten unit-test shards, relay integration, both packaging jobs, unit-selection evidence and the final verification job.
  • Mobile Checks, the mobile web app bundle, repository guards and headless-server checks pass. The web job finished on its first attempt after a prolonged WebKit installation wait; its app bundle build had already passed.
  • End-to-end and cross-version wire compatibility are skipped by path routing; the other skips are opt-in or unrelated jobs.
  • No failed CI job, no CI rerun, and no new failure against main. PR feat(mobile): show chat visuals inline in the phone's native chat #26071 stays ready and unmerged.

Local checks on this merged tree:

  • Node, web, CLI and mobile typechecks pass. The mobile test-typecheck ratchet passes: 943 test files in the program, 123 grandfathered files, four deliberate exclusions.
  • The first static-check pass completed 20/22 checks. Five existing import-cycle warnings came from mobile transport and terminal/document files and their direct partners, all byte-identical to main; both directories have no PR diff. No main fix was carried.
  • The changed-code gate's first three scans found zero new diagnostics. Its type-aware scan never obtained a slot within the machine queue's 30-minute limit, so that local gate did not complete. The full repeated static-check pass finished 22/23, including the changed-code gate, React Doctor and typechecks. Its only failure was the same five unchanged main import cycles. No queue bypass was used.
  • The 51-file mobile sweep passed 49 files and 463 tests. Eight tests timed out without an assertion failure: the iOS selection test's cold import, six short-chat snapshot cases and the long-chat snapshot case. An isolated two-file rerun repeated those eight timeouts (11 tests passed).
  • A bounded comparison using main's versions of MobileMarkdown, its parser and turn disclosure reproduced the same six short-chat timeouts; 13 tests passed, including selection and the long-chat case. The test files and configuration are identical to main. No test timeouts were increased and no source files were changed for this comparison.
  • The machine was heavily loaded. This PR's feature source and native patch are unchanged from the earlier green head c37b189a78c; main's newer web override entries were preserved. As directed by the coordinator, local timing investigation stopped after the comparison and the new head was pushed for CI to decide. Mobile Checks passes on the new head: 945 test files and 10,185 tests pass; two files and six tests are skipped. Both locally failing files pass, including all eight locally timed-out cases. These failures did not reproduce in CI; no CI rerun was needed.
  • All 11 targeted root feature suites passed (174 tests): visual shell, parser, governor, rendering, reads, security, RPC allowlist and the web override list. The additional cross-version unit suite timed out in its unchanged 180-second release-comparison setup, so its 41 cases were skipped and are not counted as passing.

No app, simulator, QA rig or agent CLI was launched for this retarget. The earlier iOS evidence remains historical evidence; it was not rerun.

Since the last summary

  • Merged the base PR's 14ff4867099.
  • Used its shared withoutNativeChatVisualDirectiveLines helper so a visual line never shows in two phone text surfaces:
    • the worktree list's agent row: a reply that is only a visual falls back to the prompt;
    • the message actions sheet's copied text: lines inside a code fence stay.
  • Tests added for both (8bcd5a738d4).

Not verified

  • Android: not built or run (no JDK on the QA machine). The Android half of the patch was checked against the androidx.webkit 1.14.0 API but not compiled.
  • The hybrid shell's web page build (off by default), which renders visuals sealed.
  • Live streaming against a real agent. Hold-back is covered by unit tests only.
  • Real hosts: a real paired desktop or SSH host. QA used the mock host.
  • Residual-risk device checks: the cost of a message flood from the visual inside the native layer, the Android file picker, execCommand('copy'), and Android shared-renderer hangs. These are disclosed in the PR body.
  • VoiceOver / TalkBack.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 7, 2026 05:12
…streaming hold

Registers agentSession.readVisual from the methods index so the structured
method file stays under its line budget, replaces reflective reads with checked
narrowing, moves the pure height governor to src/shared for mobile, and holds a
half-written directive tail while the turn works (structured text rows carry no
running state).
Re-checks after the open that the chat's visuals folder is still the real
directory at Orca's path, reports unexpected filesystem faults by code without
host paths, and lets one click in a visual open at most one page.
… review fixes

One shared helper drops visual lines (outside fenced code) from reply text where
it becomes plain text: the structured status summary that feeds the sidebar row,
dashboard, notifications, phone rows and handoffs, and AI Vault reply previews.
Review fixes: height also counts a pinned body's overflow, only the live
frontier row holds a half-written visual line, the runaway-height stop needs the
same step repeated, and any host refusal evicts the cached revision.
…nal-paths helper

Main removed the per-chat journal paths and the journal database's state
directory; the visuals folder keeps the same sha256 layout on its own and the
read method uses the profile state directory the chat host is opened in.
…sage text

A reply's ::orca-visual line renders only in the transcript. The
worktree list's agent row and the message actions sheet's copy text now
drop it with the shared helper; a reply that is only a visual falls back
to the prompt, as an empty one does.
…al lines

Reply previews in Agent Session History drop visual lines per text part before
lines are folded; the frame adds a body's overflow only when the body really
overflows; fence tracking follows CommonMark closers and openers; the copy
button copies a reply without visual lines; a coded read fault keeps its cause.
Removing visual lines now closes only the gap each removal leaves, instead of
collapsing blank lines across the whole reply and trimming its indentation; the
visuals folder is checked parent first again so a broken path answers the same
way every time.
@brennanb2025
brennanb2025 changed the base branch from brennanb2025/inline-visuals-render to main October 7, 2026 18:13
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⛔ Files ignored due to path filters (1)
  • mobile/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3ecd069d-4c92-48e2-aa1a-f420df25720b
📥 Commits

Reviewing files that changed from the base of the PR and between c37b189 and 32c3480.

⛔ Files ignored due to path filters (1)
  • mobile/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

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
📝 Walkthrough

Walkthrough

The change adds native-chat visual directives across desktop and mobile chat surfaces. It defines shared parsing, validation, filtering, sandboxing, sizing, and frame-message handling. It adds secure visual-file reads through agentSession.readVisual, with revision-aware caching and retries. Desktop and mobile clients render inline visuals and sidebar or fullscreen views. Assistant previews, labels, copied text, and summaries omit visual directive lines. WebView handlers accept only main-frame string messages.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to c37b1

Inline chat visuals are mostly well contained. Session previews can still do full-length work on very long assistant replies. On mobile, a visual can show as unavailable after one process crash instead of reloading. Both fixes are small follow-ups.

🚥 Pre-merge checks | ✅ 4 | ❓ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ❓ Inconclusive Docstring coverage is 37.84% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 50 files. (37 skipped… 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 the primary change: inline chat visual rendering on mobile.
Description check ✅ Passed The description is detailed and covers the user impact, implementation, rationale, security, testing, visual proof, compatibility, and known limitations. The linked issue field is not populated with a…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 37.84% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 50 files. (37 skipped: 3 unsupported, 34 over the file limit.)

✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 74ca2c8b-64f0-42e0-8394-f248cd46949e
📥 Commits

Reviewing files that changed from the base of the PR and between 7f43d9b and c37b189.

⛔ Files ignored due to path filters (2)
  • mobile/pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • src/shared/rpc-contract/rpc-params-catalog.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing (87)
  • mobile/patches/react-native-webview@13.16.2.patch
  • mobile/src/components/MobileMarkdown.tsx
  • mobile/src/components/WorktreeAgentRow.tsx
  • mobile/src/components/WorktreeAgentRow.visual-line.test.tsx
  • mobile/src/components/inline-script-json.ts
  • mobile/src/components/mobile-markdown-parser.ts
  • mobile/src/components/mobile-markdown-visual-lines.test.ts
  • mobile/src/components/mobile-markdown-visual-lines.ts
  • mobile/src/components/pr-sidebar/MermaidDiagram.tsx
  • mobile/src/components/use-mobile-markdown-blocks.ts
  • mobile/src/session/MobileNativeChatMessage.tsx
  • mobile/src/session/MobileNativeChatOverlay.test.ts
  • mobile/src/session/MobileNativeChatOverlay.tsx
  • mobile/src/session/MobileNativeChatVisual.tsx
  • mobile/src/session/MobileNativeChatVisualFrame.tsx
  • mobile/src/session/MobileNativeChatVisualFrame.web.tsx
  • mobile/src/session/__mocks__/mobile-prompt-controller.ts
  • mobile/src/session/mobile-native-chat-controller-contract.ts
  • mobile/src/session/mobile-native-chat-merged-snapshot-parity-hooks.test.tsx
  • mobile/src/session/mobile-native-chat-message-plain-text.test.ts
  • mobile/src/session/mobile-native-chat-message-plain-text.ts
  • mobile/src/session/mobile-native-chat-message-visuals.test.ts
  • mobile/src/session/mobile-native-chat-visual-bridge.test.ts
  • mobile/src/session/mobile-native-chat-visual-bridge.ts
  • mobile/src/session/mobile-native-chat-visual-context.ts
  • mobile/src/session/mobile-native-chat-visual-host-document.ts
  • mobile/src/session/mobile-native-chat-visual-read.test.ts
  • mobile/src/session/mobile-native-chat-visual-read.ts
  • mobile/src/session/mobile-native-chat-visual-theme.ts
  • mobile/src/session/use-mobile-native-chat-controller.ts
  • mobile/src/session/use-mobile-native-chat-turn-disclosure-growing-row.test.tsx
  • mobile/src/session/use-mobile-native-chat-turn-disclosure.ts
  • mobile/src/session/use-mobile-native-chat-visual.test.tsx
  • mobile/src/session/use-mobile-native-chat-visual.ts
  • mobile/src/session/use-mobile-structured-agent-session.ts
  • mobile/web-entry/web-overrides.json
  • src/main/ai-vault/session-scanner-accumulator.ts
  • src/main/ai-vault/session-scanner-text-normalization.ts
  • src/main/ai-vault/session-scanner-values.test.ts
  • src/main/native-chat/native-chat-visual-file-read.test.ts
  • src/main/native-chat/native-chat-visual-file-read.ts
  • src/main/native-chat/native-chat-visuals-folder.ts
  • src/main/runtime/mobile-rpc-allowlist.test.ts
  • src/main/runtime/rpc/methods/index.ts
  • src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts
  • src/main/runtime/rpc/methods/structured-agent-session-visual.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-visual.ts
  • src/main/runtime/runtime-rpc/runtime-rpc-mobile-agent-session-methods.ts
  • src/main/window/host-frame-navigation-guard.test.ts
  • src/main/window/host-frame-navigation-guard.ts
  • src/main/window/main-window-webview-security.test.ts
  • src/main/window/main-window-webview-security.ts
  • src/renderer/src/components/native-chat/NativeChatInlineVisual.tsx
  • src/renderer/src/components/native-chat/NativeChatMarkdown.tsx
  • src/renderer/src/components/native-chat/NativeChatMarkdown.visual.test.tsx
  • src/renderer/src/components/native-chat/NativeChatMessageRow.test.tsx
  • src/renderer/src/components/native-chat/NativeChatMessageRow.tsx
  • src/renderer/src/components/native-chat/NativeChatTranscriptChrome.tsx
  • src/renderer/src/components/native-chat/NativeChatView.tsx
  • src/renderer/src/components/native-chat/NativeChatVisualFrame.test.tsx
  • src/renderer/src/components/native-chat/NativeChatVisualFrame.tsx
  • src/renderer/src/components/native-chat/NativeChatVisualPanel.tsx
  • src/renderer/src/components/native-chat/native-chat-visual-markdown-extension.tsx
  • src/renderer/src/components/native-chat/native-chat-visual-markdown-syntax.test.tsx
  • src/renderer/src/components/native-chat/native-chat-visual-markdown-syntax.ts
  • src/renderer/src/components/native-chat/native-chat-visual-owner.tsx
  • src/renderer/src/components/native-chat/native-chat-visual-read-client.test.ts
  • src/renderer/src/components/native-chat/native-chat-visual-read-client.ts
  • src/renderer/src/components/native-chat/use-native-chat-visual-document.ts
  • src/renderer/src/components/native-chat/use-native-chat-visual-theme.ts
  • src/renderer/src/components/right-sidebar/index.tsx
  • src/renderer/src/components/right-sidebar/right-sidebar-panel-content.tsx
  • src/renderer/src/components/right-sidebar/right-sidebar-width.ts
  • src/renderer/src/components/sidebar/CommentMarkdown.tsx
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/store/slices/editor/actions/right-sidebar-state.ts
  • src/renderer/src/store/slices/editor/actions/right-sidebar-visual-state.test.ts
  • src/shared/native-chat-visual-directive.test.ts
  • src/shared/native-chat-visual-directive.ts
  • src/shared/native-chat-visual-height-governor.test.ts
  • src/shared/native-chat-visual-height-governor.ts
  • src/shared/native-chat-visual-shell.test.ts
  • src/shared/native-chat-visual-shell.ts
  • src/shared/rpc-contract/agent-session-visual-params.ts
  • src/shared/structured-agent-session-latest-request.test.ts
  • src/shared/structured-agent-session-latest-request.ts
  • tests/e2e/cross-version-wire/structured-agent-session-surface-manifest.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Comment on lines +153 to +161
const restart = useCallback(() => {
if (restarts.current >= MAX_PROCESS_RESTARTS) {
onFailed()
return
}
restarts.current += 1
setHeight(MOBILE_NATIVE_CHAT_VISUAL_INITIAL_HEIGHT)
setGeneration((value) => value + 1)
}, [onFailed])

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.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Reset restarts when built changes.

restarts is a component-lifetime ref. The component never resets it when html changes. Suppose the web process dies once for one revision. A later revision then reaches restart with restarts.current === 1, so its first process crash calls onFailed() without the automatic reload. The fix: reset the counter in the existing [built, generation] effect when built changes, or in a separate effect keyed on built.

🐛 Proposed fix
+  useEffect(() => {
+    restarts.current = 0
+  }, [built])

}

function previewSource(text: string, dropVisualLines: boolean): string {
return dropVisualLines ? withoutNativeChatVisualDirectiveLines(text) : text

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.

🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

🔎 Supported by static analysis

🏁 Script executed:

set -u
printf '%s\n' '--- changed file ---'
nl -ba src/main/ai-vault/session-scanner-text-normalization.ts
printf '%s\n' '--- references ---'
rg -n -F -- 'extractPreviewContentText(' . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' || test "$?" -eq 1
rg -n -F -- 'normalizePreviewText(' . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' || test "$?" -eq 1
rg -n -F -- 'previewSource' src test tests . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' || test "$?" -eq 1
printf '%s\n' '--- diff from merge base ---'
git diff --no-ext-diff --unified=80 c8dcffce46d92ddbb1f511baf88df60d322f4ac1 c37b189a78cffdcf114158d26274d67c555ee8c7 -- src/main/ai-vault/session-scanner-text-normalization.ts src/main/ai-vault

Repository: stablyai/orca

Length of output: 41223


🏁 Script executed:

set -u
printf '%s\n' '--- directive helper ---'
rg -n -F -- 'withoutNativeChatVisualDirectiveLines' src shared . --glob='!node_modules/**' --glob='!dist/**' --glob='!build/**' 2>/dev/null || test "$?" -eq 1
printf '%s\n' '--- accumulator call paths ---'
nl -ba src/main/ai-vault/session-scanner-accumulator.ts | sed -n '210,290p'
printf '%s\n' '--- parser call sites ---'
for f in \
  src/main/ai-vault/session-scanner-codex-message-records.ts \
  src/main/ai-vault/session-scanner-droid-parser.ts \
  src/main/ai-vault/session-scanner-opencode-parser.ts \
  src/main/ai-vault/session-scanner-grok-parser.ts \
  src/main/ai-vault/session-scanner-kimi-parser.ts; do
  if [ -f "$f" ]; then
    printf '%s\n' "--- $f ---"
    nl -ba "$f" | sed -n '1,180p'
  fi
done
printf '%s\n' '--- bounds and helper usages ---'
rg -n -e 'boundedText|cap.*Preview|PREVIEW.*LIMIT|preview.*slice|withoutNativeChatVisualDirectiveLines' src shared --glob='!node_modules/**' --glob='!dist/**' --glob='!build/**' 2>/dev/null || test "$?" -eq 1

Repository: stablyai/orca

Length of output: 45239


🏁 Script executed:

set -u
printf '%s\n' '--- native-chat directive implementation ---'
nl -ba src/shared/native-chat-visual-directive.ts | sed -n '130,215p'
printf '%s\n' '--- bounded text implementation ---'
nl -ba src/main/ai-vault/session-transcript-message-content.ts | sed -n '145,175p'
printf '%s\n' '--- relevant preview exports ---'
rg -n -F -- 'export {' src/main/ai-vault/session-scanner-values.ts src/main/ai-vault/session-scanner-text-normalization.ts 2>/dev/null || true

Repository: stablyai/orca

Length of output: 5044


Keep assistant preview filtering bounded.

previewSource calls withoutNativeChatVisualDirectiveLines before the 220-character limit applies. That helper scans the full input and, when it finds a marker, splits, stores, and rejoins the full reply. The content path passes raw provider content into this operation without an upstream bound.

Filter incrementally and stop after collecting enough visible text for the preview.

@brennanb2025
brennanb2025 merged commit 1617ff3 into main Oct 7, 2026
37 checks passed
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.

1 participant