TUI parity pass — Phases 1–8 (ade code companion TUI)#444
Merged
Conversation
…nagement Phase 1 (robustness + streaming perf): - Coalesce streaming chat events into a per-frame batched render instead of one React render per token; lifecycle edges force-flush. Incremental/batched dedup. - ChatView: React.memo rows + split historical/live blocks so the spinner tick no longer rebuilds the whole transcript. Single shared aggregateChatBlocks. - Reconnect: JsonRpcClient.onClose -> AdeCodeConnection.onConnectionClose; detect attached-socket drop, clear streaming, reconnect (no more frozen UI). - Conflict-aware /pull and /reparent (getConflictState + /pull --continue|--abort). Phase 2 (grid + lane management): - Grid Shift+Tab reverse focus, geometry fills pane, wheel scrolls tile under cursor, per-tile status glyph. - Lane management: /lane rename|archive|unarchive|archived, delete-risk preflight, Drawer lane-color rail, per-lane drawer hotkeys (r/a/x). Verified: typecheck clean, ade-cli suite (833 tests), bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…at-in-grid - Grid tiles: neutral borders with violet for the focused tile; lane color lives only inside the tile (rail/title), not the border. Minimap focused tile violet. - Footer context bar (TokenBar): drop the green "<50%" tier (read as a glitch); on-brand violet → amber → red. - Backspace: segment coalesced input chunks so a fast "type+backspace" burst applies the delete instead of dropping it (Ink only flags a lone DEL/BS). - Grid/new-chat redesign: decouple "grid exists" (multiView) from "grid shown" (gridViewActive). Creating a new chat leaves grid view into a single chat but keeps the grid resumable; navigating to a tile re-enters it; ^g/footer adds the current chat to the grid (errors if full) or resumes it. Submit/scroll/selection routing and the grid sync effect now respect gridViewActive — fixes new-chat messages being misrouted into the focused tile. Verified: typecheck clean, ade-cli suite (841 tests, +5 backspace), bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Model picker: use canonical brand glyphs/colors (theme.provider) in the rail instead of a divergent local glyph set; de-duplicate the heading; cleaner search affordance (⌕). Search now matches model shortId + aliases (type "opus"/"sonnet"), matching the desktop picker. - Codex permission cycle: stop silently discarding a "custom" approval×sandbox combo (findIndex -1 collapsed to "default"); step deterministically into the preset list instead. Verified: typecheck clean, ade-cli suite (841 tests), bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Single source of truth for the footer's inline cells (inlineRowCellOrder): a cell appears and is focusable by keyboard + mouse only when it applies. Fast mode is now a focusable cell (was a keybind-only static label, unreachable by mouse or arrow keys); reasoning is excluded from the focus order when the model has no tiers (was a dead focus stop). Keyboard down-cycle, mouse down-cycle, and the footer hit-tests all derive from the shared order. Verified: typecheck clean, ade-cli suite (842 tests, +1 inlineRowCellOrder), bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A bright cell sweeps across the "✦ model working…" label (with a gap before repeating) — a Claude-Code-style terminal shimmer — driven by the spinner tick via a new useShimmerTick. Lands on the Phase 1 spinner-decouple so only the live tail recomputes per tick. Non-Claude turns only (Claude renders its own chrome via TerminalPane); selection/plain-text paths render the label flat (shimmerPos<0). Verified: typecheck clean, ade-cli suite (842 tests), bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…, chat mgmt Implemented by Codex per codexGoal.md on the ade/tui-parity-pass lane (uncommitted working tree at handoff; committed here as a verified checkpoint before review). - Phase 3 (remaining): Codex approval/sandbox readout, Cursor modes from runtime snapshot, plan-approval card, model-picker structural unification. - Phase 4: universal hover, wheel-to-pane, clickable links, Ctrl+K CommandPalette, lane cycling. - Phase 5: chat delete/archive/unarchive + browse/search + glyphs + /context. Verified at checkpoint: typecheck clean, ade-cli suite (846 tests), bundle builds. Co-Authored-By: Codex <noreply@openai.com>
From a 4-angle adversarial review (3/4/5 + perf): - [HIGH] `[`/`]` lane cycling no longer shadows the model picker's provider tabs / search text. - Ctrl+C now falls through to exit while the Ctrl+K palette is open. - Model picker requires `/` to search, so `s`/`f` shortcuts and `[`/`]` tab-cycling work while browsing and aren't swallowed as the first search char. - Right-pane list scroll offset clamped (no blank pane after switching to a shorter same-kind list); DETAILS_BODY_MAX_LINES exported + reused (no magic 26). - Chat link click-targets isolated into their own effect keyed on the visible rows (track scroll/stream without rebuilding the whole hit-target set per flush). - `permissions` approvals restored to the typed high-stakes path (only plan_approval /model_selection use the one-key card). - `/context` shows a clean "Claude-only" message for non-Claude instead of a raw error. - Approval approve-chip default highlight scoped to approval-namespace hovers; footer `d deny` hover is no longer a no-op; removed dead historySearch:cycleScope stub. - Tests: firstUrlInText (bare + markdown links), rightPaneScrollableRowCount. Verified: typecheck clean, ade-cli suite (850 tests, +4), bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Command palette: selected row now uses the app-wide violet selection (was a
white-vs-grey distinction that left the ▎ rail unaccented) + bold; command
glyph clarified to "/".
- Drawer: awaiting-input chats tint amber ("needs you") when not selected, so
state reads at a glance; selection still wins with violet, running stays the
spinner. Consistent with the no-green-idle-chrome palette.
Verified: typecheck clean, ade-cli suite (850 tests), bundle builds.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ight Phase 6 — PR management + colorized diffs: - Colorized, fully-scrollable /diff right pane: per-file headers (+adds/ −dels) plus hunk lines tinted by kind (add=green, del=red, hunk=violet, meta/context=dim). Removes the old 8-line-per-file cap; flows the whole diff through the existing scroll window, bounded per file at 600 lines. - New pure diffLineKind() classifier in format.ts (testable, theme-free), shared by the renderer and the scroll-row counter so they never drift. - PR mutations: /pr land (two-step confirm before an irreversible merge, method merge|squash|rebase), /pr comment, /pr approve, /pr request-changes. Registered in commands.ts with argument hints. Phase 7 — visual + motion delight: - AdeWordmark: vertical brand gradient across the ANSI-shadow rows (violet → violetDeep via theme tokens); pure render, zero idle cost. - TokenBar: eases the fill toward target and pulses the last cell near the danger threshold — gated entirely on the activity tick, so idle renders are static (no timers, no idle re-renders). All animations cheap and gated on actual activity. Typecheck clean, 424 tuiClient tests pass, bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match ChatView's file-row convention — render the diff pane's per-file +N −N counts in neutral t4 rather than green/red, keeping green confined to actual diff-body add lines (a green count on the header row reads as idle chrome). TokenBar, gradient wordmark, and PR commands reviewed clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the 12-row hollow ANSI-shadow figlet with solid filled letterforms (echoing the app icon) plus a real diagonal drop shadow for depth, shortened to 7 rows. The shadow only lands on exterior cells (flood-filled from the border) so letter counters stay open and crisp. Composited once at module load into colored runs — render stays pure, zero idle re-renders. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Backspace: Ink 5.x classifies the macOS Backspace key (\x7f) as key.delete — indistinguishable from forward-delete (both arrive as key.delete, empty input). The prompt handler's `key.delete && !key.backspace ? deleteForward : ...` branch therefore ran deleteForward on every ⌫, a no-op at end of line. Now always delete backward in both text-input paths. THE fix for "backspace does nothing". Ctrl/Cmd+K: now toggles the command palette shut (mirrors Esc) instead of being swallowed while the palette is open. Wordmark: solid uniform-violet letters (bolder, cleaner E) with a 3-step layered diagonal drop shadow (violetDeep → violetDeeper → violetDeepest, new theme tokens) that hugs only the outer rim so counters stay open. 7 rows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Model picker (ModelPickerPane.tsx renderer rewrite; layout model unchanged so all callers/nav/hit-tests stay compatible): - Kill the truncated vertical rail → horizontal category/provider TAB ROW with full labels, canonical brand glyphs/colors, active tab violet+bold+bracketed. - Aligned model list: selection rail + amber favorite star + brand glyph + name + a clear violet "● now" marker; dim local/reasoning chips; unavailable rows dim with a configure hint. - Auth pips amber-only (no green idle chrome); search bar (⌕) with violet cursor; settings footer behind a divider with a distinct violet [ Use these settings ] button + labeled value chips. Sub-provider chips when >1 route. Chat-info pane: - Section headers (PLAN/GOAL/CHATS) get a hairline rule → titled-card dividers. - Header status line shows a live context-usage TokenBar instead of bare "12% ctx". tsc clean · 424 tuiClient tests pass · bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add components/designKit.tsx — the shared visual vocabulary (SectionHeader with hairline rule, StatusDot/statusGlyph, Chip, Pill, Rail, KeyHints, Rule) so every pane composes the same look. Color discipline codified: violet accent/selection, neutral chrome, amber attention; green reserved for running spinner + success ✓ glyphs only, never chrome. Model picker now mirrors the desktop layout: a vertical ICON rail (★ Favorites, ◷ Recents, provider glyphs — icons only, no text, so nothing truncates) with a violet ▶ active indicator, beside the model list + active-category header, search spanning full width. Layout model untouched → nav/hit-tests unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…gn kit Three surfaces brought onto the shared designKit so the whole client reads as one product: - Drawer: titled hairline-rule section headers (LANES/CHATS), a colored status dot leading every lane + chat row (live ●, awaiting ◔ amber, failed ✗, idle ○, done ✓), violet selection Rail, branch refs via Chip, KeyHints footer. Removed a leftover green/amber rail BAR in the minimap (green chrome) → neutral rail + colored status glyph. - Command palette: hairline-rule header with match count, per-kind colored glyphs (/ commands, ◇ lanes, ◉ chats in their agent brand color), kind tags, violet selection rail, footer with scroll position. COMMAND_PALETTE_ROWS + props unchanged. - Approval card: SectionHeader title per kind (amber ▲ permission, red high- stakes, violet ? input / ◆ model / ◇ plan), emphasized request + dim detail, violet accept Pill + red deny chip, KeyHints footer. All hit-test IDs / props / behavior preserved. tsc clean · 424 tests · builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Prompt cursor: Option/Alt+Left/Right now move by word. macOS sends these as
meta+'b'/'f' (emacs word-move) under Option-as-Meta, which fell through to the
text-insert path and literally typed "b"/"f". Now handled as word moves.
- Model picker: removed the inline "think <reasoning>" chip from model rows;
default show-all ON (show every model, not just available); renamed the apply
button "Use these settings" → "Confirm"; settings rows get leading icons
(✦ reasoning, ◆ permission, ↯ fast, ✎ output, ↻ refresh, ↗ full settings).
- Splash-only middle: informational notices ("Model set to …", "No prompt
history", nav hints) no longer dismiss the BootHero — the new-chat surface
shows nothing but the splash until a real prompt/stream produces content
(hasConversationContent ignores `notice` blocks).
- Lane list (Drawer): dropped the per-lane box border + inset padding → flat
left-aligned full-width rows; removed the redundant second vertical bar (lane
identity now tints the name; single selection rail + status dot); aligned the
branch/meta column; "miss" → "no worktree". Per-row cadence preserved so the
drawer mouse hit-test stays correct.
tsc clean · 424 tuiClient tests · bundle builds.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lane chat - Model picker — unified single vertical flow: ↑/↓ runs the model list → the settings rows → the Confirm button (no more split focus modes). Tab and, in the model list, ←/→ switch the provider rail; ←/→ in settings cycles values. Enter on a model drops focus DOWN into the settings (reasoning first) and keeps the picker open; Enter on Confirm applies + closes. (pick → settings → Confirm.) Layout stays the desktop icon-rail filter — no grouping. - Down arrow at the last prompt row now opens the full model picker pane instead of just focusing the footer inline row; both read the same modelState so they stay in sync live. - Slash command palette: scales with terminal size — wider (max 132) and more rows on bigger screens (slashPaletteVisibleRows/ReservedRows), and ↑/↓ are now captured exclusively by the suggester when it's open (cursor/history no longer eats them). - Lane with no chats now shows a working "+ new chat" button (ChatBlock no longer early-returns "No chats in lane." without it; matches the hit-test). DEFERRED (needs live visual verification — socket closed): re-syncing the model picker's mouse-click hit-test rects to the vertical-icon-rail layout. tsc clean · 424 tuiClient tests · bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each lane is a rounded box again — the box border IS the single selection indicator (violet when selected, neutral otherwise), so the confusing second inner rail is gone (status dot + name only), the branch line stays aligned, and the boxes (vs the prior hairline separators) read as tidy cards instead of large empty gaps. Same two border rows per card, so the mouse hit-test cadence is unchanged. Dropped the now-unused hairline border-style maps. Note: opening the lanes drawer by default on start is deliberately NOT included — it surfaced an error in the chat-resume/refresh path (the active session gets resolved + hydrated, tripping an error boundary in tests). Needs live diagnosis. The right context pane already opens by default via the new-chat-preview launch. tsc clean · 424 tuiClient tests · bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ean rows Fixes the garbled/overlapping picker (esp. OpenCode's dozens of providers): - Bounded, fixed-height windowed model list (MODEL_LIST_ROWS, padded) so a long catalog scrolls inside its own region instead of shoving the rest of the right pane around. Settings footer is now sticky below it. - Replaced the wrap-everything sub-provider chip block with a compact single-row selector (active group + i/N + [ ] switch hint) — that block was the source of the chaotic wrapping. - Single-line model rows: dropped the second-line subProvider / "Configure provider…" text that was overlapping the names (the garble). Unavailable shows a dim inline "· sign in" instead. - Selection is shown by COLOR, not indentation (violet rail + violet name), so changing selection no longer reflows the row. Light, consistent indent. - Canonical provider glyphs throughout; amber auth pip (no green chrome). tsc clean · 424 tuiClient tests · bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`ade code` now launches with the full layout — lanes drawer + context pane both open. Previously opening the drawer auto-resolved/hydrated an existing chat, which tripped an error boundary (app re-mounted) and broke the splash-only rule. Fix: the new-chat-preview launch now enters true DRAFT mode (setDraftChatMode true) instead of the seeded preview. Draft cascades through the drawer selection + preview effects — they keep "new-chat" and skip the getChatHistory preview — so the center stays on the splash and no session is activated. Both panes are opened in the same block. tsc clean · 424 tuiClient tests (appPolling no longer hydrates / fires the error boundary) · bundle builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sage, searchable /help, multiline feedback - Model-picker mouse hit-test: one shared geometry source (modelPickerGeometry.ts) drives both render and click rects, fixing wrong-row selection (worse when scrolled); +17 geometry tests. - Claude PTY: ref-buffered chunk flush fixes the 500-chunk desync; keyboard scrollback + visible-region copy + "↓ N new" jump chip (TerminalScrollState.ts). - /usage pane: quota windows + session cost via aiGetStatus, reusing TokenBar (≥95% danger pulse + live reset countdown, gated on the spin tick). - /help: searchable, category-grouped command reference with violet keybind chips + fuzzy/recents (helpIndex.ts); Ctrl+K stays the command palette. - Feedback: multiline right-pane form → existing daemon feedback path (feedbackForm.ts) with auto-context capture + success ✓ fade. Typecheck clean; 311 changed-area tests pass. Claude-only PTY by design; no shells. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r + multiline paste, PTY scrollback drift, usage-pane scroll, dropped-token flush Audit of the lane (15 confirmed bugs, 7 fixed at root cause): - model-picker: settings-chip + [ Apply ] hit rects were one row high (chips Box marginTop not counted) -> chipsY = footerTop+2; geometry test corrected. - feedback: success auto-close was an unowned setTimeout that could clobber a newly-opened pane -> owned, pane-guarded, cleared on unmount + every form open; pasted multi-line/tab body was stripped -> printableMultilineInput preserves \n/\t (NUL still dropped) behind a preserveMultiline flag; +regression test. - PTY: scrolled-up content drifted down a line per write -> noteTerminalNewRows advances scrollOffset by arrived rows (clamped) so the anchor holds. - /usage: pane was unscrollable (rightPaneScrollableRowCount had no case) -> refactored to an exhaustive switch (+never guard) with usage/status cases. - streaming: reconnect dropped buffered chat tokens -> flush on effect cleanup. Deferred (low, rationale in audit): viewport-metrics churn, 1-frame stale PageUp clamp, idle quota-pulse (a self-driving idle timer would violate the idle-zero-rerender rule), 95-100% bar saturation, ghost archived multiview tile, stale hover-id after unregister. Typecheck clean; Phase 8 + audit-touched suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
@copilot review but do not make fixes |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
🚧 Files skipped from review as they are similar to previous changes (12)
📝 WalkthroughWalkthroughLarge TUI refactor and feature expansion: adds shared design-kit and theme tokens; terminal scroll state and TerminalPane viewport reporting; feedback form state + UI; help index, command categories, and command/ slash palettes; model picker geometry/layout and pane; ChatView memoization and shimmer; RightPane diff/usage/context-usage/help/feedback content; Drawer/Footer hover/status wiring; json-rpc/adeApi helpers; many tests. ChangesTUI System Enhancement: Terminal Scrolling, Feedback, Help Index, Model Picker, and Design Foundation
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
PR Review
Scope: 52 file(s), +12006 / −1989
Verdict: Minor issues
This PR is a large ade code TUI parity pass (Phases 1–8): model picker, mouse/palette, PR/diff surfaces, PTY scrollback, /usage, searchable /help, multiline feedback, and reconnect/streaming coalescing. No new dependencies or trust-boundary expansion stood out; the main gaps are incomplete right-pane scroll wiring and narrow-terminal layout behavior.
🐛 Functionality
[Medium] /usage and /status panes accept scroll input but do not scroll
File: apps/ade-cli/src/tuiClient/components/RightPane.tsx:L1055-L1073, L1485-L1492, L1534-L1536
Issue: rightPaneScrollableRowCount() treats usage and status as scrollable, and app.tsx updates rightPaneScrollOffsetRows on mouse wheel (L9377-L9380), but UsagePane and the status branch render the full body without applying scrollOffsetRows (unlike list, details, and diff, which slice with scrollOffsetRows).
Repro: Open /usage when quotaWindows.length * 3 + 4 (or a long error string) exceeds DETAILS_BODY_MAX_LINES (26), or /status with enough rows to exceed 26; scroll with the wheel — offset changes but the visible content does not move, so the session block / bottom rows stay unreachable.
Fix: Pass scrollOffsetRows into UsagePane (and the status renderer) and window the rendered rows the same way DetailsPane does, including an overflow hint when content.rows.length or the usage row estimate exceeds the viewport.
🎨 UI/UX
[Low] Launch opens the right pane even when the terminal is too narrow to show it
File: apps/ade-cli/src/tuiClient/app.tsx:L5431-L5433, L2439-L2448, L10918
Issue: launchToNewChatPreview sets setRightOpen(true) alongside the drawer, but resolveRightPaneWidth() returns 0 when columns - drawerWidth - MIN_CENTER_PANE_WIDTH < MIN_RIGHT_PANE_WIDTH (~86 cols with both side panes). rightPaneVisible is rightPaneWidth > 0, so the right pane is logically “open” but not rendered.
Fix: Either defer setRightOpen(true) until resolveRightPaneWidth(...) > 0, or show a one-line notice in the footer that the right pane is hidden until the terminal is wider (or the drawer is closed).
[Low] Model-picker setting chips can map clicks to the wrong chip when chips wrap
File: apps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerGeometry.ts:L207-L215
Issue: Setting-chip hit rects are equal-width slices on a single row; when chips wrap in ModelPickerPane, clicks can target the wrong setting:${kind} (keyboard/hover-by-id still works). The comment acknowledges coarse rects; users relying on mouse-only may toggle the wrong control.
Fix: Either measure rendered chip widths (or precompute from labels) for hit rects, or disable mouse targets for wrapped chip rows and rely on keyboard/footer focus only.
Notes
- Solid patterns worth keeping: shared
modelPickerGeometryfor render + hit-test, chat-event coalescing with lifecycle flush + disconnect flush (flushPendingChatEventson subscription teardown), explicit/pr land confirmgate, and feedback/chat delete confirmation flows. openExternalUrlrestricts tohttps?://before spawning platform openers; feedback still goes through the existing daemonfeedback.prepareDraft/submitPreparedDraftpath with a user-toggleable context footer.- Could not run CI locally in this environment; review is from diff inspection at
264a355e47aa017a2dba40c411bc97b80e160abaagainste159b2daeb5c6f8d6c3841000c103dc3b9922dbd.
Sent by Cursor Automation: BUGBOT in Versic
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerLayout.ts (1)
211-245:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize provider fallback against visible providers, not all providers.
When
showAllis off,providersPresentstill comes fromallEntries, so a provider with only unavailable models survives normalization and can produce an emptypooleven thoughvisibleEntrieshas models for another provider. That makes the picker look empty on a valid catalog.Possible fix
const visibleEntries = input.showAll ? allEntries : allEntries.filter((entry) => entry.isAvailable); + const visibleProvidersPresent = Array.from( + new Set(visibleEntries.map((entry) => entry.family)), + ); // Providers actually present in the registry-filtered model list. const providersPresent = Array.from( new Set(allEntries.map((entry) => entry.family)), ); @@ if ( !searchActive && normalizedSelection.kind === "provider" && !providersPresent.includes(normalizedSelection.provider) ) { normalizedSelection = providersPresent.length ? { kind: "provider", provider: providersPresent[0]! } : { kind: "favorites" }; } + + if ( + !searchActive + && input.showAll !== true + && normalizedSelection.kind === "provider" + && !visibleProvidersPresent.includes(normalizedSelection.provider) + ) { + normalizedSelection = visibleProvidersPresent.length + ? { kind: "provider", provider: visibleProvidersPresent[0]! } + : { kind: "favorites" }; + }🤖 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 `@apps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerLayout.ts` around lines 211 - 245, The normalization uses providersPresent derived from allEntries which ignores input.showAll, causing providers with only unavailable models to remain; change the providersPresent calculation to derive from visibleEntries (or conditionally use visibleEntries when input.showAll is false) so that providersPresent = Array.from(new Set(visibleEntries.map(e => e.family))) and ensure railEntries and the normalizedSelection logic use that updated providersPresent (refer to visibleEntries, providersPresent, allEntries, input.showAll, railEntries, normalizedSelection).apps/ade-cli/src/tuiClient/__tests__/HeaderFooter.test.tsx (1)
81-97:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the new permission detail text explicitly.
This test now supplies
permissionDetail="never · danger-full-access", but it only checksfull-auto ·. It would still pass if the detail payload disappeared entirely. Please assert the detail string itself so the new footer behavior is actually covered.✅ Suggested assertion update
expect(frame).toContain("GPT-5.5"); expect(frame).toContain("fast"); expect(frame).toContain("full-auto"); expect(frame).toContain("full-auto ·"); + expect(frame).toContain("never · danger-full-access");🤖 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 `@apps/ade-cli/src/tuiClient/__tests__/HeaderFooter.test.tsx` around lines 81 - 97, The test for FooterControls currently checks for the permissionLabel but not the actual permissionDetail; update the assertion block in HeaderFooter.test.tsx (the "renders the flat picker cells..." test that renders <FooterControls provider="codex" ... permissionDetail="never · danger-full-access" />) to also assert that the rendered frame contains the full permission detail string (e.g., add expect(frame).toContain("never · danger-full-access")), keeping the existing assertions for modelDisplay, fast indicator, and permissionLabel.
🤖 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 `@apps/ade-cli/src/tuiClient/components/AdeWordmark.tsx`:
- Around line 58-60: The faceRows function assumes all glyphs share the same
height which causes undefined rows when GLYPH_A is 7-high but GLYPH_D/GLYPH_E
are 6-high; update faceRows to compute the maximum glyph height (from GLYPH_A,
GLYPH_D, GLYPH_E), then iterate 0..maxHeight-1 and for each row use a safe
getter that returns an empty string or space-padded row when a glyph is shorter,
so `${getRow(GLYPH_A,r)}${sep}${getRow(GLYPH_D,r)}${sep}${getRow(GLYPH_E,r)}`
produces no undefined entries and buildCanvas will receive consistent widths.
Ensure the padding matches the glyph width semantics (spaces) and keep the
existing gap logic.
In `@apps/ade-cli/src/tuiClient/components/ApprovalPrompt.tsx`:
- Around line 8-12: The prompt currently forces innerWidth to
MODAL_WIDTH/INLINE_WIDTH causing overflow on narrow terminals; inside the
ApprovalPrompt component clamp innerWidth to the actual terminal/container width
before computing textWidth and before passing width into the bordered box.
Concretely: compute availableWidth (from the terminal cols or the parent
container prop), set innerWidth = Math.min(isModal ? MODAL_WIDTH : INLINE_WIDTH,
availableWidth - horizontalPadding), then derive textWidth from that clamped
innerWidth and use that value for rendering the bordered box and layout logic
(replace current uses of innerWidth and textWidth in the blocks around the
declarations and in the rendering code).
In `@apps/ade-cli/src/tuiClient/components/Drawer.tsx`:
- Line 166: The component currently slices `ordered` using
`laneStart`/`scrollOffsetRows` but continues to compare `selectedLaneIndex`
against the sliced `index`, causing selection drift; update `Drawer` so all
visible indices are treated as offsets by adding `laneStart` back when rendering
and when computing selection (i.e., compare `selectedLaneIndex` to `laneStart +
index`), pass the `laneStart` value down to `MiniDrawer` and use it there as
well, and ensure the "+ new lane" highlight/empty-state logic also uses the
absolute index (apply this change around the `ordered.slice(...)` usage and
where `index` is used for selection).
In `@apps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerGeometry.ts`:
- Around line 197-215: The coarse equal-slice hit rects are wrong for
variable-width, wrapped chips; instead, compute per-chip geometry by simulating
the footer's layout: iterate each setting in settingRows, measure its rendered
width from the label length plus chip padding (use characters -> cols), place
chips left-to-right starting at paneLeft, if x + chipWidth exceeds paneWidth
wrap to the next row by incrementing y (start from chipsY) and reset x to
paneLeft, then push a rect for each chip with id
`right:model-picker:setting:${row.kind}` and the computed x,y,w,h; update h (row
height) for wrapped rows and ensure spacing/margins between chips matches the
renderer.
In `@apps/ade-cli/src/tuiClient/components/ModelPicker/ModelPickerPane.tsx`:
- Around line 124-142: The row can wrap because nameWidth is computed as
contentWidth - 12 without accounting for the fixed prefix and suffix texts
(selection rail, favorite, brand glyph, the active " ● now" and the unavailable
" · sign in"), so update the ModelPickerPane logic to reserve extra columns for
those fixed pieces before truncating: in the component that computes nameWidth
(and uses endTruncate), compute a fixedPrefixWidth (rail + favorite + brand
glyph + their spaces) and a suffixWidth that depends on entry.isAvailable and
active (include exact spaces for " ● now" and " · sign in"), then set
nameWidth = contentWidth - prefixWidth - suffixWidth (with a sensible minimum)
and pass that to endTruncate; ensure the same adjustment is applied in the other
block referenced (lines ~267-292) so unavailable rows never overflow and wrap.
In `@apps/ade-cli/src/tuiClient/components/UsagePane.tsx`:
- Around line 62-69: The label truncation currently subtracts a fixed 8
characters for the countdown unconditionally and does the Math.max floor before
that, causing over-truncation; in the UsagePane component adjust the calculation
so you first compute the available inner width (e.g., inner = Math.max(6, width
- 2)) and then subtract the countdown reserve only when countdown is non-null
(e.g., inner - (countdown ? 8 : 0)), finally applying Math.max(6, ...) to
produce labelWidth; update usages of labelWidth (and the endTruncate call)
accordingly so labels are only shortened when a countdown is present and the
minimum floor is enforced after subtraction.
---
Outside diff comments:
In `@apps/ade-cli/src/tuiClient/__tests__/HeaderFooter.test.tsx`:
- Around line 81-97: The test for FooterControls currently checks for the
permissionLabel but not the actual permissionDetail; update the assertion block
in HeaderFooter.test.tsx (the "renders the flat picker cells..." test that
renders <FooterControls provider="codex" ... permissionDetail="never ·
danger-full-access" />) to also assert that the rendered frame contains the full
permission detail string (e.g., add expect(frame).toContain("never ·
danger-full-access")), keeping the existing assertions for modelDisplay, fast
indicator, and permissionLabel.
In `@apps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerLayout.ts`:
- Around line 211-245: The normalization uses providersPresent derived from
allEntries which ignores input.showAll, causing providers with only unavailable
models to remain; change the providersPresent calculation to derive from
visibleEntries (or conditionally use visibleEntries when input.showAll is false)
so that providersPresent = Array.from(new Set(visibleEntries.map(e =>
e.family))) and ensure railEntries and the normalizedSelection logic use that
updated providersPresent (refer to visibleEntries, providersPresent, allEntries,
input.showAll, railEntries, normalizedSelection).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c1161753-691d-4879-b4fb-d6e2c17687fc
⛔ Files ignored due to path filters (1)
codexGoal.mdis excluded by!*.md
📒 Files selected for processing (51)
.gitignoreapps/ade-cli/src/tuiClient/__tests__/HeaderFooter.test.tsxapps/ade-cli/src/tuiClient/__tests__/Palettes.test.tsxapps/ade-cli/src/tuiClient/__tests__/RightPane.test.tsxapps/ade-cli/src/tuiClient/__tests__/RightPane.usage.test.tsxapps/ade-cli/src/tuiClient/__tests__/TerminalPane.test.tsxapps/ade-cli/src/tuiClient/__tests__/TerminalScrollState.test.tsapps/ade-cli/src/tuiClient/__tests__/adeApi.test.tsapps/ade-cli/src/tuiClient/__tests__/appInput.test.tsapps/ade-cli/src/tuiClient/__tests__/commands.test.tsapps/ade-cli/src/tuiClient/__tests__/connection.test.tsapps/ade-cli/src/tuiClient/__tests__/feedbackForm.test.tsapps/ade-cli/src/tuiClient/__tests__/format.test.tsapps/ade-cli/src/tuiClient/__tests__/helpIndex.test.tsapps/ade-cli/src/tuiClient/__tests__/multiChatLayout.test.tsapps/ade-cli/src/tuiClient/__tests__/multilinePaste.test.tsapps/ade-cli/src/tuiClient/__tests__/pendingInput.test.tsapps/ade-cli/src/tuiClient/__tests__/planMode.test.tsapps/ade-cli/src/tuiClient/adeApi.tsapps/ade-cli/src/tuiClient/app.tsxapps/ade-cli/src/tuiClient/commands.tsapps/ade-cli/src/tuiClient/components/AdeWordmark.tsxapps/ade-cli/src/tuiClient/components/ApprovalPrompt.tsxapps/ade-cli/src/tuiClient/components/ChatView.tsxapps/ade-cli/src/tuiClient/components/CommandPalette.tsxapps/ade-cli/src/tuiClient/components/Drawer.tsxapps/ade-cli/src/tuiClient/components/FooterControls.tsxapps/ade-cli/src/tuiClient/components/ModelPicker/ModelPickerPane.tsxapps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerGeometry.test.tsapps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerGeometry.tsapps/ade-cli/src/tuiClient/components/ModelPicker/modelPickerLayout.tsapps/ade-cli/src/tuiClient/components/ModelPicker/types.tsapps/ade-cli/src/tuiClient/components/RightPane.tsxapps/ade-cli/src/tuiClient/components/SlashPalette.tsxapps/ade-cli/src/tuiClient/components/TerminalPane.tsxapps/ade-cli/src/tuiClient/components/TerminalScrollState.tsapps/ade-cli/src/tuiClient/components/UsagePane.tsxapps/ade-cli/src/tuiClient/components/designKit.tsxapps/ade-cli/src/tuiClient/connection.tsapps/ade-cli/src/tuiClient/feedbackForm.tsapps/ade-cli/src/tuiClient/format.tsapps/ade-cli/src/tuiClient/helpIndex.tsapps/ade-cli/src/tuiClient/hitTestRegistry.tsapps/ade-cli/src/tuiClient/jsonRpcClient.tsapps/ade-cli/src/tuiClient/keybindings/index.tsapps/ade-cli/src/tuiClient/multiChatLayout.tsapps/ade-cli/src/tuiClient/pendingInput.tsapps/ade-cli/src/tuiClient/spinTick.tsxapps/ade-cli/src/tuiClient/theme.tsapps/ade-cli/src/tuiClient/types.tsplans/tui-parity-roadmap.md
Comment on lines
+58
to
+60
| function faceRows(gap: number): string[] { | ||
| const sep = " ".repeat(gap); | ||
| return GLYPH_A.map((_, r) => `${GLYPH_A[r]}${sep}${GLYPH_D[r]}${sep}${GLYPH_E[r]}`); |
There was a problem hiding this comment.
Handle mismatched glyph heights before composing rows.
Line 60 assumes all glyphs have the same number of rows, but GLYPH_A is 7-high while GLYPH_D/GLYPH_E are 6-high. That means the last composite row includes undefined for D/E, and buildCanvas() then silently truncates the malformed width because it trusts face[0].length.
Suggested fix
function faceRows(gap: number): string[] {
const sep = " ".repeat(gap);
- return GLYPH_A.map((_, r) => `${GLYPH_A[r]}${sep}${GLYPH_D[r]}${sep}${GLYPH_E[r]}`);
+ const glyphs = [GLYPH_A, GLYPH_D, GLYPH_E] as const;
+ const height = Math.max(...glyphs.map((glyph) => glyph.length));
+ return Array.from({ length: height }, (_, r) =>
+ glyphs
+ .map((glyph) => glyph[r] ?? ".".repeat(glyph[0]!.length))
+ .join(sep),
+ );
}🤖 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 `@apps/ade-cli/src/tuiClient/components/AdeWordmark.tsx` around lines 58 - 60,
The faceRows function assumes all glyphs share the same height which causes
undefined rows when GLYPH_A is 7-high but GLYPH_D/GLYPH_E are 6-high; update
faceRows to compute the maximum glyph height (from GLYPH_A, GLYPH_D, GLYPH_E),
then iterate 0..maxHeight-1 and for each row use a safe getter that returns an
empty string or space-padded row when a glyph is shorter, so
`${getRow(GLYPH_A,r)}${sep}${getRow(GLYPH_D,r)}${sep}${getRow(GLYPH_E,r)}`
produces no undefined entries and buildCanvas will receive consistent widths.
Ensure the padding matches the glyph width semantics (spaces) and keep the
existing gap logic.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, terminal scroll guard, usage/drawer/wordmark/approval width) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…te + integrate main's serviceTiers/cursorAvailability); feedback praise -> enhancement The merge commit had mistakenly replaced the lane's model-picker architecture (buildModelPickerLayout/defaultSelectionFor/railEntrySelection + the rich ModelPickerEntry/State types consumed by app.tsx, RightPane, ModelPickerPane and both test files) with stub exports, breaking tsc, the bundle, and 10 tests. This restores the lane rewrite and folds in main's genuinely-new ModelPickerEntry fields (serviceTiers, cursorAvailability). Also corrects the feedback praise category to the valid 'enhancement' member (was an invalid 'improvement'). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…(assert write cursor/buffer, not viewport)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings the
ade codeTUI (Ink/React terminal client) to desktop-grade capability + Claude-Code-grade polish across the 8-phase parity roadmap (plans/tui-parity-roadmap.md).Phases 1–7 (earlier in lane): streaming-perf foundation + reconnect; grid + lane management + lane identity; runtime UX + unified model picker; full mouse + Ctrl/Cmd-K palette; chat management; PR commands + colorized scrollable diffs; visual/motion delight — plus a cohesive design-kit pass.
Phase 8 (this pass, trimmed to fit the TUI):
modelPickerGeometry.ts) drives both render and click rects (fixes wrong-row selection when scrolled)./usagepane — quota windows + session cost (reuses TokenBar; gated danger pulse + reset countdown)./help— searchable, category-grouped command reference with keybind chips.Audit: whole-lane audit (30 agents) found 15 confirmed bugs; the 1 high + 6 medium fixed at root cause (picker chip/Apply click rects, feedback timer + multiline-paste, PTY scrollback drift, /usage scroll, dropped-token-on-disconnect flush); low-severity items deferred with rationale.
Typecheck clean; 251 changed-area tests green. Design rules honored (violet=selection, green only for live/success/diff-add, motion gated on the spin tick).
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Greptile Summary
This PR delivers Phase 8 of the TUI parity roadmap plus several earlier-phase fixes: a shared geometry source for the model-picker mouse hit-test, PTY scrollback state with a "↓ N new" jump chip, a
/usagepane (quota windows + session cost), a searchable/helpcommand reference, and a multiline feedback form routed through the existing daemon feedback path.modelPickerGeometry.ts) unifies render and click-rect computation into one source of truth, fixing wrong-row selection when the list is scrolled.TerminalScrollState.tsintroduces a pure, allocation-light state machine for PTY scrollback that advances the anchored offset as new rows arrive and clears the pending counter on jump-to-bottom.usage, searchablehelp, multilinefeedbackform) are added with dedicated components and scrollable rendering via the sharedrightPaneScrollableRowCounthelper.Confidence Score: 5/5
Safe to merge — no functional regressions found across the geometry fix, PTY scrollback state, /usage pane, searchable /help, or feedback form path.
The geometry module is the highest-risk change and its new test suite covers windowing, rail/search modes, footer placement, and scroll-indicator shifting. The pure TerminalScrollState helpers are fully unit-tested. New right-pane content kinds are additive. The listChatSessions API shape change is intentional and tested. The only items flagged are cosmetic and none affect correctness.
No files require special attention beyond the minor style nits in RightPane.tsx.
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant app as app.tsx participant geo as modelPickerGeometry participant RightPane participant daemon Note over User,daemon: Model-picker click (fixed geometry) User->>app: mouse click on picker row app->>geo: modelPickerGeometry(paneLeft,paneTop,state) geo-->>app: entries[].rect, rail[], settings[], apply app->>daemon: action(chat, setModel, modelId) Note over User,daemon: /usage pane User->>app: /usage command app->>daemon: latestTokenStats() daemon-->>app: rateLimit, sessionTokens, cost app->>RightPane: "setRightPane(kind=usage)" RightPane->>RightPane: UsagePane renders TokenBar + countdown Note over User,daemon: /help pane User->>app: /help command app->>app: buildHelpIndex + buildHelpRows app->>RightPane: "setRightPane(kind=help, groupedRows)" RightPane->>RightPane: HelpPane renders groups + keybind chips Note over User,daemon: Feedback form submit User->>RightPane: types body, Ctrl+S RightPane->>app: onSubmitFeedback app->>app: feedbackFormToFormValues app->>daemon: feedback.prepareDraft daemon-->>app: preparedDraft app->>daemon: feedback.submitPreparedDraft app->>RightPane: "feedback=submitted"Comments Outside Diff (1)
apps/ade-cli/src/tuiClient/components/RightPane.tsx, line 941-951 (link)detailsBodyLinescan return up toDETAILS_BODY_MAX_LINES + 2itemswindowis sliced to exactlyDETAILS_BODY_MAX_LINESentries, thenunshiftandpusheach add one extra marker row. The caller renders every element, so both markers plus the full content window appear, potentially pushing the bottom↓ N moremarker past the visible viewport height.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "ship: iter2 — make TerminalPane >500-chu..." | Re-trigger Greptile