Repository navigation
Native chat: add an inline visuals switch and simplify the skill - #26358
Conversation
|
Native chats previously received inline-visual instructions on every launch with no opt-out, and the instructions prescribed more style than needed. This follow-up adds a default-on switch in Settings > Chat for new Claude/Codex processes, plus the supplied lighter skill text. Running chats and saved visuals keep working.
Final CI completed successfully for The one final main merge completed without conflicts: merge commit All runnable native-chat cases completed; Claude’s normal Plan flow accepted by Brennan, Codex Plan N/A. Source diff remains focused and validation is complete. PR is ready for review and remains open. No PR merge or auto-merge. For the approval record: #26099’s earlier extra final-main merge and stuck-Linux-workflow cancellation were coordinator-approved, not user-approved. Round 3 did neither, and did not edit #26099 or push its old branch. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe changes add a default-enabled inline visuals setting to native chat and expose it in desktop Chat settings with search navigation. Native-chat visual preparation now checks the setting for each launch, and Claude launch flags reuse the preparation result. Tests cover setting behavior, provider launches, delivery, and rendering existing visual directives. The visual skill instructions also change their content guidance, HTML requirements, and reply formatting rules. Priority: ➖ Normal Merge Risk: 🔵 Low · up to On a cold Claude launch with inline visuals off, readable thinking may be unavailable. This is a bounded issue to accept or fix before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 22 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
2f067ec7-e22f-46bb-847c-f77e04f24713
📒 Files selected for processing (18)
resources/native-chat-visuals/skills/orca-chat-visuals/SKILL.mdsrc/main/claude/claude-structured-launch-flags.tssrc/main/native-chat/native-chat-inline-visuals-setting.test.tssrc/main/native-chat/native-chat-visuals-delivery.test.tssrc/main/native-chat/native-chat-visuals-delivery.tssrc/main/runtime/orca-runtime-get-worktree-ps.tssrc/main/runtime/runtime-store-contract.tssrc/main/runtime/structured-agent-runtime-registrations.tssrc/main/runtime/structured-agent-session-runtime.tssrc/renderer/src/components/native-chat/NativeChatMarkdown.visual.test.tsxsrc/renderer/src/components/settings/NativeChatExperimentalSetting.test.tsxsrc/renderer/src/components/settings/NativeChatExperimentalSetting.tsxsrc/renderer/src/components/settings/NativeChatInlineVisualsSetting.tsxsrc/renderer/src/components/settings/native-chat-experimental-search-entry.test.tssrc/renderer/src/components/settings/native-chat-experimental-search-entry.tssrc/renderer/src/i18n/locales/en.jsonsrc/shared/default-global-settings.tssrc/shared/global-settings-types.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| ## Writing the page | ||
|
|
||
| - Save it in the folder named by `ORCA_CHAT_VISUALS_DIR` (`printenv ORCA_CHAT_VISUALS_DIR`; in | ||
| PowerShell `$env:ORCA_CHAT_VISUALS_DIR`). If it's unset or you can't write there, skip the visual. | ||
| - One complete HTML file per visual, directly in that folder, under 512 KB, with a new file name | ||
| each time (e.g. `latency-by-region-7c1e.html`): letters, digits, `.`, `_`, `-`, ending in `.html`. | ||
| - Inline your styles, scripts and data. Libraries and fonts may load only from cdn.jsdelivr.net, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,45p' resources/native-chat-visuals/skills/orca-chat-visuals/SKILL.md
rg -n 'MAX.*(NAME|TITLE)|filename|fileName|reserved' src/main/native-chat src/renderer/src/components/native-chatRepository: stablyai/orca
Length of output: 27215
🏁 Script executed:
set +e
printf '%s\n' '--- changed hunk ---'
git diff --unified=30 c785c986c20241dba27cff35c452ebaef6b8930b e7bd0c7263a4c2da9f978883ce3755518289471d -- resources/native-chat-visuals/skills/orca-chat-visuals/SKILL.md
printf '%s\n' '--- visual references ---'
rg -n -i -e 'orca-visual' -e 'visual.*directive' -e 'directive.*visual' -e 'visual.*file' -e 'visuals.*dir' -e 'html.*(name|file)' src resources/native-chat-visuals --glob '!**/*.map' || test "$?" -eq 1
printf '%s\n' '--- candidate files ---'
rg --files src | rg -i 'visual|file.*link|attachment|directive'Repository: stablyai/orca
Length of output: 41934
🏁 Script executed:
set +e
git diff --unified=30 c785c986c20241dba27cff35c452ebaef6b8930b e7bd0c7263a4c2da9f978883ce3755518289471d -- resources/native-chat-visuals/skills/orca-chat-visuals/SKILL.md
rg -n -i -e 'orca-visual' -e 'visual.*directive' -e 'directive.*visual' -e 'visual.*file' -e 'visuals.*dir' -e 'html.*(name|file)' src resources/native-chat-visuals --glob '!**/*.map' || test "$?" -eq 1
rg --files src | rg -i 'visual|file.*link|attachment|directive'Repository: stablyai/orca
Length of output: 41817
🏁 Script executed:
printf '%s\n' '--- shared directive validator ---'
nl -ba src/shared/native-chat-visual-directive.ts | sed -n '1,145p'
printf '%s\n' '--- file read path ---'
nl -ba src/main/native-chat/native-chat-visual-file-read.ts | sed -n '1,245p'
printf '%s\n' '--- visual RPC and renderer bindings ---'
nl -ba src/main/runtime/rpc/methods/structured-agent-session-visual.ts | sed -n '1,180p'
nl -ba src/renderer/src/components/native-chat/NativeChatInlineVisual.tsx | sed -n '1,180p'
nl -ba src/renderer/src/components/native-chat/NativeChatVisualFrame.tsx | sed -n '1,180p'
printf '%s\n' '--- focused validator tests ---'
rg -n -C 4 -e '128|device|reserved|con|nul|filename|file name|isNativeChatVisualFileName|longer|\\.html' src/shared/native-chat-visual-directive.test.ts src/main/native-chat/native-chat-visual-file-read.test.ts src/main/runtime/rpc/methods/structured-agent-session-visual.test.tsRepository: stablyai/orca
Length of output: 42465
🏁 Script executed:
printf '%s\n' '--- shared directive validator ---'
nl -ba src/shared/native-chat-visual-directive.ts | sed -n '1,145p'
printf '%s\n' '--- file read path ---'
nl -ba src/main/native-chat/native-chat-visual-file-read.ts | sed -n '1,245p'
printf '%s\n' '--- visual RPC and renderer bindings ---'
nl -ba src/main/runtime/rpc/methods/structured-agent-session-visual.ts | sed -n '1,180p'
nl -ba src/renderer/src/components/native-chat/NativeChatInlineVisual.tsx | sed -n '1,180p'
nl -ba src/renderer/src/components/native-chat/NativeChatVisualFrame.tsx | sed -n '1,180p'
printf '%s\n' '--- focused validator tests ---'
rg -n -C 4 -e '128|device|reserved|con|nul|filename|file name|isNativeChatVisualFileName|longer|\.html' src/shared/native-chat-visual-directive.test.ts src/main/native-chat/native-chat-visual-file-read.test.ts src/main/runtime/rpc/methods/structured-agent-session-visual.test.tsRepository: stablyai/orca
Length of output: 42461
Document the filename constraints enforced by the visual validator.
The current guidance permits filenames longer than 128 characters, containing .., or using Windows device names. isNativeChatVisualFileName rejects these names, so parseNativeChatVisualDirectiveLine returns null and the visual is not mounted.
Suggested fix
-- One complete HTML file per visual, directly in that folder, under 512 KB, with a new file name
- each time (e.g. `latency-by-region-7c1e.html`): letters, digits, `.`, `_`, `-`, ending in `.html`.
+- One complete HTML file per visual, directly in that folder, under 512 KB, with a new file name
+ each time (e.g. `latency-by-region-7c1e.html`): start with a letter or digit, use only letters,
+ digits, `.`, `_`, and `-`, be at most 128 characters, contain no `..`, avoid Windows device
+ names such as `con` or `nul`, and end in `.html`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Writing the page | |
| - Save it in the folder named by `ORCA_CHAT_VISUALS_DIR` (`printenv ORCA_CHAT_VISUALS_DIR`; in | |
| PowerShell `$env:ORCA_CHAT_VISUALS_DIR`). If it's unset or you can't write there, skip the visual. | |
| - One complete HTML file per visual, directly in that folder, under 512 KB, with a new file name | |
| each time (e.g. `latency-by-region-7c1e.html`): letters, digits, `.`, `_`, `-`, ending in `.html`. | |
| - Inline your styles, scripts and data. Libraries and fonts may load only from cdn.jsdelivr.net, | |
| ## Writing the page | |
| - Save it in the folder named by `ORCA_CHAT_VISUALS_DIR` (`printenv ORCA_CHAT_VISUALS_DIR`; in | |
| PowerShell `$env:ORCA_CHAT_VISUALS_DIR`). If it's unset or you can't write there, skip the visual. | |
| - One complete HTML file per visual, directly in that folder, under 512 KB, with a new file name | |
| each time (e.g. `latency-by-region-7c1e.html`): start with a letter or digit, use only letters, | |
| digits, `.`, `_`, and `-`, be at most 128 characters, contain no `..`, avoid Windows device | |
| names such as `con` or `nul`, and end in `.html`. | |
| - Inline your styles, scripts and data. Libraries and fonts may load only from cdn.jsdelivr.net, |
| // probe that gave none is not asked again here. | ||
| const thinks = | ||
| thinksFirst || (deps.prepareVisuals ? await thinking(THINKING_RECHECK_BUDGET_MS, false) : false) | ||
| thinksFirst || (visuals ? await thinking(THINKING_RECHECK_BUDGET_MS, false) : false) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# Inspect the supported probe lifecycle and its timeout behavior.
ast-grep outline src/main/claude/claude-cli-flag-support.ts --items all --match 'supports|probe'
rg -n -C 8 'supports\(|startProbe|budgetMs|CLAUDE_CLI_FLAG_PROBE_KILL_AFTER_MS' src/main/claude/claude-cli-flag-support.tsRepository: stablyai/orca
Length of output: 4814
Keep the thinking recheck independent of visuals.
If the first thinking check expires while a cold CLI-version probe is still running, a launch with visuals disabled skips the bounded recheck. The launch can therefore omit readable thinking even when the probe finishes before the recheck budget ends. Keep the recheck available without starting a visuals plugin probe.
1 participant
ELI5
Every new native chat received the inline-visuals skill without a way to turn it off, and the skill directed the agent's style more than needed. Settings > Chat now has an Inline visuals switch, on by default. Turning it off stops newly started Claude and Codex chats from receiving the skill and its writable folder. Running chats and visuals already in history keep working. The skill text explains how and roughly when to show a visual, without prescribing a style.
What Changed
Why
A launch preference stops the capability at its source and respects a running chat's existing permissions. Hiding rendered replies would leave the skill and write access enabled. The row follows the desktop-only Chat settings: web clients save preferences locally but do not synchronize this setting to the execution host, so a web switch would have no effect. This setting changes the desktop machine; paired servers read their own host preference. The existing Chat-page availability rule, requiring structured native chat to be enabled, remains.
Differences from the common pattern:
Linked Issue
Follow-up to #26099, requested by Brennan. No new issue opened.
Visual Proof
Settings > Chat, ON retained after a renderer reload:
OFF retained after reload and navigation back to Chat:
Claude own-login OFF prompts and actual unset printenv result
The repeated printenv query was allowed once and returned unset; the earlier chart reply is ordinary text.
Claude own-login explicit chart: dark, light and Open in sidebar
Claude own-login implicit comparison and two visuals in one reply
Claude live ON process after OFF, then a fresh OFF process
Claude genuine Plan: expected approval to leave Plan
The provider was in Plan before the chart ask. This actual request was cancelled; it was not approved and no HTML was written.
Real Codex explicit chart: dark, light, and Open in sidebar
Real Codex implicit comparison and two visuals in one reply
The comparison chose HTML without being asked for a visual. The next two screenshots show the chart and funnel from one reply, captured by scrolling the actual conversation.
Real running-chat toggle: existing ON process and new OFF process
After the switch was turned OFF, the existing ON-launched process still rendered a chart. A new OFF process received no Orca visuals delivery and returned ordinary Mermaid; the existing Mermaid renderer reported a parse error.
Testing
Final post-merge validation passed 182 tests across 17 explicit files and 23/23
orca-ci-checks, including all three typecheck projects through the machine queue. Generated incremental typecheck state was removed first. pnpm’s unintended package-manager lockfile metadata was reversed after checks and never committed; the working tree was clean before the ordinary HTTP/1.1 push. Earlier rounds separately passed 54 settings tests, 42 post-review tests, 1908 regression tests and 12 explicit real-Claude CLI tests. Round 4 launched no app or real agent; the actual native-chat evidence below is retained from rounds 2/3.Claude Plan is accepted as expected, final validation and CI passed, and this PR is ready for review. The own-login plain-shell preflight returned
okwith exit 0 before the rig launched. Every round-3 Claude child used real HOME and the installed live-app binary, withCLAUDE_CONFIG_DIRunset; no credential copied/exported and no login repair. These successful conversations supersede the stale copied-credential round-2 401 result. Native Codex was not rerun; its round-2 results stand. Required opencode routes were tried first; unavailable/misbehaving drivers led to the permitted Codex gpt-5.6-terra medium UI-driver fallback. All UI input and screenshots used hidden Playwright/CDP, with no desktop control. Full prompts, trimmed replies, PID/session and provider-wire receipts are in the local report.monthly-signups-4b2e.html, standalone directive, rendered 120/180/150/240; light/dark text and grid adapted; Open in sidebar worked. JavaScript redraw events were not separately tested.danger-full-accessorworkspace-write; Manual permits writes. No read-only/Plan mode to test. Policy code.monthly-signups-a41f.html,signup-funnel-b82d.html), both rendered in one completed reply.monthly-signups-3f8a.htmlafter OFF; fresh PID47990 had no Orca delivery/folder/directive and returned text.Claude B4 is observed, expected: in Plan mode Claude asked for approval to leave Plan before making the chart, its normal Plan-mode flow. The driver declined; no visual HTML or directive was written. Claude’s normal plan note is listed in the local report. The real provider acknowledged Plan through Orca’s existing
agentSession.setOptionAPI before the chart ask, and the next turn’s init confirmed Plan; no native menu selector is exposed. Brennan explicitly accepted this behavior: “Yeah that plan mode behavior is fine and expected totally fine”. Codex B4 remains N/A.Final CI completed successfully for
9d6a0be765cac1fab1482b4e6aea965d9a7762c7: the latest PR rollup is 23 passed / 38 skipped / 0 failed, with no pending checks, including the workflows triggered by marking ready. The post-merge run passed static analysis/types, all ten unit-test shards, relay integration and both package builds. The ready-for-review run completed successfully with its required verification passing and its expensive jobs skipped; Linux persistence also passed. No manual rerun was needed. There are no new failures versus main. The earlier relay failures belonged to the old CI tree; main’s heartbeat-aged fixture is included by the authorized final merge.The one final main merge completed without conflicts: merge commit
9d6a0be765cac1fab1482b4e6aea965d9a7762c7, main69900f023269fd81f51b52b9357d12b4351329f7. The PR diff remains exactly its 22 intended files (473 additions, 72 deletions). Only that merge commit was pushed; no force push, second main sync, PR merge or auto-merge.Review
Claude
/code-review highreviewed the combined follow-up and the Chat-placement change. Fixed localization keys in the earlier round, then card width, shared search copy and stronger assertions in round 2. The second round-2 review returned no correctness findings. No further changes to this PR’s implementation after that review. The real Plan result is recorded above and accepted as expected; final merged-head validation passed. No PR merge or auto-merge.Agent skill upstream boundary
Notes
Live UI evidence is macOS only. Linux/Windows/paired-server visual behavior, full app quit/restart persistence and Codex comparison-filter operation are unverified. Claude Plan was selected through the existing backend API; no native menu selector was tested. Existing parser/security/remote protocol are unchanged. User directories/plugins and attachment access remain intact. English strings are covered; other locales retain their existing fallback behavior.
Round-3 before/after hashes match for personal Claude settings and Codex configuration excluding trust rows. No settings entry points into the rig. Normal Claude scratch-folder transcripts and one plan note remain in the user’s home; their exact paths are listed in the local REPORT for manual cleanup. Nothing personal was restored or deleted. The owned app was stopped using command-verified launch-recorded PIDs, profiles pruned and evidence preserved. One replacement Playwright daemon was recorded late; it was not manually signalled and exited naturally on app disconnect. This bookkeeping caveat is retained in the report.
Checklist