From 1d0a17bb967e24b96f6199a8a7fac34c40e3c810 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 6 Jan 2026 20:57:46 +0000 Subject: [PATCH] fix: prioritize user-configured embedding dimension over model default This fix addresses issue #8102 where the embedding dimension setting was not consistently applied (1536 vs 1024 mismatch). The root cause was that the code prioritized the model's built-in dimension over the user-configured dimension. This caused a dimension mismatch when: 1. User set a custom dimension (e.g., 1536) 2. Collection was created with user dimension (1536) 3. But embeddings were generated using model default (e.g., 1024) Changes: - config-manager.ts: Updated currentModelDimension getter to prioritize user-configured dimension as the single source of truth - service-factory.ts: Updated createVectorStore() to prioritize user-configured dimension when creating the vector store - Updated tests to reflect the new expected behavior The fix ensures that when a user explicitly sets an embedding dimension, that dimension is used consistently across both collection creation and embedding generation, preventing dimension mismatch errors. --- .../__tests__/config-manager.spec.ts | 35 +++++++++++--- .../__tests__/service-factory.spec.ts | 46 +++++++++++++++---- src/services/code-index/config-manager.ts | 16 +++---- src/services/code-index/service-factory.ts | 11 +++-- 4 files changed, 80 insertions(+), 28 deletions(-) diff --git a/src/services/code-index/__tests__/config-manager.spec.ts b/src/services/code-index/__tests__/config-manager.spec.ts index 27815c0bef..eaa4a7c2b4 100644 --- a/src/services/code-index/__tests__/config-manager.spec.ts +++ b/src/services/code-index/__tests__/config-manager.spec.ts @@ -1689,7 +1689,7 @@ describe("CodeIndexConfigManager", () => { vi.clearAllMocks() }) - it("should return model's built-in dimension when available", async () => { + it("should prioritize user-configured dimension over model's built-in dimension", async () => { // Mock getModelDimension to return a built-in dimension mockedGetModelDimension.mockReturnValue(1536) @@ -1697,7 +1697,7 @@ describe("CodeIndexConfigManager", () => { codebaseIndexEnabled: true, codebaseIndexEmbedderProvider: "openai", codebaseIndexEmbedderModelId: "text-embedding-3-small", - codebaseIndexEmbedderModelDimension: 2048, // Custom dimension should be ignored + codebaseIndexEmbedderModelDimension: 2048, // Custom dimension takes priority codebaseIndexQdrantUrl: "http://localhost:6333", }) mockContextProxy.getSecret.mockImplementation((key: string) => { @@ -1708,12 +1708,36 @@ describe("CodeIndexConfigManager", () => { configManager = new CodeIndexConfigManager(mockContextProxy) await configManager.loadConfiguration() - // Should return model's built-in dimension, not custom + // Should return user-configured dimension, not model's built-in + expect(configManager.currentModelDimension).toBe(2048) + // getModelDimension should not be called when user has configured dimension + }) + + it("should fall back to model's built-in dimension when no custom dimension set", async () => { + // Mock getModelDimension to return a built-in dimension + mockedGetModelDimension.mockReturnValue(1536) + + mockContextProxy.getGlobalState.mockReturnValue({ + codebaseIndexEnabled: true, + codebaseIndexEmbedderProvider: "openai", + codebaseIndexEmbedderModelId: "text-embedding-3-small", + // No custom dimension set + codebaseIndexQdrantUrl: "http://localhost:6333", + }) + mockContextProxy.getSecret.mockImplementation((key: string) => { + if (key === "codeIndexOpenAiKey") return "test-key" + return undefined + }) + + configManager = new CodeIndexConfigManager(mockContextProxy) + await configManager.loadConfiguration() + + // Should fall back to model's built-in dimension expect(configManager.currentModelDimension).toBe(1536) expect(mockedGetModelDimension).toHaveBeenCalledWith("openai", "text-embedding-3-small") }) - it("should use custom dimension only when model has no built-in dimension", async () => { + it("should use custom dimension even when model has no built-in dimension", async () => { // Mock getModelDimension to return undefined (no built-in dimension) mockedGetModelDimension.mockReturnValue(undefined) @@ -1732,9 +1756,8 @@ describe("CodeIndexConfigManager", () => { configManager = new CodeIndexConfigManager(mockContextProxy) await configManager.loadConfiguration() - // Should use custom dimension as fallback + // Custom dimension takes priority regardless of model's built-in dimension expect(configManager.currentModelDimension).toBe(2048) - expect(mockedGetModelDimension).toHaveBeenCalledWith("openai-compatible", "custom-model") }) it("should return undefined when neither model dimension nor custom dimension is available", async () => { diff --git a/src/services/code-index/__tests__/service-factory.spec.ts b/src/services/code-index/__tests__/service-factory.spec.ts index 1d8f7ba478..d0de312d18 100644 --- a/src/services/code-index/__tests__/service-factory.spec.ts +++ b/src/services/code-index/__tests__/service-factory.spec.ts @@ -420,7 +420,7 @@ describe("CodeIndexServiceFactory", () => { ) }) - it("should prioritize getModelDimension over manual modelDimension for OpenAI Compatible provider", () => { + it("should prioritize user-configured modelDimension over model's built-in dimension", () => { // Arrange const testModelId = "custom-model" const manualDimension = 1024 @@ -428,7 +428,7 @@ describe("CodeIndexServiceFactory", () => { const testConfig = { embedderProvider: "openai-compatible", modelId: testModelId, - modelDimension: manualDimension, // This should be ignored when model has built-in dimension + modelDimension: manualDimension, // User-configured dimension takes priority openAiCompatibleOptions: { baseUrl: "https://api.example.com/v1", apiKey: "test-api-key", @@ -437,22 +437,21 @@ describe("CodeIndexServiceFactory", () => { qdrantApiKey: "test-key", } mockConfigManager.getConfig.mockReturnValue(testConfig as any) - mockGetModelDimension.mockReturnValue(modelDimension) // This should be used + mockGetModelDimension.mockReturnValue(modelDimension) // This should be ignored // Act factory.createVectorStore() - // Assert - expect(mockGetModelDimension).toHaveBeenCalledWith("openai-compatible", testModelId) + // Assert - user-configured dimension takes priority as single source of truth expect(MockedQdrantVectorStore).toHaveBeenCalledWith( "/test/workspace", "http://localhost:6333", - modelDimension, // Should use model's built-in dimension, not manual + manualDimension, // Should use user-configured dimension "test-key", ) }) - it("should use manual modelDimension only when model has no built-in dimension", () => { + it("should fall back to model's built-in dimension when no user-configured dimension", () => { // Arrange const testModelId = "unknown-model" const manualDimension = 1024 @@ -473,12 +472,41 @@ describe("CodeIndexServiceFactory", () => { // Act factory.createVectorStore() - // Assert + // Assert - user-configured dimension is used + expect(MockedQdrantVectorStore).toHaveBeenCalledWith( + "/test/workspace", + "http://localhost:6333", + manualDimension, + "test-key", + ) + }) + + it("should use model dimension when user has not configured a dimension", () => { + // Arrange + const testModelId = "text-embedding-3-small" + const testConfig = { + embedderProvider: "openai-compatible", + modelId: testModelId, + // No modelDimension configured by user + openAiCompatibleOptions: { + baseUrl: "https://api.example.com/v1", + apiKey: "test-api-key", + }, + qdrantUrl: "http://localhost:6333", + qdrantApiKey: "test-key", + } + mockConfigManager.getConfig.mockReturnValue(testConfig as any) + mockGetModelDimension.mockReturnValue(1536) // Model's built-in dimension + + // Act + factory.createVectorStore() + + // Assert - falls back to model's built-in dimension expect(mockGetModelDimension).toHaveBeenCalledWith("openai-compatible", testModelId) expect(MockedQdrantVectorStore).toHaveBeenCalledWith( "/test/workspace", "http://localhost:6333", - manualDimension, // Should use manual dimension as fallback + 1536, // Should use model's built-in dimension as fallback "test-key", ) }) diff --git a/src/services/code-index/config-manager.ts b/src/services/code-index/config-manager.ts index e7f239e621..0ecf668e9a 100644 --- a/src/services/code-index/config-manager.ts +++ b/src/services/code-index/config-manager.ts @@ -503,19 +503,19 @@ export class CodeIndexConfigManager { /** * Gets the current model dimension being used for embeddings. - * Returns the model's built-in dimension if available, otherwise falls back to custom dimension. + * Prioritizes user-configured dimension as the single source of truth. + * Falls back to model's built-in dimension only when user hasn't configured one. */ public get currentModelDimension(): number | undefined { - // First try to get the model-specific dimension - const modelId = this.modelId ?? getDefaultModelId(this.embedderProvider) - const modelDimension = getModelDimension(this.embedderProvider, modelId) - - // Only use custom dimension if model doesn't have a built-in dimension - if (!modelDimension && this.modelDimension && this.modelDimension > 0) { + // User-configured dimension takes priority as the single source of truth + // This ensures consistency between collection creation and embedding generation + if (this.modelDimension && this.modelDimension > 0) { return this.modelDimension } - return modelDimension + // Fall back to model's built-in dimension when user hasn't configured one + const modelId = this.modelId ?? getDefaultModelId(this.embedderProvider) + return getModelDimension(this.embedderProvider, modelId) } /** diff --git a/src/services/code-index/service-factory.ts b/src/services/code-index/service-factory.ts index c98c65d4c1..4cbac0836e 100644 --- a/src/services/code-index/service-factory.ts +++ b/src/services/code-index/service-factory.ts @@ -141,12 +141,13 @@ export class CodeIndexServiceFactory { let vectorSize: number | undefined - // First try to get the model-specific dimension from profiles - vectorSize = getModelDimension(provider, modelId) - - // Only use manual dimension if model doesn't have a built-in dimension - if (!vectorSize && config.modelDimension && config.modelDimension > 0) { + // User-configured dimension takes priority as the single source of truth + // This ensures consistency between collection creation and embedding generation + if (config.modelDimension && config.modelDimension > 0) { vectorSize = config.modelDimension + } else { + // Fall back to model's built-in dimension from profiles + vectorSize = getModelDimension(provider, modelId) } if (vectorSize === undefined || vectorSize <= 0) {