-
-
Notifications
You must be signed in to change notification settings - Fork 146
refactor: AI SDKのProvider Registry方式に移行 #490
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
- createAIRegistry関数を追加し、リクエストごとにProvider Registryを作成 - aiServiceConfigを廃止し、createProviderRegistryを使用 - getLanguageModel関数でregistry.languageModel()を使用してモデルを取得 - streamAiText/generateAiTextの引数をregistry + service形式に変更 - テストを新しいAPIに対応 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @tegnike's task —— View job PR Review: AI SDK Provider Registry RefactoringI've completed my review of this pull request. This refactoring introduces a more structured approach to AI provider management using the AI SDK's Provider Registry pattern. Overall, it's a solid architectural improvement with good test coverage. Review Checklist:
✅ Positives
|
WalkthroughVercel AIサービスの実装をモデルインスタンスベースからプロバイダレジストリベースへリファクタリング。新たに Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant VercelAPI as "Vercel API\n(src/pages/api/ai/vercel.ts)"
participant RegistryFactory as "createAIRegistry\n(src/lib/api-services/vercelAi.ts)"
participant Registry as "AIRegistry"
participant Resolver as "getLanguageModel\n(stream/generate entry)"
participant ProviderSDK as "Provider SDK\n(`@ai-sdk/`*)"
participant LanguageModel
Client->>VercelAPI: リクエスト (service, model, options)
VercelAPI->>RegistryFactory: createAIRegistry(service, {apiKey, baseURL, resourceName})
RegistryFactory-->>VercelAPI: registry (AIRegistry)
VercelAPI->>Resolver: getLanguageModel(registry, service, model, options)
Resolver->>Registry: lookup provider / languageModel
Registry->>ProviderSDK: initialize provider (if needed)
ProviderSDK-->>Resolver: provider instance / languageModel
Resolver-->>VercelAPI: LanguageModel
VercelAPI->>LanguageModel: streamText / generateText (calls)
LanguageModel-->>Client: streamed/generated response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting 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: 2
🤖 Fix all issues with AI agents
In `@src/lib/api-services/vercelAi.ts`:
- Around line 42-119: The switch on service (in the block that populates
providers) lacks a default branch, so adding a default case that throws a clear
runtime error (e.g., throw new Error(`Unsupported VercelAIService: ${service}`))
or otherwise returns null will prevent accidentally calling
createProviderRegistry with an empty providers object when a new VercelAIService
value is added; update the switch(service) to include this default branch
referencing the same providers variable and ensure callers of
createProviderRegistry will not receive an empty providers map.
- Around line 135-144: When options are provided but the provider lookup
((registry as unknown as Record<string, CallableFunction>)[service]) returns
undefined, add a warning log so the omission of options isn't silent;
specifically, inside the branch that checks options &&
Object.keys(options).length > 0, detect when provider is falsy and log a clear
warning (including the service name and the options payload) before falling back
to registry.languageModel(model). Update the code around the provider
lookup/return (the provider variable, service, model, registry.languageModel
cast to LanguageModel) to emit this warning via the module's logger (or
console.warn if no logger is available).
🧹 Nitpick comments (6)
src/lib/api-services/vercelAi.ts (1)
31-31:Record<string, any>の使用による型安全性の低下
AIRegistryの型定義でRecord<string, any>を使用しており、プロバイダーの型情報が失われます。現時点では動作しますが、将来的なリファクタリングでの型チェックが効かなくなるリスクがあります。src/__tests__/lib/api-services/vercelAi.test.ts (1)
93-110: createAIRegistryのテストカバレッジ
openai、custom-apiのケースはテストされていますが、他のプロバイダー(azure、anthropicなど)のテストがありません。少なくともazureはresourceNameパラメータを使用するため、テスト追加を検討してください。🧪 提案: azureのテスト追加
it('returns null for custom-api service', () => { const registry = createAIRegistry('custom-api', {}) expect(registry).toBeNull() }) + + it('creates registry for azure service with resourceName', () => { + const registry = createAIRegistry('azure', { + apiKey: 'azure-key', + resourceName: 'my-resource', + }) + expect(registry).toBeDefined() + expect(mockCreateProviderRegistry).toHaveBeenCalled() + }) })src/__tests__/pages/api/vercel.test.ts (2)
166-170: baseURLの値が異なるケースのテスト
baseURL: ''、azureではbaseURL: undefinedが期待されています。これはハンドラーの実際の動作を反映していますが、一貫性のためにハンドラー側でlocalLlmUrl || undefinedのような正規化を検討しても良いかもしれません。
77-220: custom-api(registry === null)のテストケースがありません
createAIRegistryがnullを返した場合の400エラーレスポンスをテストするケースの追加を検討してください。🧪 提案: nullレジストリのテスト追加
it('returns 400 when registry is null for custom-api', async () => { mockCreateAIRegistry.mockReturnValue(null) const res = await handler( buildRequest({ messages: [], apiKey: 'test-key', aiService: 'custom-api', model: 'custom-model', stream: true, temperature: 1, maxTokens: 10, }) ) expect(res.status).toBe(400) await expect(res.json()).resolves.toEqual({ error: 'Invalid AI service', errorCode: 'InvalidAIService', }) })src/pages/api/ai/vercel.ts (2)
158-158: デバッグログが残っています
console.log('options', options)はデバッグ用と思われます。本番環境では不要な情報がログに出力される可能性があるため、削除を検討してください。🔧 提案: デバッグログの削除
- console.log('options', options)
162-179: 型キャストの重複
aiService as VercelAIServiceがLines 114、165、175で繰り返されています。Line 114で型付き変数に代入すれば、後続のキャストは不要になります。♻️ 提案: 型キャストの統一
// Provider Registryの作成 + const service = aiService as VercelAIService - const registry = createAIRegistry(aiService as VercelAIService, { + const registry = createAIRegistry(service, { apiKey: aiApiKey, baseURL: localLlmUrl, resourceName: modifiedAzureEndpoint, })そして後続の呼び出しで:
return await streamAiText({ model: modifiedModel, registry, - service: aiService as VercelAIService, + service, messages: modifiedMessages,
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/__tests__/lib/api-services/vercelAi.test.tssrc/__tests__/pages/api/vercel.test.tssrc/lib/api-services/vercelAi.tssrc/pages/api/ai/vercel.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/__tests__/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
テストは
__tests__ディレクトリに配置
Files:
src/__tests__/pages/api/vercel.test.tssrc/__tests__/lib/api-services/vercelAi.test.ts
🧠 Learnings (1)
📚 Learning: 2025-08-05T17:30:02.776Z
Learnt from: CR
Repo: tegnike/aituber-kit PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-05T17:30:02.776Z
Learning: Node.js環境用にcanvasをモック化済み
Applied to files:
src/__tests__/pages/api/vercel.test.ts
🧬 Code graph analysis (4)
src/__tests__/pages/api/vercel.test.ts (1)
src/lib/api-services/vercelAi.ts (1)
createAIRegistry(36-122)
src/lib/api-services/vercelAi.ts (1)
src/features/constants/settings.ts (1)
VercelAIService(26-26)
src/pages/api/ai/vercel.ts (2)
src/lib/api-services/vercelAi.ts (1)
createAIRegistry(36-122)src/features/constants/settings.ts (1)
VercelAIService(26-26)
src/__tests__/lib/api-services/vercelAi.test.ts (1)
src/lib/api-services/vercelAi.ts (2)
createAIRegistry(36-122)getLanguageModel(127-147)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: claude-review
🔇 Additional comments (5)
src/lib/api-services/vercelAi.ts (1)
180-183: エラーハンドリングの改善は良い変更です
error: unknownの型付けとinstanceof Errorチェックにより、型安全なエラーメッセージ抽出が実現されています。両関数で一貫したパターンが使用されている点も良いです。src/__tests__/lib/api-services/vercelAi.test.ts (2)
112-133: getLanguageModelのテストは適切ですオプションなし・ありの両方のコードパスがテストされており、引数の検証も行われています。
135-225: streamAiTextとgenerateAiTextのテストが新しいAPIに正しく対応しています
registryとserviceパラメータへの移行が適切にテストされており、エラーケースも含めてカバレッジが維持されています。src/__tests__/pages/api/vercel.test.ts (1)
41-45: mockRegistryの構造が実装と一致しています
languageModelメソッドとプロバイダー別のメソッド(azure)を持つモックは、実際のレジストリ構造を適切に模倣しています。src/pages/api/ai/vercel.ts (1)
113-131: レジストリベースのアーキテクチャへの移行は適切です
createAIRegistryを使用した動的プロバイダー作成と、nullチェックによるcustom-apiケースの処理が明確に実装されています。エラーレスポンスも適切な形式です。
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| switch (service) { | ||
| case 'openai': | ||
| providers.openai = createOpenAI({ apiKey: params.apiKey }) | ||
| break | ||
| case 'anthropic': | ||
| providers.anthropic = createAnthropic({ | ||
| apiKey: params.apiKey, | ||
| }) as unknown as ReturnType<typeof createOpenAI> | ||
| break | ||
| case 'google': | ||
| providers.google = createGoogleGenerativeAI({ | ||
| apiKey: params.apiKey, | ||
| }) as unknown as ReturnType<typeof createOpenAI> | ||
| break | ||
| case 'azure': | ||
| providers.azure = createAzure({ | ||
| resourceName: params.resourceName, | ||
| apiKey: params.apiKey, | ||
| }) as unknown as ReturnType<typeof createOpenAI> | ||
| break | ||
| case 'xai': | ||
| providers.xai = createXai({ | ||
| apiKey: params.apiKey, | ||
| }) as unknown as ReturnType<typeof createOpenAI> | ||
| break | ||
| case 'groq': | ||
| providers.groq = createOpenAI({ | ||
| baseURL: 'https://api.groq.com/openai/v1', | ||
| apiKey: params.apiKey, | ||
| }) | ||
| break | ||
| case 'cohere': | ||
| providers.cohere = createCohere({ | ||
| apiKey: params.apiKey, | ||
| }) as unknown as ReturnType<typeof createOpenAI> | ||
| break | ||
| case 'mistralai': | ||
| providers.mistralai = createMistral({ | ||
| apiKey: params.apiKey, | ||
| }) as unknown as ReturnType<typeof createOpenAI> | ||
| break | ||
| case 'perplexity': | ||
| providers.perplexity = createOpenAI({ | ||
| baseURL: 'https://api.perplexity.ai/', | ||
| apiKey: params.apiKey, | ||
| }) | ||
| break | ||
| case 'fireworks': | ||
| providers.fireworks = createOpenAI({ | ||
| baseURL: 'https://api.fireworks.ai/inference/v1', | ||
| apiKey: params.apiKey, | ||
| }) | ||
| break | ||
| case 'deepseek': | ||
| providers.deepseek = createDeepSeek({ | ||
| apiKey: params.apiKey, | ||
| }) as unknown as ReturnType<typeof createOpenAI> | ||
| break | ||
| case 'openrouter': | ||
| providers.openrouter = createOpenRouter({ | ||
| apiKey: params.apiKey, | ||
| }) as unknown as ReturnType<typeof createOpenAI> | ||
| break | ||
| case 'lmstudio': | ||
| providers.lmstudio = createOpenAICompatible({ | ||
| name: 'lmstudio', | ||
| baseURL: params.baseURL ?? '', | ||
| }) as unknown as ReturnType<typeof createOpenAI> | ||
| break | ||
| case 'ollama': | ||
| providers.ollama = createOllama({ | ||
| baseURL: params.baseURL ?? '', | ||
| }) as unknown as ReturnType<typeof createOpenAI> | ||
| break | ||
| case 'custom-api': | ||
| // custom-apiは別途処理されるため、ここでは空 | ||
| return null | ||
| } |
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.
switch文にdefaultケースがありません
VercelAIService型に新しいサービスが追加された場合、switch文がどのケースにもマッチせず、空のprovidersオブジェクトでcreateProviderRegistryが呼ばれます。これは実行時エラーの原因になる可能性があります。
🔧 提案: defaultケースの追加
case 'custom-api':
// custom-apiは別途処理されるため、ここでは空
return null
+ default:
+ // 未知のサービスの場合はnullを返す
+ console.warn(`Unknown AI service: ${service}`)
+ return null
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| switch (service) { | |
| case 'openai': | |
| providers.openai = createOpenAI({ apiKey: params.apiKey }) | |
| break | |
| case 'anthropic': | |
| providers.anthropic = createAnthropic({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'google': | |
| providers.google = createGoogleGenerativeAI({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'azure': | |
| providers.azure = createAzure({ | |
| resourceName: params.resourceName, | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'xai': | |
| providers.xai = createXai({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'groq': | |
| providers.groq = createOpenAI({ | |
| baseURL: 'https://api.groq.com/openai/v1', | |
| apiKey: params.apiKey, | |
| }) | |
| break | |
| case 'cohere': | |
| providers.cohere = createCohere({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'mistralai': | |
| providers.mistralai = createMistral({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'perplexity': | |
| providers.perplexity = createOpenAI({ | |
| baseURL: 'https://api.perplexity.ai/', | |
| apiKey: params.apiKey, | |
| }) | |
| break | |
| case 'fireworks': | |
| providers.fireworks = createOpenAI({ | |
| baseURL: 'https://api.fireworks.ai/inference/v1', | |
| apiKey: params.apiKey, | |
| }) | |
| break | |
| case 'deepseek': | |
| providers.deepseek = createDeepSeek({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'openrouter': | |
| providers.openrouter = createOpenRouter({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'lmstudio': | |
| providers.lmstudio = createOpenAICompatible({ | |
| name: 'lmstudio', | |
| baseURL: params.baseURL ?? '', | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'ollama': | |
| providers.ollama = createOllama({ | |
| baseURL: params.baseURL ?? '', | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'custom-api': | |
| // custom-apiは別途処理されるため、ここでは空 | |
| return null | |
| } | |
| switch (service) { | |
| case 'openai': | |
| providers.openai = createOpenAI({ apiKey: params.apiKey }) | |
| break | |
| case 'anthropic': | |
| providers.anthropic = createAnthropic({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'google': | |
| providers.google = createGoogleGenerativeAI({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'azure': | |
| providers.azure = createAzure({ | |
| resourceName: params.resourceName, | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'xai': | |
| providers.xai = createXai({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'groq': | |
| providers.groq = createOpenAI({ | |
| baseURL: 'https://api.groq.com/openai/v1', | |
| apiKey: params.apiKey, | |
| }) | |
| break | |
| case 'cohere': | |
| providers.cohere = createCohere({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'mistralai': | |
| providers.mistralai = createMistral({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'perplexity': | |
| providers.perplexity = createOpenAI({ | |
| baseURL: 'https://api.perplexity.ai/', | |
| apiKey: params.apiKey, | |
| }) | |
| break | |
| case 'fireworks': | |
| providers.fireworks = createOpenAI({ | |
| baseURL: 'https://api.fireworks.ai/inference/v1', | |
| apiKey: params.apiKey, | |
| }) | |
| break | |
| case 'deepseek': | |
| providers.deepseek = createDeepSeek({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'openrouter': | |
| providers.openrouter = createOpenRouter({ | |
| apiKey: params.apiKey, | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'lmstudio': | |
| providers.lmstudio = createOpenAICompatible({ | |
| name: 'lmstudio', | |
| baseURL: params.baseURL ?? '', | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'ollama': | |
| providers.ollama = createOllama({ | |
| baseURL: params.baseURL ?? '', | |
| }) as unknown as ReturnType<typeof createOpenAI> | |
| break | |
| case 'custom-api': | |
| // custom-apiは別途処理されるため、ここでは空 | |
| return null | |
| default: | |
| // 未知のサービスの場合はnullを返す | |
| console.warn(`Unknown AI service: ${service}`) | |
| return null | |
| } |
🤖 Prompt for AI Agents
In `@src/lib/api-services/vercelAi.ts` around lines 42 - 119, The switch on
service (in the block that populates providers) lacks a default branch, so
adding a default case that throws a clear runtime error (e.g., throw new
Error(`Unsupported VercelAIService: ${service}`)) or otherwise returns null will
prevent accidentally calling createProviderRegistry with an empty providers
object when a new VercelAIService value is added; update the switch(service) to
include this default branch referencing the same providers variable and ensure
callers of createProviderRegistry will not receive an empty providers map.
| if (options && Object.keys(options).length > 0) { | ||
| // オプションがある場合(例:Google Search Grounding) | ||
| // registryから直接プロバイダーを取得してオプション付きでモデルを作成 | ||
| const provider = (registry as unknown as Record<string, CallableFunction>)[ | ||
| service | ||
| ] | ||
| if (provider) { | ||
| return provider(model, options) as LanguageModel | ||
| } | ||
| } |
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.
optionsが無視される可能性がある
optionsが提供されているがproviderが見つからない場合、Line 146のregistry.languageModel()へフォールスルーし、optionsが暗黙的に無視されます。Google Search Groundingなどの機能が動作しない可能性があります。
🔧 提案: 警告ログの追加
if (options && Object.keys(options).length > 0) {
// オプションがある場合(例:Google Search Grounding)
// registryから直接プロバイダーを取得してオプション付きでモデルを作成
const provider = (registry as unknown as Record<string, CallableFunction>)[
service
]
if (provider) {
return provider(model, options) as LanguageModel
}
+ console.warn(`Provider ${service} not found in registry, options will be ignored`)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (options && Object.keys(options).length > 0) { | |
| // オプションがある場合(例:Google Search Grounding) | |
| // registryから直接プロバイダーを取得してオプション付きでモデルを作成 | |
| const provider = (registry as unknown as Record<string, CallableFunction>)[ | |
| service | |
| ] | |
| if (provider) { | |
| return provider(model, options) as LanguageModel | |
| } | |
| } | |
| if (options && Object.keys(options).length > 0) { | |
| // オプションがある場合(例:Google Search Grounding) | |
| // registryから直接プロバイダーを取得してオプション付きでモデルを作成 | |
| const provider = (registry as unknown as Record<string, CallableFunction>)[ | |
| service | |
| ] | |
| if (provider) { | |
| return provider(model, options) as LanguageModel | |
| } | |
| console.warn(`Provider ${service} not found in registry, options will be ignored`) | |
| } |
🤖 Prompt for AI Agents
In `@src/lib/api-services/vercelAi.ts` around lines 135 - 144, When options are
provided but the provider lookup ((registry as unknown as Record<string,
CallableFunction>)[service]) returns undefined, add a warning log so the
omission of options isn't silent; specifically, inside the branch that checks
options && Object.keys(options).length > 0, detect when provider is falsy and
log a clear warning (including the service name and the options payload) before
falling back to registry.languageModel(model). Update the code around the
provider lookup/return (the provider variable, service, model,
registry.languageModel cast to LanguageModel) to emit this warning via the
module's logger (or console.warn if no logger is available).
テストで期待されていたzh→zh-TWのマッピングが 実装に欠けていたため追加 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @tegnike's task —— View job PR Review: AI SDK Provider Registry RefactoringI've completed my review of this pull request. This refactoring introduces a more structured approach to AI provider management using the AI SDK's Provider Registry pattern. Overall, it's a solid architectural improvement with good test coverage. Review Checklist:
✅ Positives
|
Summary by CodeRabbit
新機能
バグ修正
テスト
✏️ Tip: You can customize this high-level summary in your review settings.