feat(app): multiline composer with inline model & thinking selectors - #57
feat(app): multiline composer with inline model & thinking selectors#57leduckhc wants to merge 8 commits into
Conversation
Redesign the chat composer into a taller multiline input (auto-grows to 10 lines) with a footer toolbar: model + thinking-effort selectors on the left, add/send on the right. Desktop always shows the full form; mobile keeps its one-line compact state when unfocused and expands to the desktop form on focus. The selectors reuse the existing /model and /thinking flows. The model selector renders nothing until session.meta arrives with a non-empty models list; the thinking selector is hidden when the agent reports no thinking support. Model/Thinking are removed from the desktop header overflow menu (now Rename / Quit only) and the read-only model-thinking line is dropped from the mobile session header, since both are now editable inline in the composer.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe composer now supports always-expanded, auto-growing input with footer selectors. Session metadata includes ACP modes, while Pi and ACP adapters synchronize selector state with agent protocols. Desktop menus move model and thinking controls into the composer, and worktree icons reflect pull-request presence. ChangesComposer selector migration
Adapter session metadata
Worktree icon state
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Composer
participant Selector
participant Adapter
participant AgentProtocol
User->>Composer: Open expanded composer
Composer->>Selector: Render session control
Selector->>Adapter: Send model, thinking, or mode action
Adapter->>AgentProtocol: Forward protocol command
AgentProtocol->>Adapter: Return updated session state
Adapter->>Composer: Emit session.meta
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
app/test/composer_test.dart (1)
58-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a companion test: footerActions hidden while compact.
The new test covers
alwaysExpanded: trueshowing footer actions, but there's no coverage for the mobile default (alwaysExpanded: false, unfocused) confirmingfooterActionscontent is NOT rendered until the field is focused/expanded — the other half of this new contract.testWidgets('footerActions are hidden while compact (unfocused, not alwaysExpanded)', ( tester, ) async { await tester.pumpWidget( wrap( const Composer( onSend: _noop, footerActions: [Text('MODEL'), Text('THINK')], ), ), ); expect(find.text('MODEL'), findsNothing); expect(find.text('THINK'), findsNothing); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/test/composer_test.dart` around lines 58 - 81, Add a companion widget test in the Composer test suite for the default compact state: render Composer without alwaysExpanded, leave the field unfocused, and assert the footerActions labels “MODEL” and “THINK” are not found. Keep the existing alwaysExpanded coverage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/ui/composer/composer.dart`:
- Around line 193-199: Update the footerActions loop in the composer Row so each
action is wrapped with Flexible as the direct Row child, containing the existing
right-padding and action widget. Preserve the current padding value and
_buildPlus placement while allowing long action labels to flex within narrow
widths.
---
Nitpick comments:
In `@app/test/composer_test.dart`:
- Around line 58-81: Add a companion widget test in the Composer test suite for
the default compact state: render Composer without alwaysExpanded, leave the
field unfocused, and assert the footerActions labels “MODEL” and “THINK” are not
found. Keep the existing alwaysExpanded coverage unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7de59f6e-691b-4b68-a7bf-f5264aaa271a
📒 Files selected for processing (8)
app/lib/desktop/chat/desktop_chat_pane.dartapp/lib/ui/composer/composer.dartapp/lib/ui/composer/composer_selectors.dartapp/lib/ui/session/session_screen.dartapp/test/composer_selectors_test.dartapp/test/composer_test.dartapp/test/desktop/desktop_chat_pane_menu_test.dartapp/test/desktop/desktop_chat_pane_test.dart
Wrap the footer selectors in an Expanded row of Flexible children so long model/thinking labels ellipsize instead of triggering a RenderFlex overflow on narrow (phone) widths. Add tests for the compact-hidden footer and the narrow-width overflow regression (CodeRabbit review).
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/test/composer_test.dart (1)
40-55: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover actual multiline growth, not only the
maxLinesswitch.This test verifies that focus changes
TextField.maxLinesfrom 1 to 10, but never enters multiline text or checks the bounded/scrolling behavior described by the test name. Add 10+ lines of input and verify the field remains capped and scrollable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/test/composer_test.dart` around lines 40 - 55, The test `field starts compact (1 line) and grows to 10 lines on focus` must exercise actual multiline input, not only inspect `maxLines`. After focusing the TextField, enter at least 10 newline-separated lines, then verify it remains capped at 10 lines and uses scrolling behavior, while preserving the existing pre-focus and post-focus assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/test/composer_test.dart`:
- Around line 103-124: Update the testWidgets case “long footer selector labels
ellipsize instead of overflowing” to use the actual footer selector widgets
instead of bare Text entries in Composer.footerActions. Keep the long labels and
narrow 320-width layout, and retain the exception assertion so the test
exercises the selectors’ ellipsis behavior rather than only Flexible wrapping.
---
Nitpick comments:
In `@app/test/composer_test.dart`:
- Around line 40-55: The test `field starts compact (1 line) and grows to 10
lines on focus` must exercise actual multiline input, not only inspect
`maxLines`. After focusing the TextField, enter at least 10 newline-separated
lines, then verify it remains capped at 10 lines and uses scrolling behavior,
while preserving the existing pre-focus and post-focus assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 35485d3a-7a3a-492e-aaad-0f92af28eac9
📒 Files selected for processing (2)
app/lib/ui/composer/composer.dartapp/test/composer_test.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- app/lib/ui/composer/composer.dart
Replace the bare-Text narrow-width test with one using the actual ComposerModelSelector/ComposerThinkingSelector and a long model name, asserting the label is single-line ellipsized (not just that Flexible wrapping avoids overflow). Moved to composer_selectors_test.dart where Riverpod is wired (CodeRabbit review).
|
@coderabbitai review |
✅ Action performedReview finished.
|
PR #56 switched every worktree row to the call_merge symbol. Restore the prior fork_right (branch) icon for worktrees without an open PR, keeping call_merge (accent) only where a PR is open.
session.meta (model + thinking + selectable models) stopped being emitted when pi-mirroring was removed in #26, so the app's model/thinking selectors had no data. Re-source it from pi's rpc protocol: query get_state + get_available_models at boot and emit a merged session.meta; map the composer's model/thinking actions to set_model/set_thinking_level and re-query on success. Note: ACP (0.26) exposes only session *modes*, not a model list or thinking level, so ACP agents (codex) can't feed these selectors — tracked separately.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/src/adapters/pi.ts (1)
72-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared type alias for the repeated
{provider, id, name}shape.The
{provider: string; id: string; name: string}object type is inlined three times (metaModel,metaModels, andnormalizeModel's return type). A shared alias reduces drift risk if the shape ever changes.♻️ Proposed refactor
+type MetaModel = { provider: string; id: string; name: string }; + export class PiAdapter extends EventEmitter implements AgentAdapter { ... - private metaModel: { provider: string; id: string; name: string } | null = null; + private metaModel: MetaModel | null = null; private metaThinking = ""; - private metaModels: { provider: string; id: string; name: string }[] = []; + private metaModels: MetaModel[] = [];-function normalizeModel( - m: unknown, -): { provider: string; id: string; name: string } | null { +function normalizeModel(m: unknown): MetaModel | null {Also applies to: 622-638
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/src/adapters/pi.ts` around lines 72 - 74, Define a shared type alias for the `{ provider: string; id: string; name: string }` model shape, then replace the repeated inline types used by `metaModel`, `metaModels`, and `normalizeModel`’s return type with that alias.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/src/adapters/pi.ts`:
- Around line 72-74: Define a shared type alias for the `{ provider: string; id:
string; name: string }` model shape, then replace the repeated inline types used
by `metaModel`, `metaModels`, and `normalizeModel`’s return type with that
alias.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4456c3b8-4720-4bf7-9934-36a504cf74c2
📒 Files selected for processing (2)
server/src/adapters/pi.test.tsserver/src/adapters/pi.ts
ACP agents (e.g. codex) have no model/thinking concept — only session modes (ask/code/architect). Surface them as a third composer selector: - server (acp.ts): capture newSession modes, emit them via session.meta, keep them in sync on current_mode_update, and map the 'mode' action to ACP's session/set_session_mode. - app: extend SessionMeta with a modes state; add ComposerModeSelector (hidden unless modes exist) wired into the desktop + mobile composer footers. Native pi sessions carry no modes (selector hidden); ACP sessions carry no model/thinking (those selectors hidden). Tests added on both sides.
Deduplicate the repeated {provider,id,name} shape into a shared MetaModel alias (CodeRabbit nitpick).
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
server/src/adapters/acp.test.ts (2)
124-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover agent-originated mode updates.
The assertion only tests the adapter's optimistic update after
sendAction; it does not execute Lines 238–246 inacp.ts. Send acurrent_mode_updatethroughagentRef.update(...)and verify the emitted metadata.As per coding guidelines, “Use test-driven development: write a failing test before production logic, then make it pass and refactor.” <coding_guidelines>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/src/adapters/acp.test.ts` around lines 124 - 129, Extend the mode test around adapter.sendAction and the existing session.meta assertions to cover agent-originated updates: invoke agentRef.update with a current_mode_update payload, then collect the resulting session.meta event and assert its modes.current value. Ensure this test exercises the update-handling path in acp.ts rather than only the optimistic sendAction path.Source: Coding guidelines
81-81: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a
current_mode_updatetest case here. ThesendAction("mode", ...)path is covered, but the agent-originated mode-sync branch insessionUpdatecan still regress without a case that emitscurrent_mode_updateand checks the resultingsession.meta.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/src/adapters/acp.test.ts` at line 81, Add a test case near the existing mode-related tests in acp.test.ts that emits a current_mode_update through the agent-originated sessionUpdate path, then verifies session.meta reflects the updated mode. Reuse the existing session setup and assertions where possible, and keep the sendAction("mode", ...) coverage unchanged.app/test/composer_selectors_test.dart (1)
129-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise mode selection and action dispatch.
This only verifies the initial label. Tap the pill, select
Ask, and assert thatsendSessionAction('s1', 'mode', args: {'id': 'ask'})is issued.As per coding guidelines, “Use test-driven development: write a failing test before production logic, then make it pass and refactor.” <coding_guidelines>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/test/composer_selectors_test.dart` around lines 129 - 151, Extend the `shows the current mode name from the available modes` widget test to tap the mode selector, choose `Ask`, and verify the container records or dispatches `sendSessionAction('s1', 'mode', args: {'id': 'ask'})`. Reuse the existing `_container`, `_wrap`, and session-action test setup, and keep the initial `Code` label assertion.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/ui/composer/composer_selectors.dart`:
- Around line 164-183: Make the mode-selection bottom sheet built in the
showModalBottomSheet builder scrollable by replacing or wrapping the
non-scrollable Column containing modes.available with the project’s appropriate
scrollable widget. Preserve the SheetHeader, SafeArea, ListTile selection
behavior, and Navigator.pop result while allowing all modes to remain reachable
under large lists or text scaling.
---
Nitpick comments:
In `@app/test/composer_selectors_test.dart`:
- Around line 129-151: Extend the `shows the current mode name from the
available modes` widget test to tap the mode selector, choose `Ask`, and verify
the container records or dispatches `sendSessionAction('s1', 'mode', args:
{'id': 'ask'})`. Reuse the existing `_container`, `_wrap`, and session-action
test setup, and keep the initial `Code` label assertion.
In `@server/src/adapters/acp.test.ts`:
- Around line 124-129: Extend the mode test around adapter.sendAction and the
existing session.meta assertions to cover agent-originated updates: invoke
agentRef.update with a current_mode_update payload, then collect the resulting
session.meta event and assert its modes.current value. Ensure this test
exercises the update-handling path in acp.ts rather than only the optimistic
sendAction path.
- Line 81: Add a test case near the existing mode-related tests in acp.test.ts
that emits a current_mode_update through the agent-originated sessionUpdate
path, then verifies session.meta reflects the updated mode. Reuse the existing
session setup and assertions where possible, and keep the sendAction("mode",
...) coverage unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eb727365-544c-434a-808e-25f81c94319f
📒 Files selected for processing (9)
app/lib/desktop/chat/desktop_chat_pane.dartapp/lib/store/models.dartapp/lib/ui/composer/composer_selectors.dartapp/lib/ui/session/session_screen.dartapp/test/composer_selectors_test.dartserver/src/adapters/acp.test.tsserver/src/adapters/acp.tsserver/src/adapters/pi.test.tsserver/src/adapters/pi.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- server/src/adapters/pi.test.ts
- app/lib/ui/session/session_screen.dart
- app/lib/desktop/chat/desktop_chat_pane.dart
- server/src/adapters/pi.ts
Wrap the mode bottom-sheet Column in a SingleChildScrollView so a long list of agent modes (or large text scaling) stays reachable (CodeRabbit review).
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
What & why
The desktop chat composer looked cramped and buried model/thinking controls in
an overflow menu. This redesigns the composer into a taller, multiline input
with an inline footer toolbar, so switching model / thinking effort is one click
and lives right where you type. The composer is shared across desktop and
mobile.
scrolls. Footer toolbar layout is
[model] [thinking] … [+] [send/stop](add moved next to send, hint at top-left). New
alwaysExpanded+footerActionsonComposer.unfocused (
[+] [Message…] [stop if running]), expands to the desktop form onfocus, collapses back on blur (text preserved).
[agent logo] <model name>) and thinking-effortselector (
[signal_cellular_alt] <level>) reuse the existing/modeland/thinkingflows. The model selector renders nothing untilsession.metaarrives with a non-empty models list; the thinking selector is hidden when the
agent reports no thinking support.
overflow menu (now Rename / Quit only) and dropped the read-only
model · thinkingline from the mobile session header, since both are noweditable inline in the composer.
How it was tested
cd app && flutter test— 304 passing (new coverage: selector visibility &empty-states,
alwaysExpandedfull form, 10-line growth, trimmed menus).cd app && flutter analyze— No issues found.Server untouched (no
pnpmrun needed).Not manually run on a desktop GUI (no GUI target on this VM); verified via
analyzer + widget tests.
cd server && pnpm test && pnpm typecheck(if server touched)cd app && flutter test && flutter analyze(if app touched)Checklist
CONTRIBUTING.md.Summary by CodeRabbit