Add OpenCode Support in ModelSelector #517
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @mcbodge, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for OpenCode as a new AI provider and performs a significant and commendable refactoring of the ModelSelector component. The introduction of the useProviderModels hook, transformModels helper, and DynamicModelList component greatly improves code reuse and maintainability.
My review focuses on further enhancing the robustness and reusability of these new abstractions. I've identified a few points:
- Removing
@ts-nocheckto leverage TypeScript's type safety, which would have caught a subtle issue with model properties. - Making the
transformModelsfunction more robust to handle different property names for similar concepts (hasThinkingvs.hasReasoning). - Improving the reusability of the
DynamicModelListcomponent by decoupling it from specific badge styles.
Overall, this is a high-quality contribution that significantly improves the codebase. Addressing these points will make the new components even more solid and maintainable.
| @@ -1,17 +1,201 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
This file has @ts-nocheck at the top, which disables TypeScript type checking. This is generally not recommended as it can hide potential bugs and make the code harder to maintain. For example, the issue with hasThinking vs hasReasoning in transformModels could have been caught by the type checker. I recommend removing @ts-nocheck and fixing any resulting type errors. This will improve the long-term quality and stability of this component.
| function transformModels< | ||
| T extends { | ||
| id: string; | ||
| tier?: string; | ||
| hasThinking?: boolean; | ||
| name?: string; | ||
| label?: string; | ||
| description?: string; | ||
| }, | ||
| >( | ||
| models: T[], | ||
| provider: ModelProvider, | ||
| badgeMap: Record<string, string>, | ||
| defaultHasThinking = false | ||
| ): ModelOption[] { | ||
| return models.map((model) => ({ | ||
| id: model.id, | ||
| label: model.name || model.label || model.id, | ||
| description: model.description || '', | ||
| badge: model.tier ? badgeMap[model.tier] : undefined, | ||
| provider, | ||
| hasThinking: model.hasThinking ?? defaultHasThinking, | ||
| })); | ||
| } |
There was a problem hiding this comment.
The transformModels function is a great refactoring. However, it can be made more robust. The generic type T only considers hasThinking, but ModelDefinition (the type for dynamicOpencodeModels) uses hasReasoning. To correctly handle both codexModels and dynamicOpencodeModels, you should add hasReasoning?: boolean to the type T and use it as a fallback.
function transformModels<
T extends {
id: string;
tier?: string;
hasThinking?: boolean;
hasReasoning?: boolean;
name?: string;
label?: string;
description?: string;
},
>(
models: T[],
provider: ModelProvider,
badgeMap: Record<string, string>,
defaultHasThinking = false
): ModelOption[] {
return models.map((model) => ({
id: model.id,
label: model.name || model.label || model.id,
description: model.description || '',
badge: model.tier ? badgeMap[model.tier] : undefined,
provider,
hasThinking: model.hasThinking ?? model.hasReasoning ?? defaultHasThinking,
}));
}
| : option.badge === 'Free' | ||
| ? 'border-green-500/50 text-green-600 dark:text-green-400' | ||
| : badgeColorClass | ||
| )} |
There was a problem hiding this comment.
The DynamicModelList component has a hardcoded style for the "Free" badge. This makes the component less reusable as it has specific knowledge about badge types. To improve reusability and separation of concerns, consider passing a map of badge names to CSS classes as a prop (e.g., badgeColorMap?: Record<string, string>). The parent component would then be responsible for providing the styles for specific badges.
mcbodge
changed the title
|
If u could adress pr comments before i review it |
Shironex
added
Enhancement
|
Sorry I wasn't able to address everything in time. I was actually doing it w |
3 participants
Summary
This PR adds OpenCode as a new AI provider option in the
ModelSelectorcomponent, alongside existing providers (Claude, Cursor, Codex). It also includes significant refactoring to reduce code duplication and improve maintainability.Changes
Features
Refactoring
Based on code review feedback, the following improvements were made to reduce duplication and improve maintainability:
useProviderModelsCustom HookuseEffectlogic for fetching provider modelstransformModelsHelper FunctionModelOptionformatCODEX_BADGE_MAP,OPENCODE_BADGE_MAP) instead of if/else chainsDynamicModelListComponentDEFAULT_OPENCODE_MODELConstant'opencode/big-pickle'magic string with imported constant from@automaker/typesFiles Changed
apps/ui/src/components/views/board-view/shared/model-selector.tsxScreenshots