From a42c2d0b304e614a6860bba79c5479a847e680aa Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 18 Aug 2025 09:34:59 -0500 Subject: [PATCH 1/3] fix: prevent duplicate LM Studio models with case-insensitive deduplication - Keep both listDownloadedModels and listLoaded APIs to support JIT loading - Implement case-insensitive deduplication to prevent duplicates - When duplicates are found, prefer loaded model data for accurate runtime info - Add test coverage for deduplication logic - Addresses feedback about LM Studio's JIT Model Loading feature (v0.3.5+) Fixes #6954 --- .../fetchers/__tests__/lmstudio.test.ts | 44 +++++++++++++++++++ src/api/providers/fetchers/lmstudio.ts | 28 +++++++++++- 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/lmstudio.test.ts b/src/api/providers/fetchers/__tests__/lmstudio.test.ts index ff9a109e50..7b6d8dd2b1 100644 --- a/src/api/providers/fetchers/__tests__/lmstudio.test.ts +++ b/src/api/providers/fetchers/__tests__/lmstudio.test.ts @@ -143,6 +143,50 @@ describe("LMStudio Fetcher", () => { expect(result).toEqual({ [mockRawModel.modelKey]: expectedParsedModel }) }) + it("should deduplicate models when both downloaded and loaded", async () => { + const mockDownloadedModel: LLMInfo = { + type: "llm" as const, + modelKey: "mistralai/devstral-small-2505", + format: "safetensors", + displayName: "Devstral Small 2505", + path: "mistralai/devstral-small-2505", + sizeBytes: 13277565112, + architecture: "mistral", + vision: false, + trainedForToolUse: false, + maxContextLength: 131072, + } + + const mockLoadedModel: LLMInstanceInfo = { + type: "llm", + modelKey: "devstral-small-2505", // Different key but should match case-insensitively + format: "safetensors", + displayName: "Devstral Small 2505", + path: "mistralai/devstral-small-2505", + sizeBytes: 13277565112, + architecture: "mistral", + identifier: "mistralai/devstral-small-2505", + instanceReference: "RAP5qbeHVjJgBiGFQ6STCuTJ", + vision: false, + trainedForToolUse: false, + maxContextLength: 131072, + contextLength: 7161, // Runtime context info + } + + mockedAxios.get.mockResolvedValueOnce({ data: { status: "ok" } }) + mockListDownloadedModels.mockResolvedValueOnce([mockDownloadedModel]) + mockListLoaded.mockResolvedValueOnce([{ getModelInfo: vi.fn().mockResolvedValueOnce(mockLoadedModel) }]) + + const result = await getLMStudioModels(baseUrl) + + // Should only have one model, with the loaded model's runtime info taking precedence + expect(Object.keys(result)).toHaveLength(1) + + // The downloaded model's path should be the key, but with loaded model's data + const expectedParsedModel = parseLMStudioModel(mockLoadedModel) + expect(result[mockDownloadedModel.path]).toEqual(expectedParsedModel) + }) + it("should use default baseUrl if an empty string is provided", async () => { const defaultBaseUrl = "http://localhost:1234" const defaultLmsUrl = "ws://localhost:1234" diff --git a/src/api/providers/fetchers/lmstudio.ts b/src/api/providers/fetchers/lmstudio.ts index 976822c67d..9526081fd5 100644 --- a/src/api/providers/fetchers/lmstudio.ts +++ b/src/api/providers/fetchers/lmstudio.ts @@ -81,13 +81,37 @@ export async function getLMStudioModels(baseUrl = "http://localhost:1234"): Prom } catch (error) { console.warn("Failed to list downloaded models, falling back to loaded models only") } - // We want to list loaded models *anyway* since they provide valuable extra info (context size) + + // Get loaded models for their runtime info (context size) const loadedModels = (await client.llm.listLoaded().then((models: LLM[]) => { return Promise.all(models.map((m) => m.getModelInfo())) })) as Array + // Deduplicate loaded models - only add if no existing key contains the model's identifier (case-insensitive) for (const lmstudioModel of loadedModels) { - models[lmstudioModel.modelKey] = parseLMStudioModel(lmstudioModel) + const modelIdentifier = lmstudioModel.modelKey.toLowerCase() + + // Check if any existing model key contains this loaded model's identifier + const isDuplicate = Object.keys(models).some( + (existingKey) => + existingKey.toLowerCase().includes(modelIdentifier) || + modelIdentifier.includes(existingKey.toLowerCase()), + ) + + if (!isDuplicate) { + // Use modelKey for loaded models to maintain consistency + models[lmstudioModel.modelKey] = parseLMStudioModel(lmstudioModel) + } else { + // If it's a duplicate, update the existing entry with loaded model info for better context data + const existingKey = Object.keys(models).find( + (key) => key.toLowerCase().includes(modelIdentifier) || modelIdentifier.includes(key.toLowerCase()), + ) + if (existingKey) { + // Update with loaded model data which has more accurate runtime info + models[existingKey] = parseLMStudioModel(lmstudioModel) + } + } + modelsWithLoadedDetails.add(lmstudioModel.modelKey) } } catch (error) { From 4b2d305c4f2217b665455e0776a13edd9899d585 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 18 Aug 2025 09:48:04 -0500 Subject: [PATCH 2/3] fix: correct deduplication logic to prefer loaded models - When a loaded model ID is found in any downloaded model key (case-insensitive) - Remove the downloaded model and replace with the loaded model - This ensures loaded models with runtime info take precedence - Updated tests to verify the correct deduplication behavior --- .../fetchers/__tests__/lmstudio.test.ts | 9 ++++-- src/api/providers/fetchers/lmstudio.ts | 30 +++++++------------ 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/lmstudio.test.ts b/src/api/providers/fetchers/__tests__/lmstudio.test.ts index 7b6d8dd2b1..90c01522b0 100644 --- a/src/api/providers/fetchers/__tests__/lmstudio.test.ts +++ b/src/api/providers/fetchers/__tests__/lmstudio.test.ts @@ -179,12 +179,15 @@ describe("LMStudio Fetcher", () => { const result = await getLMStudioModels(baseUrl) - // Should only have one model, with the loaded model's runtime info taking precedence + // Should only have one model, with the loaded model replacing the downloaded one expect(Object.keys(result)).toHaveLength(1) - // The downloaded model's path should be the key, but with loaded model's data + // The loaded model's key should be used, with loaded model's data const expectedParsedModel = parseLMStudioModel(mockLoadedModel) - expect(result[mockDownloadedModel.path]).toEqual(expectedParsedModel) + expect(result[mockLoadedModel.modelKey]).toEqual(expectedParsedModel) + + // The downloaded model should have been removed + expect(result[mockDownloadedModel.path]).toBeUndefined() }) it("should use default baseUrl if an empty string is provided", async () => { diff --git a/src/api/providers/fetchers/lmstudio.ts b/src/api/providers/fetchers/lmstudio.ts index 9526081fd5..44f8dbf8dc 100644 --- a/src/api/providers/fetchers/lmstudio.ts +++ b/src/api/providers/fetchers/lmstudio.ts @@ -87,31 +87,21 @@ export async function getLMStudioModels(baseUrl = "http://localhost:1234"): Prom return Promise.all(models.map((m) => m.getModelInfo())) })) as Array - // Deduplicate loaded models - only add if no existing key contains the model's identifier (case-insensitive) + // Deduplicate: For each loaded model, check if any existing model contains its ID (case-insensitive) + // If found, remove the downloaded version and add the loaded model (prefer loaded over downloaded) for (const lmstudioModel of loadedModels) { - const modelIdentifier = lmstudioModel.modelKey.toLowerCase() + const loadedModelId = lmstudioModel.modelKey.toLowerCase() - // Check if any existing model key contains this loaded model's identifier - const isDuplicate = Object.keys(models).some( - (existingKey) => - existingKey.toLowerCase().includes(modelIdentifier) || - modelIdentifier.includes(existingKey.toLowerCase()), - ) + // Find if any existing model key includes the loaded model's ID + const existingKey = Object.keys(models).find((key) => key.toLowerCase().includes(loadedModelId)) - if (!isDuplicate) { - // Use modelKey for loaded models to maintain consistency - models[lmstudioModel.modelKey] = parseLMStudioModel(lmstudioModel) - } else { - // If it's a duplicate, update the existing entry with loaded model info for better context data - const existingKey = Object.keys(models).find( - (key) => key.toLowerCase().includes(modelIdentifier) || modelIdentifier.includes(key.toLowerCase()), - ) - if (existingKey) { - // Update with loaded model data which has more accurate runtime info - models[existingKey] = parseLMStudioModel(lmstudioModel) - } + if (existingKey) { + // Remove the downloaded version + delete models[existingKey] } + // Add the loaded model (either as replacement or new entry) + models[lmstudioModel.modelKey] = parseLMStudioModel(lmstudioModel) modelsWithLoadedDetails.add(lmstudioModel.modelKey) } } catch (error) { From 6aba8e562f61bb4e898ac5a939833eca26165384 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 18 Aug 2025 10:05:26 -0500 Subject: [PATCH 3/3] fix: improve deduplication logic and add comprehensive test coverage - Enhanced deduplication to use path segment matching instead of simple substring - Prevents false positives like 'llama' matching 'codellama' - Added comprehensive test cases for edge cases and multiple scenarios - Maintains support for JIT Model Loading feature --- .../fetchers/__tests__/lmstudio.test.ts | 175 ++++++++++++++++++ src/api/providers/fetchers/lmstudio.ts | 20 +- 2 files changed, 191 insertions(+), 4 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/lmstudio.test.ts b/src/api/providers/fetchers/__tests__/lmstudio.test.ts index 90c01522b0..8e7e36c73f 100644 --- a/src/api/providers/fetchers/__tests__/lmstudio.test.ts +++ b/src/api/providers/fetchers/__tests__/lmstudio.test.ts @@ -190,6 +190,181 @@ describe("LMStudio Fetcher", () => { expect(result[mockDownloadedModel.path]).toBeUndefined() }) + it("should handle deduplication with path-based matching", async () => { + const mockDownloadedModel: LLMInfo = { + type: "llm" as const, + modelKey: "Meta/Llama-3.1/8B-Instruct", + format: "gguf", + displayName: "Llama 3.1 8B Instruct", + path: "Meta/Llama-3.1/8B-Instruct", + sizeBytes: 8000000000, + architecture: "llama", + vision: false, + trainedForToolUse: false, + maxContextLength: 8192, + } + + const mockLoadedModel: LLMInstanceInfo = { + type: "llm", + modelKey: "Llama-3.1", // Should match the path segment + format: "gguf", + displayName: "Llama 3.1", + path: "Meta/Llama-3.1/8B-Instruct", + sizeBytes: 8000000000, + architecture: "llama", + identifier: "Meta/Llama-3.1/8B-Instruct", + instanceReference: "ABC123", + vision: false, + trainedForToolUse: false, + maxContextLength: 8192, + contextLength: 4096, + } + + mockedAxios.get.mockResolvedValueOnce({ data: { status: "ok" } }) + mockListDownloadedModels.mockResolvedValueOnce([mockDownloadedModel]) + mockListLoaded.mockResolvedValueOnce([{ getModelInfo: vi.fn().mockResolvedValueOnce(mockLoadedModel) }]) + + const result = await getLMStudioModels(baseUrl) + + expect(Object.keys(result)).toHaveLength(1) + expect(result[mockLoadedModel.modelKey]).toBeDefined() + expect(result[mockDownloadedModel.path]).toBeUndefined() + }) + + it("should not deduplicate models with similar but distinct names", async () => { + const mockDownloadedModels: LLMInfo[] = [ + { + type: "llm" as const, + modelKey: "mistral-7b", + format: "gguf", + displayName: "Mistral 7B", + path: "mistralai/mistral-7b-instruct", + sizeBytes: 7000000000, + architecture: "mistral", + vision: false, + trainedForToolUse: false, + maxContextLength: 4096, + }, + { + type: "llm" as const, + modelKey: "codellama", + format: "gguf", + displayName: "Code Llama", + path: "meta/codellama/7b", + sizeBytes: 7000000000, + architecture: "llama", + vision: false, + trainedForToolUse: false, + maxContextLength: 4096, + }, + ] + + const mockLoadedModel: LLMInstanceInfo = { + type: "llm", + modelKey: "llama", // Should not match "codellama" or "mistral-7b" + format: "gguf", + displayName: "Llama", + path: "meta/llama/7b", + sizeBytes: 7000000000, + architecture: "llama", + identifier: "meta/llama/7b", + instanceReference: "XYZ789", + vision: false, + trainedForToolUse: false, + maxContextLength: 4096, + contextLength: 2048, + } + + mockedAxios.get.mockResolvedValueOnce({ data: { status: "ok" } }) + mockListDownloadedModels.mockResolvedValueOnce(mockDownloadedModels) + mockListLoaded.mockResolvedValueOnce([{ getModelInfo: vi.fn().mockResolvedValueOnce(mockLoadedModel) }]) + + const result = await getLMStudioModels(baseUrl) + + // Should have 3 models: mistral-7b (not deduped), codellama (not deduped), and llama (loaded) + expect(Object.keys(result)).toHaveLength(3) + expect(result["mistralai/mistral-7b-instruct"]).toBeDefined() // Should NOT be removed + expect(result["meta/codellama/7b"]).toBeDefined() // Should NOT be removed (codellama != llama) + expect(result[mockLoadedModel.modelKey]).toBeDefined() + }) + + it("should handle multiple loaded models with various duplicate scenarios", async () => { + const mockDownloadedModels: LLMInfo[] = [ + { + type: "llm" as const, + modelKey: "mistral-7b", + format: "gguf", + displayName: "Mistral 7B", + path: "mistralai/mistral-7b/instruct", + sizeBytes: 7000000000, + architecture: "mistral", + vision: false, + trainedForToolUse: false, + maxContextLength: 8192, + }, + { + type: "llm" as const, + modelKey: "llama-3.1", + format: "gguf", + displayName: "Llama 3.1", + path: "meta/llama-3.1/8b", + sizeBytes: 8000000000, + architecture: "llama", + vision: false, + trainedForToolUse: false, + maxContextLength: 8192, + }, + ] + + const mockLoadedModels: LLMInstanceInfo[] = [ + { + type: "llm", + modelKey: "mistral-7b", // Exact match with path segment + format: "gguf", + displayName: "Mistral 7B", + path: "mistralai/mistral-7b/instruct", + sizeBytes: 7000000000, + architecture: "mistral", + identifier: "mistralai/mistral-7b/instruct", + instanceReference: "REF1", + vision: false, + trainedForToolUse: false, + maxContextLength: 8192, + contextLength: 4096, + }, + { + type: "llm", + modelKey: "gpt-4", // No match, new model + format: "gguf", + displayName: "GPT-4", + path: "openai/gpt-4", + sizeBytes: 10000000000, + architecture: "gpt", + identifier: "openai/gpt-4", + instanceReference: "REF2", + vision: true, + trainedForToolUse: true, + maxContextLength: 32768, + contextLength: 16384, + }, + ] + + mockedAxios.get.mockResolvedValueOnce({ data: { status: "ok" } }) + mockListDownloadedModels.mockResolvedValueOnce(mockDownloadedModels) + mockListLoaded.mockResolvedValueOnce( + mockLoadedModels.map((model) => ({ getModelInfo: vi.fn().mockResolvedValueOnce(model) })), + ) + + const result = await getLMStudioModels(baseUrl) + + // Should have 3 models: llama-3.1 (downloaded), mistral-7b (loaded, replaced), gpt-4 (loaded, new) + expect(Object.keys(result)).toHaveLength(3) + expect(result["meta/llama-3.1/8b"]).toBeDefined() // Downloaded, not replaced + expect(result["mistralai/mistral-7b/instruct"]).toBeUndefined() // Downloaded, replaced + expect(result["mistral-7b"]).toBeDefined() // Loaded, replaced downloaded + expect(result["gpt-4"]).toBeDefined() // Loaded, new + }) + it("should use default baseUrl if an empty string is provided", async () => { const defaultBaseUrl = "http://localhost:1234" const defaultLmsUrl = "ws://localhost:1234" diff --git a/src/api/providers/fetchers/lmstudio.ts b/src/api/providers/fetchers/lmstudio.ts index 44f8dbf8dc..1e2e016df2 100644 --- a/src/api/providers/fetchers/lmstudio.ts +++ b/src/api/providers/fetchers/lmstudio.ts @@ -87,13 +87,25 @@ export async function getLMStudioModels(baseUrl = "http://localhost:1234"): Prom return Promise.all(models.map((m) => m.getModelInfo())) })) as Array - // Deduplicate: For each loaded model, check if any existing model contains its ID (case-insensitive) - // If found, remove the downloaded version and add the loaded model (prefer loaded over downloaded) + // Deduplicate: For each loaded model, check if any downloaded model path contains the loaded model's key + // This handles cases like loaded "llama-3.1" matching downloaded "Meta/Llama-3.1/Something" + // If found, remove the downloaded version and add the loaded model (prefer loaded over downloaded for accurate runtime info) for (const lmstudioModel of loadedModels) { const loadedModelId = lmstudioModel.modelKey.toLowerCase() - // Find if any existing model key includes the loaded model's ID - const existingKey = Object.keys(models).find((key) => key.toLowerCase().includes(loadedModelId)) + // Find if any downloaded model path contains the loaded model's key as a path segment + // Use word boundaries or path separators to avoid false matches like "llama" matching "codellama" + const existingKey = Object.keys(models).find((key) => { + const keyLower = key.toLowerCase() + // Check if the loaded model ID appears as a distinct segment in the path + // This matches "llama-3.1" in "Meta/Llama-3.1/Something" but not "llama" in "codellama" + return ( + keyLower.includes(`/${loadedModelId}/`) || + keyLower.includes(`/${loadedModelId}`) || + keyLower.startsWith(`${loadedModelId}/`) || + keyLower === loadedModelId + ) + }) if (existingKey) { // Remove the downloaded version