mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-06 08:18:39 +00:00
fix: prioritize user-configured embedding dimension over model default
- Updated config-manager.ts to check user-configured dimension first - Updated service-factory.ts to use the same dimension priority logic - Fixed tests to reflect the new behavior - Added specific test for issue #8102 scenario (1536 vs 1024) Fixes #8102
This commit is contained in:
parent
87b45def18
commit
0c35f226d0
3 changed files with 49 additions and 24 deletions
|
|
@ -1688,7 +1688,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)
|
||||
|
||||
|
|
@ -1696,7 +1696,7 @@ describe("CodeIndexConfigManager", () => {
|
|||
codebaseIndexEnabled: true,
|
||||
codebaseIndexEmbedderProvider: "openai",
|
||||
codebaseIndexEmbedderModelId: "text-embedding-3-small",
|
||||
codebaseIndexEmbedderModelDimension: 2048, // Custom dimension should be ignored
|
||||
codebaseIndexEmbedderModelDimension: 2048, // Custom dimension should take priority
|
||||
codebaseIndexQdrantUrl: "http://localhost:6333",
|
||||
})
|
||||
mockContextProxy.getSecret.mockImplementation((key: string) => {
|
||||
|
|
@ -1707,20 +1707,46 @@ 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 custom dimension is set
|
||||
expect(mockedGetModelDimension).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should fall back to model's built-in dimension when no custom dimension is 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 () => {
|
||||
// Mock getModelDimension to return undefined (no built-in dimension)
|
||||
mockedGetModelDimension.mockReturnValue(undefined)
|
||||
it("should use user-configured dimension (1536) even when model default is different (1024)", async () => {
|
||||
// This test specifically addresses the issue reported in #8102
|
||||
// Mock getModelDimension to return 1024 (model default)
|
||||
mockedGetModelDimension.mockReturnValue(1024)
|
||||
|
||||
mockContextProxy.getGlobalState.mockReturnValue({
|
||||
codebaseIndexEnabled: true,
|
||||
codebaseIndexEmbedderProvider: "openai-compatible",
|
||||
codebaseIndexEmbedderModelId: "custom-model",
|
||||
codebaseIndexEmbedderModelDimension: 2048, // Custom dimension should be used
|
||||
codebaseIndexEmbedderModelId: "some-model-with-1024-default",
|
||||
codebaseIndexEmbedderModelDimension: 1536, // User explicitly sets 1536
|
||||
codebaseIndexQdrantUrl: "http://localhost:6333",
|
||||
})
|
||||
mockContextProxy.getSecret.mockImplementation((key: string) => {
|
||||
|
|
@ -1731,9 +1757,10 @@ describe("CodeIndexConfigManager", () => {
|
|||
configManager = new CodeIndexConfigManager(mockContextProxy)
|
||||
await configManager.loadConfiguration()
|
||||
|
||||
// Should use custom dimension as fallback
|
||||
expect(configManager.currentModelDimension).toBe(2048)
|
||||
expect(mockedGetModelDimension).toHaveBeenCalledWith("openai-compatible", "custom-model")
|
||||
// Should use user-configured dimension (1536), not model default (1024)
|
||||
expect(configManager.currentModelDimension).toBe(1536)
|
||||
// getModelDimension should not be called when custom dimension is set
|
||||
expect(mockedGetModelDimension).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it("should return undefined when neither model dimension nor custom dimension is available", async () => {
|
||||
|
|
|
|||
|
|
@ -442,19 +442,17 @@ 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.
|
||||
* Returns the user-configured custom dimension if set, otherwise falls back to model's built-in dimension.
|
||||
*/
|
||||
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) {
|
||||
// Prioritize user-configured custom dimension if explicitly set
|
||||
if (this.modelDimension && this.modelDimension > 0) {
|
||||
return this.modelDimension
|
||||
}
|
||||
|
||||
return modelDimension
|
||||
// Fall back to model-specific dimension from profiles
|
||||
const modelId = this.modelId ?? getDefaultModelId(this.embedderProvider)
|
||||
return getModelDimension(this.embedderProvider, modelId)
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -123,12 +123,12 @@ 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) {
|
||||
// Prioritize user-configured custom dimension if explicitly set
|
||||
if (config.modelDimension && config.modelDimension > 0) {
|
||||
vectorSize = config.modelDimension
|
||||
} else {
|
||||
// Fall back to model-specific dimension from profiles
|
||||
vectorSize = getModelDimension(provider, modelId)
|
||||
}
|
||||
|
||||
if (vectorSize === undefined || vectorSize <= 0) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue