-
Notifications
You must be signed in to change notification settings - Fork 627
feat(providerdb): add reasoning/search defaults #967
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdates provider model schemas to include optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI
participant CP as ConfigPresenter
participant S as Sanitizer (fetch-provider-db)
participant DB as Provider DB
UI->>CP: Request provider model config
CP->>S: Ask for sanitized provider models
S->>DB: Read raw provider JSON
DB-->>S: Raw models (may include reasoning.default/search.default)
S-->>CP: Sanitized ProviderModel { display_name, tool_call, reasoning?, search? }
rect rgba(200,235,255,0.25)
note right of CP: Mapping & defaults
CP->>CP: name = model.display_name
CP->>CP: functionCall = model.tool_call
CP->>CP: reasoning.supported = model.reasoning?.supported
CP->>CP: reasoning.default = model.reasoning?.default ?? false
CP->>CP: search.supported = model.search?.supported
CP->>CP: search.default = model.search?.default ?? false
CP->>UI: finalConfig (reasoning/search derive from `.default`)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)scripts/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ 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)
🔇 Additional comments (2)
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 |
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| temperature: 0.6, | ||
| vision: isImageInputSupported(model), | ||
| functionCall: model.tool_call ?? false, | ||
| reasoning: Boolean(model.reasoning?.supported), | ||
| reasoning: Boolean(model.reasoning?.default ?? false), | ||
| type: ModelType.Chat, | ||
| thinkingBudget: model.reasoning?.budget?.default ?? undefined, | ||
| enableSearch: Boolean(model.search?.supported), | ||
| enableSearch: Boolean(model.search?.default ?? false), | ||
| forcedSearch: Boolean(model.search?.forced_search), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restore fallback when reasoning/search default missing
ModelConfigHelper now initializes reasoning and enableSearch from model.reasoning?.default and model.search?.default. The provider database currently contains no default flags for these capabilities (e.g. rg -c '"default"' resources/model-db/providers.json only yields budget defaults), so both fields evaluate to false for every model even when supported is true. This regresses prior behavior where supported reasoning/search were enabled by default and effectively disables those features until the user toggles them manually. Consider falling back to the supported flag when default is undefined.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting |
add reasoning/search defaults
Summary by CodeRabbit