Skip to content

Model diag cleanup#40

Closed
shuv1337 wants to merge 46 commits intodevfrom
model-diag-cleanup
Closed

Model diag cleanup#40
shuv1337 wants to merge 46 commits intodevfrom
model-diag-cleanup

Conversation

@shuv1337
Copy link
Collaborator

@shuv1337 shuv1337 commented Nov 26, 2025

Summary by CodeRabbit

  • New Features

    • Mark models as favorites for quick access
    • Cycle through favorite models using keyboard shortcuts (forward and reverse directions)
    • Automatically tracks recently used models for improved workflow
  • Configuration

    • Added keybind options: model_cycle_favorite and model_cycle_favorite_reverse (default: disabled)
  • Style

    • Improved spacing in keybind display bar

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 26, 2025

Walkthrough

This PR introduces a favorites feature to the OpenCode TUI, enabling users to mark preferred models as favorites and cycle through them using new keybinds. The changes add a new favorite collection to local state with disk persistence, refactor DialogModel to support data-driven favorites and recent models selection, and extend keybind configuration across all SDKs.

Changes

Cohort / File(s) Summary
TUI Command & Local State
packages/opencode/src/cli/cmd/tui/app.tsx, packages/opencode/src/cli/cmd/tui/context/local.tsx
Adds two new TUI commands for cycling through favorites (forward/reverse) and introduces a comprehensive favorites/recents API in Local context with disk persistence via save(), including favorite(), cycleFavorite(direction), addRecent(), and toggleFavorite() methods.
TUI Dialog Logic
packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx
Refactors DialogModel to data-driven construction of options using query-aware flow with new toOption helper and ModelRef predicates. Adds Favorite action bound to ctrl+f to toggle favorite status of highlighted option. Restructures model listing to filter by query and exclude already-favored/recent items.
Prompt Submission
packages/opencode/src/cli/cmd/tui/component/prompt/index.tsx
Tracks currently selected model as recently used by calling local.model.addRecent() after message submission.
UI Components
packages/opencode/src/cli/cmd/tui/ui/dialog-select.tsx, packages/ui/src/components/select-dialog.tsx
Adjusts horizontal gap in keybind bar from 1 to 2. Adds optional onKeyEvent callback prop to SelectDialogProps for keyboard event handling.
Keybind Configuration
packages/opencode/src/config/config.ts, packages/web/src/content/docs/keybinds.mdx
Adds two new keybind schema properties: model_cycle_favorite and model_cycle_favorite_reverse, both optional strings with default "none".
SDK Updates
packages/sdk/go/config.go, packages/sdk/python/src/opencode_ai/models/keybinds_config.py
Propagates new ModelCycleFavorite and ModelCycleFavoriteReverse fields across Go and Python SDKs with JSON serialization support.
Formatting
packages/plugin/package.json, packages/sdk/js/package.json
Adds trailing newlines to package.json files.

Sequence Diagram

sequenceDiagram
    actor User
    participant TUI
    participant Dialog
    participant LocalModel
    participant Storage

    User->>TUI: Press ctrl+up (cycle favorite forward)
    TUI->>LocalModel: cycleFavorite(1)
    LocalModel->>LocalModel: Find next valid favorite<br/>with wrap-around
    LocalModel->>TUI: Switch to next favorite model

    Note over Dialog: Model Dialog Flow
    User->>Dialog: Open model selection dialog
    Dialog->>LocalModel: Get favorites, recents, all models
    Dialog->>Dialog: Build options (favorites,<br/>recents, all, providers)
    Dialog->>User: Display prioritized options

    User->>Dialog: Highlight option + Press ctrl+f
    Dialog->>LocalModel: toggleFavorite(model)
    LocalModel->>LocalModel: Toggle in favorites[]
    LocalModel->>Storage: save() to model.json
    Storage-->>LocalModel: Persisted

    User->>Dialog: Select model + Submit
    Dialog->>LocalModel: set(model) + addRecent(model)
    LocalModel->>LocalModel: Dedup & cap recent items
    LocalModel->>Storage: save() to model.json
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Specific areas requiring extra attention:
    • packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx: Significant refactor introducing new data flow with ModelRef comparisons, toOption helper, and restructured model listing logic. Verify filtering and deduplication logic for favorites/recents.
    • packages/opencode/src/cli/cmd/tui/context/local.tsx: New state management for favorites with persistence. Ensure isModelValid validation is correctly applied and save() is called at appropriate points. Check cycleFavorite wrap-around and toast behavior.
    • Cross-SDK consistency in keybind field naming and default values across Go, Python, and TypeScript/JS SDKs.

