-
Notifications
You must be signed in to change notification settings - Fork 614
feat(provider-db): support reasoning/search objects #964
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
WalkthroughExpands model metadata schema to include structured reasoning and search capabilities, updates sanitization and presenter logic to derive flags and budgets from these objects, adjusts model configuration derivations accordingly, and updates tests to assert new fields and defaults. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Provider DB Source
participant Fetch as fetch-provider-db.mjs
participant Types as Model Types/Sanitizer
participant Presenter as configPresenter
participant Consumer as Model Config Consumer
Dev->>Fetch: Provide raw model entries (reasoning/search)
Fetch->>Types: Normalize reasoning/search
Note over Types: getReasoning / getSearch<br/>- reasoning: supported, budget.default<br/>- search: supported, forced_search, search_strategy
Types-->>Fetch: Sanitized model objects
Fetch-->>Presenter: Provider models with reasoning/search objects
Presenter->>Presenter: Map to flags<br/>enableSearch, forcedSearch,<br/>thinkingBudget, searchStrategy
Presenter-->>Consumer: Final config (incl. new fields)
Note over Consumer: Defaults applied when fields absent
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/main/presenter/configPresenter/index.ts (1)
656-658: Avoidany; type the provider model and drop castsUse ProviderModel to access fields; remove
(m as any)casts.Apply within this block:
- functionCall: Boolean((m as any).tool_call), - reasoning: Boolean((m as any).reasoning?.supported), - enableSearch: Boolean((m as any).search?.supported), + functionCall: Boolean(m.tool_call), + reasoning: Boolean(m.reasoning?.supported), + enableSearch: Boolean(m.search?.supported),Outside this range, type the map arg and import ProviderModel:
// add to imports at Line 35 import { ProviderAggregate, ProviderModel } from '@shared/types/model-db' // type the map arg in getDbProviderModels return provider.models.map((m: ProviderModel) => ({ // ... }))As per coding guidelines
src/shared/types/model-db.ts (2)
6-24: Schema additions look good; consider narrowing search strategyOptionally restrict
search_strategyto allowed values to catch typos early, e.g., z.enum(['turbo','max']). Keep current if provider may add more.
141-150: Handle booleansearchfor backward compatibilityMirror
reasoninghandling sosearch: truesanitizes to{ supported: true }.-function getSearch(obj: unknown): ProviderModel['search'] { - if (!isRecord(obj)) return undefined +function getSearch(obj: unknown): ProviderModel['search'] { + if (typeof obj === 'boolean') { + return { supported: obj } + } + if (!isRecord(obj)) return undefined const supported = getBoolean(obj, 'supported') const forced_search = getBoolean(obj, 'forced_search') const search_strategy = getString(obj, 'search_strategy') if (supported !== undefined || forced_search !== undefined || search_strategy !== undefined) { return { supported, forced_search, search_strategy } } return undefined }test/main/presenter/providerDbModelConfig.test.ts (1)
69-71: Tests align; add coverage for 'max' strategy and forced gatingConsider adding:
- A model with
search: { supported: true, search_strategy: 'max' }to assert'max'mapping.- A model with
search: { supported: false, forced_search: true }to assertforcedSearchstays false if unsupported (after gating change below).Also applies to: 94-97, 107-110, 114-117
src/main/presenter/configPresenter/modelConfig.ts (1)
296-302: GateforcedSearchbyenableSearchPrevent inconsistent state when
supportedis false butforced_searchis true.- enableSearch: Boolean(model.search?.supported), - forcedSearch: Boolean(model.search?.forced_search), + enableSearch: Boolean(model.search?.supported), + forcedSearch: Boolean(model.search?.supported && model.search?.forced_search), searchStrategy: model.search?.search_strategy === 'max' ? 'max' : 'turbo',As per coding guidelines
scripts/fetch-provider-db.mjs (1)
81-90: Also accept booleansearchfor symmetry with reasoningSupport
search: true|falseas{ supported: <bool> }for older sources.- // search (new schema) + // search (new schema; accept boolean legacy) let search const s = m.search - if (s && typeof s === 'object') { + if (typeof s === 'boolean') { + search = { supported: s } + } else if (s && typeof s === 'object') { const so = {} if (typeof s.supported === 'boolean') so.supported = s.supported if (typeof s.forced_search === 'boolean') so.forced_search = s.forced_search if (typeof s.search_strategy === 'string') so.search_strategy = s.search_strategy if (Object.keys(so).length) search = so }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
scripts/fetch-provider-db.mjs(2 hunks)src/main/presenter/configPresenter/index.ts(1 hunks)src/main/presenter/configPresenter/modelConfig.ts(1 hunks)src/shared/types/model-db.ts(4 hunks)test/main/presenter/providerDbModelConfig.test.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (17)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
**/*.{js,jsx,ts,tsx}: Use OxLint for JS/TS code; pre-commit hooks run lint-staged and typecheck
Use camelCase for variables and functions
Use PascalCase for types and classes
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/configPresenter/modelConfig.tssrc/shared/types/model-db.tssrc/main/presenter/configPresenter/index.tstest/main/presenter/providerDbModelConfig.test.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/configPresenter/index.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/configPresenter/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/configPresenter/modelConfig.tssrc/shared/types/model-db.tssrc/main/presenter/configPresenter/index.tstest/main/presenter/providerDbModelConfig.test.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/configPresenter/index.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/configPresenter/modelConfig.tssrc/shared/types/model-db.tssrc/main/presenter/configPresenter/index.tstest/main/presenter/providerDbModelConfig.test.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Files:
src/main/presenter/configPresenter/modelConfig.tssrc/shared/types/model-db.tssrc/main/presenter/configPresenter/index.tstest/main/presenter/providerDbModelConfig.test.ts
src/main/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all Electron main-process code under src/main/
Files:
src/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/configPresenter/index.ts
src/main/presenter/**
📄 CodeRabbit inference engine (AGENTS.md)
src/main/presenter/**: Organize main-process presenters under src/main/presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider)
Follow the Presenter pattern for main-process modules
Files:
src/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/configPresenter/index.ts
**/*.{js,jsx,ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting: single quotes, no semicolons, max width 100
Files:
src/main/presenter/configPresenter/modelConfig.tssrc/shared/types/model-db.tssrc/main/presenter/configPresenter/index.tstest/main/presenter/providerDbModelConfig.test.ts
scripts/**
📄 CodeRabbit inference engine (AGENTS.md)
Place build/signing/installer/runtime and commit-related scripts in scripts/
Files:
scripts/fetch-provider-db.mjs
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared目录
Files:
src/shared/types/model-db.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared types, utilities, constants, and IPC contract definitions under src/shared/
Files:
src/shared/types/model-db.ts
src/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Store shared TypeScript types/utilities in src/shared/
Files:
src/shared/types/model-db.ts
test/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit and integration tests under the test/ directory mirroring project structure
Files:
test/main/presenter/providerDbModelConfig.test.ts
test/{main,renderer}/**
📄 CodeRabbit inference engine (AGENTS.md)
Mirror source structure in tests under test/main/** and test/renderer/** (with setup files)
Files:
test/main/presenter/providerDbModelConfig.test.ts
test/{main,renderer}/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
test/{main,renderer}/**/*.{test,spec}.ts: Name test files with *.test.ts or *.spec.ts
Write tests with Vitest (jsdom) and Vue Test Utils
Files:
test/main/presenter/providerDbModelConfig.test.ts
⏰ 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: build-check (x64)
🔇 Additional comments (2)
src/shared/types/model-db.ts (2)
125-139: Reasoning parser LGTMCovers boolean/object and validates budget default nonnegative.
206-208: Switch to helper parsers LGTMConsistent use of
getReasoning/getSearch.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ 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 |
* feat(ui): new prompt input and layout wip * feat(provider-db): support reasoning/search objects (#964) * chore: sync shadcn components (#963) * chore(shadcn): sync new ui components * chore(playground): showcase new shadcn suites * feat: replace kbg and kb for ShortcutSettings * feat(ui): prompt input with new chooser --------- Co-authored-by: yyhhyyyyyy <yyhhyyyyyy8@gmail.com>
support reasoning/search objects
Summary by CodeRabbit
New Features
Tests