Repository navigation
Mobile chat: remove the Tools toggle and its open-everything mode - #31188
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 14 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely identifies the main change: removing the mobile Tools toggle and its open-everything behavior. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description is complete and matches the required template. It explains the user-visible change, implementation, rationale, testing, visual proof, scope, and checklist status. The maintainer except… Full details: Description check
-
Explanation
The description is complete and matches the required template. It explains the user-visible change, implementation, rationale, testing, visual proof, scope, and checklist status. The maintainer exception explains the missing linked issue. Some checklist items remain unchecked, and the Review section is empty, but these do not make the description incomplete.
-
✨ 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
-
- Autofix · 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.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Keep the final render working while checking… · mobile-native-chat-scroll-ownership.test.tsx:399-403
mobile/src/session/mobile-native-chat-scroll-ownership.test.tsx:399-403
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the final render working while checking frame reuse.
The final
draw([user, a, b])settles the turn. The turn fold then hides the tool rows, soToolRundoes not mount androots.frame(\run:${owners[1]}`)returnsundefined`. The unsuffixed frame key is correct.Suggested fix
- draw([user, a, b]) + draw([user, a, b], { working: true })
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b2a19046-0fd9-4688-9202-bf6a6deff42a
📒 Files selected for processing: 18
mobile/src/session/MobileNativeChatDiffCard.tsxmobile/src/session/MobileNativeChatFloatingControls.tsxmobile/src/session/MobileNativeChatMessage.test.tsmobile/src/session/MobileNativeChatMessage.tool-result-availability.test.tsxmobile/src/session/MobileNativeChatMessage.tsxmobile/src/session/MobileNativeChatToolLine.tsxmobile/src/session/MobileNativeChatToolRun.identity.test.tsxmobile/src/session/MobileNativeChatToolRun.test.tsxmobile/src/session/MobileNativeChatToolRun.tsxmobile/src/session/MobileNativeChatView.tail-follow.test.tsmobile/src/session/MobileNativeChatView.tsxmobile/src/session/mobile-native-chat-composer-layout.tsmobile/src/session/mobile-native-chat-disclosure-store.tsmobile/src/session/mobile-native-chat-reading-place.test.tsxmobile/src/session/mobile-native-chat-scroll-ownership.test.tsxmobile/src/session/mobile-native-chat-tool-run-props.tsmobile/src/session/mobile-native-chat-view-styles.tsmobile/src/session/use-mobile-native-chat-row-renderer.tsx
💤 Files with no reviewable changes: 4
- mobile/src/session/mobile-native-chat-composer-layout.ts
- mobile/src/session/mobile-native-chat-tool-run-props.ts
- mobile/src/session/use-mobile-native-chat-row-renderer.tsx
- mobile/src/session/mobile-native-chat-view-styles.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
… old Tools default did
|
Problem: the phone chat had a floating "Tools · N" button over the transcript. Tapping it opened every tool run, tool line and file change in the whole chat at once, and also un-hid the steps a finished turn tucks behind its "Worked for" row. It covered the transcript and made long chats hard to read. What changes for the user: the button is gone. Each tool run, line and file change opens only when you tap it; a finished turn's steps stay tucked away until you tap its "Worked for" row; the jump-to-latest arrow is unchanged. The space the list reserved for the button is gone too. Review (full path): round 1 correctness and architecture reviews, the readiness checklist and a final mechanism comparison found no issues at any severity, so no fixes were needed. Since then the branch only merged Verified:
Not verified:
Seen on both main and the branch (not caused by this PR): after sending, the model chip shows the placeholder "Reasoning effort", and creating a new file is labelled "Edited". For Brennan's review:
|
ELI5
The phone chat had a floating "Tools · N" button in the bottom-right corner that, when tapped, opened every tool run and every tool line in the whole chat at once (and turned into "Collapse"). It took space over the transcript and made long chats unreadable. This removes the button and everything behind it. Tool runs still open one at a time when you tap them, as before.
What Changed
Before: any mobile chat with tool calls showed a "Tools · N" pill floating over the transcript (bottom-right, next to the jump-to-latest arrow). Tapping it expanded every run, every tool line and every diff card, and also re-showed rows a settled turn had folded behind its "Worked for" line.
After: no pill. Runs, lines and diff cards start closed and open only when you tap them. A settled turn's folded rows stay folded until you open the turn. The jump-to-latest arrow is unchanged.
Mechanism removed:
MobileNativeChatFloatingControls, its styles, the tool-call counter that decided whether to show it, and the extra bottom padding the list reserved so the last row could scroll clear of it.toolsExpandedstate inMobileNativeChatViewand its threading through the row renderer andMobileNativeChatMessage(including the branch that un-folded settled turns' rows when it was on).defaultExpanded/expandChildrenon the tool run, anddefaultExpandedon tool lines and diff cards: those existed only to carry the "open everything" default. They now always start closed.Why
Asked for directly: the toggle is unwanted on mobile and its supporting code should go too. Removing the default from the disclosure keys, rather than pinning it to
false, leaves one key per run/line instead of a split that no longer means anything.Plan and review path
Plan: remove the pill and everything that only existed for its open-everything mode; keep each run, line and diff card opening on its own tap. No saved data or wire changes.
Review path: full (11 production files, above the light-path limit of 5).
Linked Issue
None (maintainer request).
Visual Proof
Native phone QA —
98cab4b44946edd183b04df69485458440cfdc7cReal Codex (
gpt-6.1-sol) in a signed iPhone 17 Pro simulator client, paired normally with desktop1.4.214. The same prompt and fixture produced three real list/create/read calls and a completedhelloreply on both builds. Main isd98b8eddb41eb6c6cc5a25e10d9099bb0e023e35(footer5f42ac3ef07c); branch footer is49e931ec6a5f.On main, tapping Tools changes it to Collapse and opens the finished activity and outputs. On the branch, runs, lines and the file-edit disclosure opened only on their own taps; Worked for opened and folded the turn; jump-to-latest returned to the newest reply; the original run stayed open after scrolling away through a real reply of 60 sentences and returning. The latest-message gap above the composer is smaller, with no space reserved for the removed pill.
Additional transitions: run open, line closed, file-edit detail open / closed.
The exact second prompt, “Now delete notes.txt”, showed its working line and running tool, then replied “Deleted notes.txt”; disk verification confirmed deletion. The fixture keeps calls separate and pauses deletion for three seconds so live activity can be captured. The long reply was a third, supplementary prompt.
Coverage limits: main coverage stops at the finished-chat and global-toggle baseline; its remaining disclosure and scroll checks were not completed. All applicable branch flows passed. The file-edit body was exercised, but this Codex run showed no separate diff card with added/deleted line markers; that presentation and whether rows were actually unmounted were not directly verified. Both builds also display the model-chip placeholder “Reasoning effort” after sending, and label creation of the new file “Edited”. The floating gear belongs to the development client.
Testing
Tests that drove the toggle were removed or rewritten: tests that relied on runs starting open now tap the run header (and lines) open, as a reader would.
Mobile tests run in CI (Mobile Checks).
I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
Notes
Mobile only. No saved data or wire changes: the open/closed memory is in-memory per chat, so changing its keys needs no migration. Desktop is untouched.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)