Possibly related PRs

  • kcrommett/opencode#30: Related refactor of TUI model/dialog code and keybind wiring in dialog-model.tsx and model selection logic.

Poem

🐰 A rabbit hops through favorites now,
With ctrl+f to mark the best,
Cycling models, circular and wow,
Persisting choices—no more quest! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Model diag cleanup' is vague and generic, using non-descriptive terms that don't convey meaningful information about the changeset's primary purpose. Clarify the title to describe the actual changes, such as 'Add model favoriting system with cycle keybinds' or 'Implement model favorite cycling and UI improvements'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch model-diag-cleanup

Comment @coderabbitai help to get the list of available commands and usage tips.

@shuv1337 shuv1337 self-assigned this Nov 26, 2025
@shuv1337 shuv1337 requested a review from Copilot November 26, 2025 00:11
@shuv1337
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Nov 26, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/opencode/src/cli/cmd/tui/context/local.tsx (1)

130-138: Fire-and-forget persistence is acceptable but consider error handling.

Bun.write returns a Promise that isn't awaited, which is fine for non-critical persistence. However, if writes fail silently, users may lose favorites unexpectedly. Consider adding a .catch() to log errors or show a toast on failure.

 function save() {
   Bun.write(
     file,
     JSON.stringify({
       recent: modelStore.recent,
       favorite: modelStore.favorite,
     }),
-  )
+  ).catch(() => {
+    // Optional: log or show toast on save failure
+  })
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 125938c and 677cbc4.

⛔ Files ignored due to path filters (1)
  • packages/sdk/js/src/gen/types.gen.ts is excluded by !**/gen/**
📒 Files selected for processing (12)
  • packages/opencode/src/cli/cmd/tui/app.tsx (1 hunks)
  • packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx (3 hunks)
  • packages/opencode/src/cli/cmd/tui/component/prompt/index.tsx (1 hunks)
  • packages/opencode/src/cli/cmd/tui/context/local.tsx (4 hunks)
  • packages/opencode/src/cli/cmd/tui/ui/dialog-select.tsx (1 hunks)
  • packages/opencode/src/config/config.ts (1 hunks)
  • packages/plugin/package.json (1 hunks)
  • packages/sdk/go/config.go (2 hunks)
  • packages/sdk/js/package.json (1 hunks)
  • packages/sdk/python/src/opencode_ai/models/keybinds_config.py (6 hunks)
  • packages/ui/src/components/select-dialog.tsx (2 hunks)
  • packages/web/src/content/docs/keybinds.mdx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,jsx,ts,tsx}: DO NOT do unnecessary destructuring of variables
DO NOT use else statements unless necessary
DO NOT use try/catch if it can be avoided
AVOID try/catch where possible
AVOID let statements
PREFER single word variable names where possible
Use as many Bun APIs as possible like Bun.file()

Files:

  • packages/opencode/src/cli/cmd/tui/component/prompt/index.tsx
  • packages/opencode/src/cli/cmd/tui/app.tsx
  • packages/opencode/src/config/config.ts
  • packages/ui/src/components/select-dialog.tsx
  • packages/opencode/src/cli/cmd/tui/ui/dialog-select.tsx
  • packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx
  • packages/opencode/src/cli/cmd/tui/context/local.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

AVOID using any type

Files:

  • packages/opencode/src/cli/cmd/tui/component/prompt/index.tsx
  • packages/opencode/src/cli/cmd/tui/app.tsx
  • packages/opencode/src/config/config.ts
  • packages/ui/src/components/select-dialog.tsx
  • packages/opencode/src/cli/cmd/tui/ui/dialog-select.tsx
  • packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx
  • packages/opencode/src/cli/cmd/tui/context/local.tsx
packages/opencode/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)

Use Bun with TypeScript ESM modules as the runtime environment

Files:

  • packages/opencode/src/cli/cmd/tui/component/prompt/index.tsx
  • packages/opencode/src/cli/cmd/tui/app.tsx
  • packages/opencode/src/config/config.ts
  • packages/opencode/src/cli/cmd/tui/ui/dialog-select.tsx
  • packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx
  • packages/opencode/src/cli/cmd/tui/context/local.tsx
packages/opencode/**/*.{ts,tsx}

📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)

packages/opencode/**/*.{ts,tsx}: Use relative imports for local modules with named imports preferred
Use Zod schemas for validation
Use TypeScript interfaces for defining data structures
Use camelCase for variable and function names
Use PascalCase for class and namespace names
Use Result patterns for error handling; avoid throwing exceptions in tools
Organize code using namespace-based structure (e.g., Tool.define(), Session.create())
Pass sessionID in tool context and use App.provide() for dependency injection
Validate all inputs with Zod schemas
Use Log.create({ service: "name" }) pattern for logging
Use Storage namespace for data persistence

Files:

  • packages/opencode/src/cli/cmd/tui/component/prompt/index.tsx
  • packages/opencode/src/cli/cmd/tui/app.tsx
  • packages/opencode/src/config/config.ts
  • packages/opencode/src/cli/cmd/tui/ui/dialog-select.tsx
  • packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx
  • packages/opencode/src/cli/cmd/tui/context/local.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-24T22:47:41.129Z
Learnt from: CR
Repo: kcrommett/opencode PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2025-11-24T22:47:41.129Z
Learning: Applies to packages/opencode/**/*.{ts,tsx} : Use `Storage` namespace for data persistence

Applied to files:

  • packages/opencode/src/cli/cmd/tui/context/local.tsx
🧬 Code graph analysis (1)
packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx (1)
packages/opencode/src/cli/cmd/tui/ui/dialog-select.tsx (1)
  • filter (165-167)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Upload results
🔇 Additional comments (21)
packages/sdk/js/package.json (1)

29-29: ✅ Formatting cleanup approved.

The addition of the trailing newline aligns with standard formatting conventions and best practices.

packages/plugin/package.json (1)

27-27: LGTM! Standard formatting.

Adding a trailing newline at the end of the file is standard practice and improves consistency.

packages/opencode/src/cli/cmd/tui/ui/dialog-select.tsx (1)

256-256: LGTM! Improved spacing for keybind bar.

The increased gap improves readability, especially with the addition of new favorite cycling keybinds in the broader feature set.

packages/web/src/content/docs/keybinds.mdx (1)

41-42: LGTM! Documentation updated consistently.

The new keybind entries follow the established pattern and are documented with sensible defaults. Setting them to "none" by default allows users to opt-in to the favorite cycling feature without keyboard shortcut conflicts.

packages/sdk/go/config.go (1)

1938-1941: LGTM! Auto-generated code follows established pattern.

The new keybind fields are properly structured with correct Go naming conventions and JSON tags. Since this file is auto-generated from the OpenAPI spec (line 1), ensure the source specification is also updated correctly.

Also applies to: 2015-2016

packages/ui/src/components/select-dialog.tsx (1)

17-17: LGTM! Good extensibility and optimization.

The addition of the optional onKeyEvent prop is a non-breaking enhancement that provides parent components with more control over keyboard interactions. The refactoring of handleKey to compute selected once and reuse it (lines 69-75) is an improvement over the previous implementation that would have recomputed it for the Enter key case.

Also applies to: 69-79

packages/opencode/src/config/config.ts (1)

431-432: LGTM! Schema additions follow established pattern.

The new keybind configurations are properly defined using Zod schema with:

  • Optional fields with sensible defaults ("none")
  • Clear descriptions
  • Consistency with existing model cycling keybinds

The default value of "none" allows users to opt-in to the feature without automatic keyboard shortcut assignments, which prevents conflicts.

packages/opencode/src/cli/cmd/tui/app.tsx (1)

251-268: local.model.cycleFavorite method is properly defined and exists.

The method is implemented in packages/opencode/src/cli/cmd/tui/context/local.tsx (lines 228-250) with the correct signature cycleFavorite(direction: 1 | -1). The implementation handles cycling through favorite models with appropriate wrapping behavior, matching the pattern used in the new commands at lines 251-268.

