feat: add LiteLLM as AI gateway provider#15
Open
RheagalFire wants to merge 1 commit into
Open
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
📝 WalkthroughWalkthroughThis PR introduces ChangesLiteLLM Provider Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
Reviewer's GuideAdds a new LiteLLMProvider that wraps the existing OpenAIProvider, wires it into AIModelProviderManager under the Sequence diagram for LiteLLMProvider model discovery via /v1/modelssequenceDiagram
actor Client
participant LiteLLMProvider
participant LiteLLMProxy
Client->>LiteLLMProvider: listAvailableModels(filter)
LiteLLMProvider->>LiteLLMProxy: fetch /v1/models
LiteLLMProxy-->>LiteLLMProvider: models JSON
LiteLLMProvider-->>Client: ModelInfo[]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Review Summary by QodoAdd LiteLLM as AI gateway provider with dynamic model discovery
WalkthroughsDescription• Adds LiteLLMProvider as new AI gateway supporting 100+ LLM providers • Delegates to OpenAIProvider with custom proxy baseURL configuration • Implements dynamic model discovery via proxy /v1/models endpoint • Includes 12 comprehensive unit tests covering initialization and delegation Diagramflowchart LR
A["LiteLLMProvider"] -->|"delegates to"| B["OpenAIProvider"]
A -->|"queries"| C["LiteLLM Proxy /v1/models"]
C -->|"returns"| D["Available Models"]
B -->|"routes requests"| E["100+ LLM Providers"]
A -->|"registered in"| F["AIModelProviderManager"]
File Changes1. src/core/llm/providers/implementations/LiteLLMProvider.ts
|
Code Review by Qodo
1. Unbounded model fetch
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Consider replacing the
console.logcalls ininitializeandshutdownwith the project's existing logging/telemetry mechanism so provider lifecycle events are captured consistently and are easier to control in production. - The error message in
initializetightly couples the config-based API key to a specific env var name (LITELLM_API_KEY); either accept the env var directly here or rephrase the message to reflect that the key is provided via the config object rather than requiring a specific environment variable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider replacing the `console.log` calls in `initialize` and `shutdown` with the project's existing logging/telemetry mechanism so provider lifecycle events are captured consistently and are easier to control in production.
- The error message in `initialize` tightly couples the config-based API key to a specific env var name (`LITELLM_API_KEY`); either accept the env var directly here or rephrase the message to reflect that the key is provided via the config object rather than requiring a specific environment variable.
## Individual Comments
### Comment 1
<location path="src/core/llm/providers/implementations/LiteLLMProvider.ts" line_range="96-98" />
<code_context>
+ /** @inheritdoc */
+ public defaultModelId?: string;
+
+ private delegate = new OpenAIProvider();
+ private proxyBaseURL: string = 'http://localhost:4000/v1';
+ private proxyApiKey: string = '';
+
+ constructor() {}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider guarding public methods that rely on initialization to avoid subtle failures when used before initialize() is called.
Public methods like `generateCompletion`, `generateCompletionStream`, `generateEmbeddings`, and `listAvailableModels` assume `delegate`, `proxyApiKey`, and `proxyBaseURL` have been set by `initialize()`, but they don’t verify `isInitialized`. If `initialize()` is skipped, some calls (e.g., `listAvailableModels`) will run with defaults and an empty API key. Please add a guard (e.g., throw when `!this.isInitialized`) so incorrect usage fails fast rather than producing silent, hard-to-debug behavior.
Suggested implementation:
```typescript
constructor() {}
/**
* Throws if the provider has not been initialized.
*/
private ensureInitialized(): void {
if (!this.isInitialized) {
throw new Error('LiteLLMProvider must be initialized before use. Call initialize() first.');
}
}
/**
* Initializes the provider by configuring the underlying OpenAI delegate
* with the LiteLLM proxy URL and API key.
*/
```
```typescript
public async generateCompletion(
params: ProviderGenerateCompletionParams,
): Promise<ProviderGenerateCompletionResult> {
this.ensureInitialized();
```
```typescript
public async generateCompletionStream(
params: ProviderGenerateCompletionStreamParams,
): Promise<ProviderGenerateCompletionStreamResult> {
this.ensureInitialized();
```
```typescript
public async generateEmbeddings(
params: ProviderGenerateEmbeddingsParams,
): Promise<ProviderGenerateEmbeddingsResult> {
this.ensureInitialized();
```
```typescript
public async listAvailableModels(): Promise<ProviderModelInfo[]> {
this.ensureInitialized();
```
If any of the method signatures (`generateCompletion`, `generateCompletionStream`, `generateEmbeddings`, `listAvailableModels`) differ slightly (e.g., different parameter type names or return types), adjust the SEARCH patterns to exactly match the existing code blocks and keep only the added `this.ensureInitialized();` call as the first statement in each method body. The new `ensureInitialized` helper can be reused by any future public methods that also require initialization.
</issue_to_address>
### Comment 2
<location path="src/core/llm/providers/implementations/LiteLLMProvider.ts" line_range="166-171" />
<code_context>
+ const response = await fetch(url, {
+ headers: { Authorization: `Bearer ${this.proxyApiKey}` },
+ });
+ if (!response.ok) {
+ return [];
+ }
+ const data = (await response.json()) as {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Swallowing non-OK responses and errors may make diagnosing configuration issues difficult.
With the current behavior, any non-OK status or thrown error causes `listAvailableModels` to return `[]`, making misconfigurations (bad URL, invalid key, auth failures) indistinguishable from a valid “no models” response. Consider at least logging the HTTP status / error, or exposing a richer error signal, so proxy/config issues are debuggable while still allowing callers to handle an empty list when it’s genuine.
```suggestion
const response = await fetch(url, {
headers: { Authorization: `Bearer ${this.proxyApiKey}` },
});
if (!response.ok) {
// Log non-OK responses so misconfigurations (bad URL, invalid key, auth failures, etc.)
// are visible during debugging, while still returning an empty list to callers.
console.error(
`[LiteLLMProvider] listAvailableModels: request to ${url} failed with status ${response.status} ${response.statusText}`,
);
return [];
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+96
to
+98
| private delegate = new OpenAIProvider(); | ||
| private proxyBaseURL: string = 'http://localhost:4000/v1'; | ||
| private proxyApiKey: string = ''; |
There was a problem hiding this comment.
suggestion (bug_risk): Consider guarding public methods that rely on initialization to avoid subtle failures when used before initialize() is called.
Public methods like generateCompletion, generateCompletionStream, generateEmbeddings, and listAvailableModels assume delegate, proxyApiKey, and proxyBaseURL have been set by initialize(), but they don’t verify isInitialized. If initialize() is skipped, some calls (e.g., listAvailableModels) will run with defaults and an empty API key. Please add a guard (e.g., throw when !this.isInitialized) so incorrect usage fails fast rather than producing silent, hard-to-debug behavior.
Suggested implementation:
constructor() {}
/**
* Throws if the provider has not been initialized.
*/
private ensureInitialized(): void {
if (!this.isInitialized) {
throw new Error('LiteLLMProvider must be initialized before use. Call initialize() first.');
}
}
/**
* Initializes the provider by configuring the underlying OpenAI delegate
* with the LiteLLM proxy URL and API key.
*/ public async generateCompletion(
params: ProviderGenerateCompletionParams,
): Promise<ProviderGenerateCompletionResult> {
this.ensureInitialized(); public async generateCompletionStream(
params: ProviderGenerateCompletionStreamParams,
): Promise<ProviderGenerateCompletionStreamResult> {
this.ensureInitialized(); public async generateEmbeddings(
params: ProviderGenerateEmbeddingsParams,
): Promise<ProviderGenerateEmbeddingsResult> {
this.ensureInitialized(); public async listAvailableModels(): Promise<ProviderModelInfo[]> {
this.ensureInitialized();If any of the method signatures (generateCompletion, generateCompletionStream, generateEmbeddings, listAvailableModels) differ slightly (e.g., different parameter type names or return types), adjust the SEARCH patterns to exactly match the existing code blocks and keep only the added this.ensureInitialized(); call as the first statement in each method body. The new ensureInitialized helper can be reused by any future public methods that also require initialization.
Comment on lines
+166
to
+171
| const response = await fetch(url, { | ||
| headers: { Authorization: `Bearer ${this.proxyApiKey}` }, | ||
| }); | ||
| if (!response.ok) { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Swallowing non-OK responses and errors may make diagnosing configuration issues difficult.
With the current behavior, any non-OK status or thrown error causes listAvailableModels to return [], making misconfigurations (bad URL, invalid key, auth failures) indistinguishable from a valid “no models” response. Consider at least logging the HTTP status / error, or exposing a richer error signal, so proxy/config issues are debuggable while still allowing callers to handle an empty list when it’s genuine.
Suggested change
| const response = await fetch(url, { | |
| headers: { Authorization: `Bearer ${this.proxyApiKey}` }, | |
| }); | |
| if (!response.ok) { | |
| return []; | |
| } | |
| const response = await fetch(url, { | |
| headers: { Authorization: `Bearer ${this.proxyApiKey}` }, | |
| }); | |
| if (!response.ok) { | |
| // Log non-OK responses so misconfigurations (bad URL, invalid key, auth failures, etc.) | |
| // are visible during debugging, while still returning an empty list to callers. | |
| console.error( | |
| `[LiteLLMProvider] listAvailableModels: request to ${url} failed with status ${response.status} ${response.statusText}`, | |
| ); | |
| return []; | |
| } |
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+161
to
+168
| public async listAvailableModels( | ||
| filter?: { capability?: string }, | ||
| ): Promise<ModelInfo[]> { | ||
| try { | ||
| const url = this.proxyBaseURL.replace(/\/+$/, '') + '/models'; | ||
| const response = await fetch(url, { | ||
| headers: { Authorization: `Bearer ${this.proxyApiKey}` }, | ||
| }); |
There was a problem hiding this comment.
1. Unbounded model fetch 🐞 Bug ☼ Reliability
LiteLLMProvider.listAvailableModels() calls fetch() without any timeout/AbortController, so AIModelProviderManager.initialize() and listAllAvailableModels() can hang indefinitely while waiting for model discovery.
Agent Prompt
## Issue description
`LiteLLMProvider.listAvailableModels()` uses a bare `fetch()` with no timeout/abort handling. Since `AIModelProviderManager` awaits `listAvailableModels()` during initialization and when listing all models, an unresponsive LiteLLM proxy can stall these flows indefinitely.
## Issue Context
Other providers in this codebase bound network calls with request timeouts (often via `AbortController`). LiteLLMProvider already accepts `requestTimeout` in config, but it is only passed to the OpenAI delegate and not applied to `/models` discovery.
## Fix Focus Areas
- src/core/llm/providers/implementations/LiteLLMProvider.ts[106-122]
- src/core/llm/providers/implementations/LiteLLMProvider.ts[161-190]
## Suggested fix
- Persist a `requestTimeoutMs` field on `LiteLLMProvider` during `initialize()` (default to 60000).
- In `listAvailableModels()`, use `AbortController` + `setTimeout(() => controller.abort(), requestTimeoutMs)` and pass `{ signal: controller.signal }` to `fetch`.
- Ensure the timer is cleared in `finally` to avoid leaks.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+169
to
+189
| if (!response.ok) { | ||
| return []; | ||
| } | ||
| const data = (await response.json()) as { | ||
| data?: Array<{ id: string; owned_by?: string }>; | ||
| }; | ||
| const models: ModelInfo[] = (data.data ?? []).map((m) => ({ | ||
| modelId: m.id, | ||
| providerId: 'litellm', | ||
| displayName: m.id, | ||
| capabilities: ['chat'] as string[], | ||
| supportsStreaming: true, | ||
| status: 'active' as const, | ||
| })); | ||
| if (filter?.capability) { | ||
| return models.filter((m) => m.capabilities.includes(filter.capability!)); | ||
| } | ||
| return models; | ||
| } catch { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
2. Discovery failures misroute models 🐞 Bug ≡ Correctness
LiteLLMProvider.listAvailableModels() returns [] for non-OK responses and all exceptions, preventing AIModelProviderManager from populating modelToProviderMap and causing getProviderForModel() to fall back to its '/'-prefix heuristic (e.g., routing "anthropic/..." to the Anthropic provider instead of LiteLLM).
Agent Prompt
## Issue description
`LiteLLMProvider.listAvailableModels()` silently converts both HTTP failures and runtime errors into an empty model list. This means `AIModelProviderManager` cannot map discovered model IDs to the `litellm` provider; when a modelId contains `/`, the manager may route by prefix instead (treating the first segment as a providerId), which can select the wrong provider.
## Issue Context
- Manager routing prefers `modelToProviderMap`, then falls back to splitting `modelId` on `/`.
- LiteLLM model IDs commonly include provider prefixes like `anthropic/claude-...` (tests and docs).
## Fix Focus Areas
- src/core/llm/providers/implementations/LiteLLMProvider.ts[161-190]
- src/core/llm/providers/AIModelProviderManager.ts[240-258]
## Suggested fix
- Do not fully swallow discovery failures:
- At minimum: `console.warn` (or debug-gated log) with status code / error when `/models` fails.
- Preferably: throw an Error on non-OK and on caught exceptions so `AIModelProviderManager`’s existing `catch` paths can log the providerId and context.
- To reduce misrouting when discovery is temporarily unavailable, consider returning a minimal fallback catalog containing at least `defaultModelId` (if set) so the manager can still map the primary configured model to `litellm`.
- Update/extend tests to assert the new behavior (log/throw and/or fallback model inclusion).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/llm/providers/AIModelProviderManager.ts (1)
33-44:⚠️ Potential issue | 🟡 MinorAdd
LiteLLMProviderConfigtoProviderConfigEntry.configunion
providerId: 'litellm'is wired toLiteLLMProvider, andLiteLLMProviderConfigis imported, butProviderConfigEntry.configomits it—so LiteLLM entries don’t get typed config support.Suggested fix
export interface ProviderConfigEntry { providerId: string; enabled: boolean; - config: Partial<OpenAIProviderConfig | OpenRouterProviderConfig | OllamaProviderConfig | AnthropicProviderConfig | GroqProviderConfig | TogetherProviderConfig | MistralProviderConfig | XAIProviderConfig | GeminiProviderConfig | ClaudeCodeProviderConfig | GeminiCLIProviderConfig | Record<string, any>>; + config: Partial<OpenAIProviderConfig | OpenRouterProviderConfig | OllamaProviderConfig | AnthropicProviderConfig | GroqProviderConfig | TogetherProviderConfig | MistralProviderConfig | XAIProviderConfig | GeminiProviderConfig | ClaudeCodeProviderConfig | GeminiCLIProviderConfig | LiteLLMProviderConfig | Record<string, any>>; isDefault?: boolean; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/llm/providers/AIModelProviderManager.ts` around lines 33 - 44, ProviderConfigEntry.config currently omits LiteLLMProviderConfig even though LiteLLMProvider is wired to providerId 'litellm' and LiteLLMProviderConfig is already imported; update the ProviderConfigEntry interface to include LiteLLMProviderConfig in the union for the config property so entries with providerId 'litellm' get proper typings, i.e., add LiteLLMProviderConfig alongside the other provider config types referenced in ProviderConfigEntry.config.
🧹 Nitpick comments (1)
src/core/llm/providers/__tests__/LiteLLMProvider.test.ts (1)
161-178: ⚡ Quick winAssert the request contract for model discovery, not just the parsed response.
The test verifies parsed models, but it doesn’t verify that
listAvailableModels()called the expected endpoint and auth. Add assertions thatfetchwas called with/v1/modelsand anAuthorizationheader (derived fromapiKey) so proxy-routing regressions are caught.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/llm/providers/__tests__/LiteLLMProvider.test.ts` around lines 161 - 178, The test currently checks parsed models but not the request sent; after calling provider.initialize({ apiKey: 'sk-test' }) and await provider.listAvailableModels(), add assertions on the mockFetch call (mockFetch or global fetch stub) to ensure it was invoked with the '/v1/models' URL and that the request options include an Authorization header derived from the apiKey (e.g., 'Authorization: Bearer sk-test'); use the recorded call arguments from mockFetch to assert the URL and headers so proxy-routing/auth regressions are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/llm/providers/__tests__/LiteLLMProvider.test.ts`:
- Around line 160-205: The tests stub the global fetch inside each it block
using vi.stubGlobal but call vi.unstubAllGlobals only inside the happy path,
which can leak fetch if an assertion fails; refactor the
describe('listAvailableModels') suite to move vi.stubGlobal usage into each test
as-is but add an afterEach block that calls vi.unstubAllGlobals() (or
vi.restoreAllMocks()/appropriate Vitest cleanup) to ensure fetch is always
restored; reference the test suite, the provider.initialize(...) and
provider.listAvailableModels() usages to locate where stubbing happens and add
the afterEach cleanup for all cases.
In `@src/core/llm/providers/implementations/LiteLLMProvider.ts`:
- Around line 161-168: listAvailableModels() performs a direct fetch to the
proxy /models without honoring the configured requestTimeout, so a stalled proxy
can hang discovery; update listAvailableModels (in LiteLLMProvider) to create an
AbortController, start a timer using this.requestTimeout that calls
controller.abort(), pass controller.signal to fetch (using this.proxyBaseURL and
this.proxyApiKey as before), and ensure the timeout timer is cleared in a
finally block so it is always cleaned up even on errors; mirror the
timeout-handling pattern used in initialize() or other methods that forward
requestTimeout to the delegate.
---
Outside diff comments:
In `@src/core/llm/providers/AIModelProviderManager.ts`:
- Around line 33-44: ProviderConfigEntry.config currently omits
LiteLLMProviderConfig even though LiteLLMProvider is wired to providerId
'litellm' and LiteLLMProviderConfig is already imported; update the
ProviderConfigEntry interface to include LiteLLMProviderConfig in the union for
the config property so entries with providerId 'litellm' get proper typings,
i.e., add LiteLLMProviderConfig alongside the other provider config types
referenced in ProviderConfigEntry.config.
---
Nitpick comments:
In `@src/core/llm/providers/__tests__/LiteLLMProvider.test.ts`:
- Around line 161-178: The test currently checks parsed models but not the
request sent; after calling provider.initialize({ apiKey: 'sk-test' }) and await
provider.listAvailableModels(), add assertions on the mockFetch call (mockFetch
or global fetch stub) to ensure it was invoked with the '/v1/models' URL and
that the request options include an Authorization header derived from the apiKey
(e.g., 'Authorization: Bearer sk-test'); use the recorded call arguments from
mockFetch to assert the URL and headers so proxy-routing/auth regressions are
caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 38ac08b5-c708-434f-8443-1eb4577dbf8d
📒 Files selected for processing (3)
src/core/llm/providers/AIModelProviderManager.tssrc/core/llm/providers/__tests__/LiteLLMProvider.test.tssrc/core/llm/providers/implementations/LiteLLMProvider.ts
Comment on lines
+160
to
+205
| describe('listAvailableModels', () => { | ||
| it('fetches models from proxy /v1/models', async () => { | ||
| const mockFetch = vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: async () => ({ | ||
| data: [ | ||
| { id: 'gpt-4o-mini' }, | ||
| { id: 'anthropic/claude-sonnet-4-6' }, | ||
| ], | ||
| }), | ||
| }); | ||
| vi.stubGlobal('fetch', mockFetch); | ||
|
|
||
| await provider.initialize({ apiKey: 'sk-test' }); | ||
| const models = await provider.listAvailableModels(); | ||
|
|
||
| expect(models).toHaveLength(2); | ||
| expect(models[0].modelId).toBe('gpt-4o-mini'); | ||
| expect(models[1].modelId).toBe('anthropic/claude-sonnet-4-6'); | ||
|
|
||
| vi.unstubAllGlobals(); | ||
| }); | ||
|
|
||
| it('returns empty array on fetch error', async () => { | ||
| const mockFetch = vi.fn().mockResolvedValue({ ok: false }); | ||
| vi.stubGlobal('fetch', mockFetch); | ||
|
|
||
| await provider.initialize({ apiKey: 'sk-test' }); | ||
| const models = await provider.listAvailableModels(); | ||
|
|
||
| expect(models).toEqual([]); | ||
|
|
||
| vi.unstubAllGlobals(); | ||
| }); | ||
|
|
||
| it('returns empty array on network failure', async () => { | ||
| const mockFetch = vi.fn().mockRejectedValue(new Error('ECONNREFUSED')); | ||
| vi.stubGlobal('fetch', mockFetch); | ||
|
|
||
| await provider.initialize({ apiKey: 'sk-test' }); | ||
| const models = await provider.listAvailableModels(); | ||
|
|
||
| expect(models).toEqual([]); | ||
|
|
||
| vi.unstubAllGlobals(); | ||
| }); |
There was a problem hiding this comment.
Ensure global fetch cleanup runs even when assertions fail.
In Line 180, Line 192, and Line 204, vi.unstubAllGlobals() is only reached on the happy path. If an assertion throws earlier, fetch can leak into later tests and create order-dependent failures. Move cleanup to an afterEach for this suite.
Suggested fix
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
@@
describe('LiteLLMProvider', () => {
let provider: LiteLLMProvider;
beforeEach(() => {
vi.clearAllMocks();
provider = new LiteLLMProvider();
});
+
+ afterEach(() => {
+ vi.unstubAllGlobals();
+ });
@@
- vi.unstubAllGlobals();
@@
- vi.unstubAllGlobals();
@@
- vi.unstubAllGlobals();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/llm/providers/__tests__/LiteLLMProvider.test.ts` around lines 160 -
205, The tests stub the global fetch inside each it block using vi.stubGlobal
but call vi.unstubAllGlobals only inside the happy path, which can leak fetch if
an assertion fails; refactor the describe('listAvailableModels') suite to move
vi.stubGlobal usage into each test as-is but add an afterEach block that calls
vi.unstubAllGlobals() (or vi.restoreAllMocks()/appropriate Vitest cleanup) to
ensure fetch is always restored; reference the test suite, the
provider.initialize(...) and provider.listAvailableModels() usages to locate
where stubbing happens and add the afterEach cleanup for all cases.
Comment on lines
+161
to
+168
| public async listAvailableModels( | ||
| filter?: { capability?: string }, | ||
| ): Promise<ModelInfo[]> { | ||
| try { | ||
| const url = this.proxyBaseURL.replace(/\/+$/, '') + '/models'; | ||
| const response = await fetch(url, { | ||
| headers: { Authorization: `Bearer ${this.proxyApiKey}` }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 "requestTimeout|listAvailableModels|fetch\\(" src/core/llm/providers/implementations/LiteLLMProvider.tsRepository: framerslab/agentos
Length of output: 1138
Add requestTimeout/AbortController to LiteLLMProvider.listAvailableModels() proxy /models fetch (src/core/llm/providers/implementations/LiteLLMProvider.ts:161-168).
initialize() forwards requestTimeout to the delegate, but listAvailableModels() performs a direct fetch(.../models) with no signal/timeout handling, so a stalled proxy can hang model discovery. Use the configured timeout for this fetch (AbortController) and ensure any timer is cleared even on errors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/llm/providers/implementations/LiteLLMProvider.ts` around lines 161 -
168, listAvailableModels() performs a direct fetch to the proxy /models without
honoring the configured requestTimeout, so a stalled proxy can hang discovery;
update listAvailableModels (in LiteLLMProvider) to create an AbortController,
start a timer using this.requestTimeout that calls controller.abort(), pass
controller.signal to fetch (using this.proxyBaseURL and this.proxyApiKey as
before), and ensure the timeout timer is cleared in a finally block so it is
always cleaned up even on errors; mirror the timeout-handling pattern used in
initialize() or other methods that forward requestTimeout to the delegate.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GroqProviderdelegation pattern.LiteLLMProviderwrapsOpenAIProviderwith the user's LiteLLM proxy URL, enabling access to 100+ LLM providers (Anthropic, Google, Bedrock, Azure, Ollama, etc.) through a single OpenAI-compatible gateway.Checklist
Changes:
src/core/llm/providers/implementations/LiteLLMProvider.ts- new provider (delegates toOpenAIProvider, dynamic model discovery via/v1/models)src/core/llm/providers/AIModelProviderManager.ts- registered'litellm'casesrc/core/llm/providers/__tests__/LiteLLMProvider.test.ts- 12 unit testsLive E2E against a real LiteLLM proxy (routing to Claude via Azure Foundry):
Usage:
Additive only. No existing providers modified. Zero new dependencies.
Summary by Sourcery
Add a new LiteLLM provider that delegates to the existing OpenAI provider and register it with the AI model provider manager.
New Features:
Tests:
Summary by CodeRabbit