-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: prevent model mixing between LiteLLM configuration profiles #9170
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
base: main
Are you sure you want to change the base?
Conversation
- Add unique cache keys that include base URL hash for providers with multiple instances - Update file cache naming to include instance-specific information - Improve cache flushing to handle all cache keys for a provider Fixes #9169
Review completed. Found 2 issues that should be addressed:
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
|
|
||
| // For providers that can have multiple instances with different base URLs, | ||
| // include the base URL in the cache key | ||
| if (baseUrl && ["litellm", "ollama", "lmstudio", "requesty", "deepinfra"].includes(provider)) { |
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.
The "roo" provider is missing from this list but also supports optional baseUrl (see line 166 in src/shared/api.ts). When users configure multiple Roo provider instances with different base URLs, their model caches will incorrectly share the same key, causing the same model mixing issue this PR aims to fix.
Fix it with Roo Code or mention @roomote and request a fix.
| export const flushModels = async (router: RouterName) => { | ||
| memoryCache.del(router) | ||
| // For providers that can have multiple instances, we need to flush all possible cache keys | ||
| // Since we don't know all the possible base URLs, we'll flush all keys that start with the provider name | ||
| const keys = memoryCache.keys() | ||
| for (const key of keys) { | ||
| if (key.startsWith(router)) { | ||
| memoryCache.del(key) | ||
| } | ||
| } | ||
| } |
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.
The flushModels function only clears the in-memory cache but doesn't remove corresponding file cache entries (created by writeModels at line 64). When a user flushes models for a provider with multiple instances, stale file caches will persist and could be read back by readModels (line 72), defeating the purpose of the flush operation. Consider iterating through cache files matching the provider pattern and deleting them, similar to how the memory cache keys are being cleared.
Fix it with Roo Code or mention @roomote and request a fix.
This PR attempts to address Issue #9169. Feedback and guidance are welcome.
Problem
When two configuration profiles are created with LiteLLM API provider but different instances (different base URLs), the list of models was always pulled from one of them, causing issues when they don't provide the same models.
Root Cause
The model cache was using only the provider name as the cache key, not distinguishing between different LiteLLM instances with different base URLs.
Solution
Testing
Fixes #9169
Important
Fixes model cache issue by using unique cache keys with base URL hash for LiteLLM and similar providers in
modelCache.ts.modelCache.ts.getCacheKey()andgetCacheFilename()to generate unique cache keys and filenames.writeModels(),readModels(), andgetModels()to use new cache key logic.flushModels()to clear all cache keys for a provider.This description was created by
for 2081cb1. You can customize this summary. It will automatically update as commits are pushed.