mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
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.
This commit is contained in:
parent
503f40241d
commit
1d0a17bb96
4 changed files with 80 additions and 28 deletions
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue