refactor(terminal): split panel features#74
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR splits terminal panel functionality into focused modules: panel action dispatch, model selection, scoped models, session resume, settings UI, session tree navigation, and command wiring, with tests and helpers for each feature. ChangesPanel System Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #74 +/- ##
==========================================
+ Coverage 64.20% 65.96% +1.76%
==========================================
Files 200 205 +5
Lines 17822 17828 +6
==========================================
+ Hits 11443 11761 +318
+ Misses 5299 4976 -323
- Partials 1080 1091 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@internal/terminal/panel_actions.go`:
- Around line 17-35: The switch in applyPanelSelection currently falls through
to return nil for unknown app.selectedPanelKind; change it to return an explicit
error when no case matches (e.g., construct and return fmt.Errorf("unknown panel
kind: %v", app.selectedPanelKind) or use errors.New with context) so callers can
observe the bad state; update imports (fmt or errors) and ensure functions that
previously returned nil still return errors where applicable
(applyPanelSelection signature remains the same, so add the error return at the
end of the function).
In `@internal/terminal/panel_session.go`:
- Around line 48-52: Move the "resumed session" system message so it is only
posted after the resume actually completes successfully: call
app.loadInitialMessages(ctx) first and if that returns nil then call
app.addSystemMessage("resumed session: "+value) and app.closePanel(); otherwise
return the error without emitting the success message. Update the sequence
around the existing addSystemMessage, loadInitialMessages, and closePanel calls
in the panel resume logic to ensure the success message is only logged on
success.
In `@internal/terminal/panel_settings.go`:
- Line 30: openHotkeysPanel and openChangelogPanel currently call
newSelectionPanel with panelSettings which makes applyPanelSelection treat
selections as settings and rebuild the Settings panel; update those callers to
use the correct panel kind (e.g. panelHotkeys for openHotkeysPanel and
panelChangelog for openChangelogPanel) so applyPanelSelection does not route the
selection to applySettingSelection, or alternatively adjust applyPanelSelection
to dispatch based on the panel identity created by newSelectionPanel; target
symbols: openHotkeysPanel, openChangelogPanel, newSelectionPanel,
applyPanelSelection, applySettingSelection, panelSettings (replace with
panelHotkeys/panelChangelog).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e5e0a06-7fd9-4d79-b0b7-bce2b2d4e2bb
📒 Files selected for processing (13)
internal/terminal/panel_actions.gointernal/terminal/panel_model.gointernal/terminal/panel_model_test.gointernal/terminal/panel_scoped_models.gointernal/terminal/panel_scoped_models_test.gointernal/terminal/panel_session.gointernal/terminal/panel_session_test.gointernal/terminal/panel_settings.gointernal/terminal/panel_settings_test.gointernal/terminal/panel_test_helpers_test.gointernal/terminal/panel_tree.gointernal/terminal/panel_tree_test.gointernal/terminal/panels.go
💤 Files with no reviewable changes (1)
- internal/terminal/panels.go
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
internal/terminal/auth_commands.go (1)
193-206: 💤 Low valueRedundant switch case: explicit non-auth panel handling duplicates default.
The switch case at line 201 explicitly returns
nilfor non-auth panel kinds, but the default case at line 205 also returnsnil. This creates redundancy without adding clarity or safety.Consider one of these options:
- Option 1 (simpler): Remove the explicit case for non-auth panels and rely on the default.
- Option 2 (safer): Make the default case return an error for truly unknown panel kinds, keeping the explicit case as documentation of expected non-auth panels.
♻️ Option 2 example: explicit error for unknown kinds
func (app *App) applyAuthSelection(ctx context.Context, value string) error { switch app.selectedPanelKind { case panelAuthLogin: app.closePanel() return app.loginCommand(ctx, value) case panelAuthLogout: app.closePanel() return app.logoutCommand(ctx, value) case panelModel, panelScopedModels, panelSettings, panelHotkeys, panelChangelog, panelSessions, panelTree: return nil + default: + return fmt.Errorf("applyAuthSelection called with unexpected panel kind: %q", app.selectedPanelKind) } - - return nil }🤖 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 `@internal/terminal/auth_commands.go` around lines 193 - 206, In applyAuthSelection, keep the explicit case listing known non-auth panels (panelModel, panelScopedModels, panelSettings, panelHotkeys, panelChangelog, panelSessions, panelTree) but remove the trailing unconditional return nil; instead make the switch's default branch return an error like fmt.Errorf("unknown panel kind: %v", app.selectedPanelKind) and ensure fmt is imported; this preserves documented non-auth handling while surfacing truly unexpected panel kinds.internal/terminal/panel.go (1)
13-24: 💤 Low valueInconsistent constant pattern: mix of string literals and forward-referenced command name constants.
Most
panelKindconstants use direct string literals ("model","scoped_models", etc.), butpanelHotkeysandpanelChangelogreferencehotkeysCommandNameandchangelogCommandName, which are defined later in the same const block.While Go allows forward-references within const blocks, this inconsistency makes the code harder to follow:
- The actual string values for hotkeys/changelog are obscured.
- It's unclear why only these two panel kinds need this indirection.
- The pattern suggests coupling between panel kinds and command names that may not hold for all panels (e.g.,
panelAuthLoginvs command"login").Consider using string literals consistently unless there's a documented reason to share command names across multiple contexts.
♻️ Example: consistent string literals
const ( panelModel panelKind = "model" panelScopedModels panelKind = "scoped_models" panelAuthLogin panelKind = "auth_login" panelAuthLogout panelKind = "auth_logout" panelSettings panelKind = "settings" - panelHotkeys panelKind = hotkeysCommandName - panelChangelog panelKind = changelogCommandName + panelHotkeys panelKind = "hotkeys" + panelChangelog panelKind = "changelog" panelSessions panelKind = "sessions" panelTree panelKind = "tree" - hotkeysCommandName = "hotkeys" - changelogCommandName = "changelog" )Then define command name constants separately if they're needed for the command handler map:
+const ( + hotkeysCommandName = "hotkeys" + changelogCommandName = "changelog" +)🤖 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 `@internal/terminal/panel.go` around lines 13 - 24, The panelKind constants mix direct string literals and forward-referenced command name constants; make this consistent by replacing panelHotkeys and panelChangelog with their literal string values (e.g., "hotkeys" and "changelog") or by moving hotkeysCommandName and changelogCommandName above the panelKind const block and using them everywhere; update the const block containing panelModel, panelScopedModels, panelAuthLogin, panelAuthLogout, panelSettings, panelHotkeys, panelChangelog, panelSessions, panelTree (and the hotkeysCommandName/changelogCommandName identifiers) so all panelKind values are defined consistently and clearly reference either literals or shared named constants.internal/terminal/panel_settings.go (1)
88-94: ⚡ Quick win
applySettingSelectioncan assignapp.paneldirectly (no extraopenPanelbookkeeping)
openPanelininternal/terminal/panel_actions.goonly setsapp.mode = modePanel,app.selectedPanelKind = panel.kind, andapp.panel = panel; there’s no additional state tracking thatapplySettingSelectionwould bypass.- Since
applySettingSelectionis reached via thepanelSettingsbranch ofapplyPanelSelection, the existingselectedPanelKind/mode fields remain consistent.- Optional: extract a small helper to build the settings
selectionPaneland reuse it from bothopenSettingsPanelandapplySettingSelectionto avoid duplicate construction.app.panel = newSelectionPanel( panelSettings, "Settings", "Enter cycles values; Esc returns", app.settingsItems(), false, )🤖 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 `@internal/terminal/panel_settings.go` around lines 88 - 94, The current applySettingSelection constructs a selection panel then calls openPanel, but openPanel only sets mode and panel fields, so simplify by assigning app.panel directly in applySettingSelection instead of calling openPanel; locate applySettingSelection and replace the openPanel(...) invocation with app.panel = newSelectionPanel(panelSettings, "Settings", "Enter cycles values; Esc returns", app.settingsItems(), false). Optionally extract a helper (e.g., buildSettingsPanel) that returns newSelectionPanel(...) and use it from both openSettingsPanel and applySettingSelection to remove duplication while preserving app.mode and app.selectedPanelKind assignments in openPanel/openSettingsPanel.
🤖 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 `@internal/terminal/panel_session_test.go`:
- Around line 74-88: The test currently mutates secondSession.UpdatedAt
(secondSession.UpdatedAt = firstSession.UpdatedAt.Add(time.Minute)) but
CreateSession returns an in-memory entity so that change is never persisted and
openSessionPanel re-reads from the repository; either persist the updated
timestamp before calling openSessionPanel (e.g. call the repository update
method for the session returned by CreateSession so that the repository's
ordering reflects the new UpdatedAt and then assert deterministic ordering of
app.panel.items[0].Value) or remove the no-op mutation and its unused time
import and keep the existing non-deterministic assertion, ensuring you reference
secondSession, firstSession, CreateSession and openSessionPanel when making the
change.
---
Nitpick comments:
In `@internal/terminal/auth_commands.go`:
- Around line 193-206: In applyAuthSelection, keep the explicit case listing
known non-auth panels (panelModel, panelScopedModels, panelSettings,
panelHotkeys, panelChangelog, panelSessions, panelTree) but remove the trailing
unconditional return nil; instead make the switch's default branch return an
error like fmt.Errorf("unknown panel kind: %v", app.selectedPanelKind) and
ensure fmt is imported; this preserves documented non-auth handling while
surfacing truly unexpected panel kinds.
In `@internal/terminal/panel_settings.go`:
- Around line 88-94: The current applySettingSelection constructs a selection
panel then calls openPanel, but openPanel only sets mode and panel fields, so
simplify by assigning app.panel directly in applySettingSelection instead of
calling openPanel; locate applySettingSelection and replace the openPanel(...)
invocation with app.panel = newSelectionPanel(panelSettings, "Settings", "Enter
cycles values; Esc returns", app.settingsItems(), false). Optionally extract a
helper (e.g., buildSettingsPanel) that returns newSelectionPanel(...) and use it
from both openSettingsPanel and applySettingSelection to remove duplication
while preserving app.mode and app.selectedPanelKind assignments in
openPanel/openSettingsPanel.
In `@internal/terminal/panel.go`:
- Around line 13-24: The panelKind constants mix direct string literals and
forward-referenced command name constants; make this consistent by replacing
panelHotkeys and panelChangelog with their literal string values (e.g.,
"hotkeys" and "changelog") or by moving hotkeysCommandName and
changelogCommandName above the panelKind const block and using them everywhere;
update the const block containing panelModel, panelScopedModels, panelAuthLogin,
panelAuthLogout, panelSettings, panelHotkeys, panelChangelog, panelSessions,
panelTree (and the hotkeysCommandName/changelogCommandName identifiers) so all
panelKind values are defined consistently and clearly reference either literals
or shared named constants.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba7f27c2-3a8d-48a0-97bc-ad1aaacf47e2
📒 Files selected for processing (12)
internal/terminal/auth_commands.gointernal/terminal/autocomplete.gointernal/terminal/commands.gointernal/terminal/panel.gointernal/terminal/panel_actions.gointernal/terminal/panel_model_test.gointernal/terminal/panel_scoped_models_test.gointernal/terminal/panel_session.gointernal/terminal/panel_session_test.gointernal/terminal/panel_settings.gointernal/terminal/panel_settings_test.gointernal/terminal/panel_tree_test.go
✅ Files skipped from review due to trivial changes (1)
- internal/terminal/autocomplete.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/terminal/panel_actions.go
- internal/terminal/panel_session.go
|
This was referenced Jun 1, 2026
This was referenced Jun 9, 2026
Merged
Merged
Merged
This was referenced Jul 15, 2026
Merged
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
2 participants
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.
Summary\n- split generic panel framework from feature-specific panel implementations\n- move model, session, settings, scoped model, and tree panel helpers into dedicated files\n- add focused panel tests without changing behavior\n\n## Validation\n- mise exec -- task ci\n