-
Notifications
You must be signed in to change notification settings - Fork 625
fix: add fallback model lookup when provider missing #1001
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
WalkthroughReworks provider/model lookup: getModel now accepts optional providerId, normalizes it, attempts provider-scoped lookup and falls back to a cross-provider search if needed. modelConfig uses a helper to build ModelConfig from a ProviderModel and applies the same provider-first then global-fallback search to populate finalConfig or return defaults. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant ConfigPresenter
participant ModelCaps as ModelCapabilities
participant Providers as ProviderMaps
Caller->>ConfigPresenter: request config (providerId?, modelId)
ConfigPresenter->>ModelCaps: getModel(modelId, providerId?)
alt providerId present
ModelCaps->>ModelCaps: normalize providerId -> pid
ModelCaps->>Providers: lookup Providers[pid][modelId]?
alt found
Providers-->>ModelCaps: ProviderModel
ModelCaps-->>ConfigPresenter: ProviderModel
else not found
ModelCaps->>Providers: scan all provider maps for modelId
alt found
Providers-->>ModelCaps: ProviderModel
ModelCaps-->>ConfigPresenter: ProviderModel
else not found
ModelCaps-->>ConfigPresenter: undefined
end
end
else no providerId
ModelCaps->>Providers: scan all provider maps for modelId
alt found
Providers-->>ModelCaps: ProviderModel
ModelCaps-->>ConfigPresenter: ProviderModel
else not found
ModelCaps-->>ConfigPresenter: undefined
end
end
alt ProviderModel returned
ConfigPresenter->>ConfigPresenter: buildConfigFromProviderModel(ProviderModel)
ConfigPresenter-->>Caller: finalConfig
else no model
ConfigPresenter-->>Caller: default finalConfig
end
note right of ModelCaps: New: provider normalization + global fallback scan
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/presenter/configPresenter/modelCapabilities.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/configPresenter/modelCapabilities.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/modelCapabilities.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/modelCapabilities.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/configPresenter/modelCapabilities.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/configPresenter/modelCapabilities.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/configPresenter/modelCapabilities.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/configPresenter/modelCapabilities.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/configPresenter/modelCapabilities.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/configPresenter/modelCapabilities.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/configPresenter/modelCapabilities.ts
🧬 Code graph analysis (1)
src/main/presenter/configPresenter/modelCapabilities.ts (1)
scripts/fetch-provider-db.mjs (3)
mid(41-41)pid(34-34)models(37-37)
⏰ 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/main/presenter/configPresenter/modelCapabilities.ts (2)
59-61: Verify that strict matching behavior is intentional.When the provider exists in the index but does not contain the requested model, the function returns
undefinedimmediately (line 60) without searching other providers. This prevents the fallback mechanism from activating in this scenario.Per the PR description: "Preserves strict matching behavior when a provider exists but does not include the requested model." Please confirm this is the intended behavior, as it means the fallback only applies when the provider itself is not found, not when the model is missing from an existing provider.
56-74: getModel fallback for missing providerId is unnecessary
All callers pass a non-emptyproviderId, so the “no provider” scenario (returning undefined at line 74) never occurs and requires no change.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/presenter/configPresenter/modelConfig.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/configPresenter/modelConfig.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.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.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.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/configPresenter/modelConfig.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/configPresenter/modelConfig.ts
🧠 Learnings (1)
📚 Learning: 2025-09-06T03:07:23.817Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T03:07:23.817Z
Learning: Applies to src/main/presenter/configPresenter/providers.ts : Add provider configuration entries in src/main/presenter/configPresenter/providers.ts
Applied to files:
src/main/presenter/configPresenter/modelConfig.ts
🧬 Code graph analysis (1)
src/main/presenter/configPresenter/modelConfig.ts (1)
src/shared/types/model-db.ts (1)
isImageInputSupported(79-82)
⏰ 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 (1)
src/main/presenter/configPresenter/modelConfig.ts (1)
315-350: Fallback logic correctly implements cross-provider search.The fallback mechanism properly iterates all providers when the specified provider doesn't exist, maintains early exit optimization, and correctly populates the model config. This aligns well with the PR objective to add fallback model lookup when provider is missing.
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: 1
🧹 Nitpick comments (5)
src/main/presenter/configPresenter/modelCapabilities.ts (2)
20-23: Deduplicate provider ID aliasesAlias map appears in multiple classes. Centralize in a shared module to avoid drift and ease updates.
56-78: Unreachable branch and overcomplicated flow in getModel; simplify and keep semanticsThe else-block has an unreachable fallback call and the hasProviderId check is unnecessary. Simplify while preserving “strict when provider exists; fallback only when provider missing”.
Apply:
- const normalizedProviderId = providerId ? providerId.toLowerCase() : '' - const hasProviderId = normalizedProviderId.length > 0 - const pid = hasProviderId ? this.resolveProviderId(normalizedProviderId) : undefined - - if (pid) { - const providerModels = this.index.get(pid) - if (providerModels) { - const providerMatch = providerModels.get(mid) - if (providerMatch) { - return providerMatch - } - return undefined - } - - return this.findModelAcrossProviders(mid) - } - - if (!hasProviderId) { - return undefined - } - - return this.findModelAcrossProviders(mid) + const normalizedProviderId = providerId ? providerId.toLowerCase() : '' + if (!normalizedProviderId) return undefined + const pid = this.resolveProviderId(normalizedProviderId) + + const providerModels = this.index.get(pid) + if (providerModels) { + // Strict: only return if the model exists under the provider + return providerModels.get(mid) + } + // Provider missing: fallback across providers + return this.findModelAcrossProviders(mid)src/main/presenter/configPresenter/modelConfig.ts (3)
306-309: Provider key lookup may be case-sensitive depending on DB normalizationresolvedProviderId is lowercased/aliased, but provider keys in DB might not be normalized. Consider a case-insensitive lookup fallback to avoid missing the provider entry.
Apply:
- const providerEntry = resolvedProviderId ? providers?.[resolvedProviderId] : undefined + let providerEntry = resolvedProviderId ? providers?.[resolvedProviderId] : undefined + if (!providerEntry && resolvedProviderId && providers) { + const matchKey = Object.keys(providers).find(k => k.toLowerCase() === resolvedProviderId) + providerEntry = matchKey ? providers[matchKey] : undefined + }Please confirm whether providerDbLoader normalizes provider IDs to lowercase; if yes, this fallback is optional.
251-253: Use English for comments per coding guidelinesReplace the Chinese comment with English to comply with project standards.
Apply:
- /** - * 获取模型配置(优先级:用户自定义 > 远端缓存/本地内置 Provider DB 严格匹配 > 默认兜底) - * 严格匹配要求 providerId 与 modelId 全等;不再做模糊匹配。 - */ + /** + * Get model config (priority: User-defined > Provider DB strict match > Default fallback) + * Strict match requires providerId and modelId to match exactly; no fuzzy matching. + */As per coding guidelines
23-26: Deduplicate provider ID aliasesSame alias mapping exists in ModelCapabilities. Extract to a shared constant to prevent inconsistencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/presenter/configPresenter/modelCapabilities.ts(1 hunks)src/main/presenter/configPresenter/modelConfig.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/configPresenter/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.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/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.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/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/configPresenter/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/configPresenter/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/configPresenter/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/configPresenter/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/configPresenter/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/configPresenter/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/configPresenter/modelCapabilities.tssrc/main/presenter/configPresenter/modelConfig.ts
🧠 Learnings (1)
📚 Learning: 2025-09-06T03:07:23.817Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T03:07:23.817Z
Learning: Applies to src/main/presenter/configPresenter/providers.ts : Add provider configuration entries in src/main/presenter/configPresenter/providers.ts
Applied to files:
src/main/presenter/configPresenter/modelConfig.ts
🧬 Code graph analysis (2)
src/main/presenter/configPresenter/modelCapabilities.ts (2)
scripts/fetch-provider-db.mjs (3)
mid(41-41)pid(34-34)models(37-37)src/shared/types/model-db.ts (1)
ProviderModel(57-57)
src/main/presenter/configPresenter/modelConfig.ts (3)
src/shared/types/model-db.ts (2)
ProviderModel(57-57)isImageInputSupported(79-82)src/shared/types/presenters/legacy.presenters.d.ts (1)
ModelConfig(132-150)src/main/presenter/configPresenter/providerDbLoader.ts (1)
providerDbLoader(235-235)
⏰ 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/main/presenter/configPresenter/modelCapabilities.ts (1)
73-75: Confirm intent: no fallback when providerId is omittedCurrent behavior returns undefined when providerId is not provided. If you intended global fallback when providerId is omitted, change the guard to:
- if (!normalizedProviderId) return this.findModelAcrossProviders(mid)
src/main/presenter/configPresenter/modelConfig.ts (1)
54-71: Good extraction: single-source-of-truth for ModelConfig mappingHelper cleanly removes duplication and aligns fields consistently.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68ee17af4b0c832ca75c23448af387fa
Summary by CodeRabbit