packages/opencode/src/cli/cmd/tui/component/prompt/index.tsx (1)

478-479: The addRecent method exists and is correctly used with the proper type signature.

Verification confirms:

  • addRecent method is defined at line 251 in packages/opencode/src/cli/cmd/tui/context/local.tsx with signature: addRecent(model: { providerID: string; modelID: string })
  • local.model.current returns the result of a createMemo that yields { providerID: string; modelID: string }
  • The call at lines 478-479 correctly passes an object matching the expected type
  • The placement after message submission is appropriate for tracking recently used models
packages/sdk/python/src/opencode_ai/models/keybinds_config.py (2)

46-47: LGTM!

New keybind attributes follow the existing pattern with appropriate docstrings and default values of "none" (disabled by default), consistent with other keybinds in the class.

Also applies to: 100-101


183-191: Serialization and deserialization correctly extended.

The to_dict and from_dict methods are properly updated to include the new model_cycle_favorite and model_cycle_favorite_reverse fields, following the established pattern for handling UNSET values.

Also applies to: 288-291, 396-399, 469-470

packages/opencode/src/cli/cmd/tui/component/dialog-model.tsx (5)

10-10: Good introduction of ModelRef type and helper predicates.

The ModelRef type and associated predicates (isMatch, isFavorite, isRecent, isCurrent) improve type safety and readability. The predicates encapsulate comparison logic cleanly.

Also applies to: 30-33


40-55: Well-structured toOption helper.

The function centralizes option creation with proper null checks for missing provider/model. Using flatMap with ?? [] downstream correctly handles the undefined return case.


61-65: Note: Recents are capped at 5 items here but 10 in local.tsx.

The recentList is sliced to 5 items for display (line 64), while addRecent in local.tsx caps storage at 10 items. This is likely intentional to show fewer items in the UI while preserving more history, but verify this is the desired behavior.


67-96: Query filtering correctly excludes favorites/recents when no query is active.

The logic at line 81 ensures favorites and recents aren't duplicated in allModels when browsing without a query, but are included when searching. This provides a good UX where search results include all matching models.


119-125: Based on my verification, I can now provide the final rewritten review comment.

The cast to ModelRef is unsafe and can receive invalid option types.

The DialogSelect component is instantiated without an explicit type parameter (defaulting to unknown), and the options() array combines incompatible option types:

  • Model options with value: ModelRef
  • Provider options with value: string (provider ID, added when !connected())

When a user presses ctrl+f on a provider option, the cast bypasses type checking. While toggleFavorite includes runtime validation via isModelValid() that prevents crashes and shows a warning, this defeats compile-time type safety.

Suggested fixes:

  1. Use a discriminated union type to distinguish model vs. provider options
  2. Add an explicit type guard in the onTrigger handler to validate option type before toggling favorite
  3. Filter provider options out of the keybind handler, or only enable the favorite keybind for model options
packages/opencode/src/cli/cmd/tui/context/local.tsx (5)

117-126: Favorite collection correctly added to store.

The favorite array is properly typed and initialized, following the same structure as recent. This aligns with the Storage namespace pattern for data persistence as per coding guidelines.


143-144: Loading handles both arrays with proper type guards.

The Array.isArray() checks ensure backward compatibility with existing model.json files that don't have the favorite field.


228-250: cycleFavorite implementation looks correct.

The method properly:

  • Filters invalid models before cycling
  • Shows informative toast when no favorites exist
  • Handles wrap-around in both directions
  • Falls back to first/last item when current model isn't in favorites

One note: cycling to a favorite also adds it to recent (via { recent: true }). Verify this is intentional UX behavior.


251-256: Clean extraction of addRecent method.

Extracting the recent-model logic into a dedicated method improves reusability. The deduplication via uniqueBy and cap at 10 items is appropriate.


271-290: toggleFavorite correctly validates and persists.

The implementation:

  • Validates the model exists before toggling
  • Properly adds/removes from favorites array
  • Persists changes immediately via save()
  • Uses batch() appropriately for atomic store updates

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@shuv1337 shuv1337 closed this Nov 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants