Skip to content

Commit 0c35f22

Browse files
committed
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
1 parent 87b45de commit 0c35f22

File tree

3 files changed

+49
-24
lines changed

3 files changed

+49
-24
lines changed

src/services/code-index/__tests__/config-manager.spec.ts

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1688,15 +1688,15 @@ describe("CodeIndexConfigManager", () => {
16881688
vi.clearAllMocks()
16891689
})
16901690

1691-
it("should return model's built-in dimension when available", async () => {
1691+
it("should prioritize user-configured dimension over model's built-in dimension", async () => {
16921692
// Mock getModelDimension to return a built-in dimension
16931693
mockedGetModelDimension.mockReturnValue(1536)
16941694

16951695
mockContextProxy.getGlobalState.mockReturnValue({
16961696
codebaseIndexEnabled: true,
16971697
codebaseIndexEmbedderProvider: "openai",
16981698
codebaseIndexEmbedderModelId: "text-embedding-3-small",
1699-
codebaseIndexEmbedderModelDimension: 2048, // Custom dimension should be ignored
1699+
codebaseIndexEmbedderModelDimension: 2048, // Custom dimension should take priority
17001700
codebaseIndexQdrantUrl: "http://localhost:6333",
17011701
})
17021702
mockContextProxy.getSecret.mockImplementation((key: string) => {
@@ -1707,20 +1707,46 @@ describe("CodeIndexConfigManager", () => {
17071707
configManager = new CodeIndexConfigManager(mockContextProxy)
17081708
await configManager.loadConfiguration()
17091709

1710-
// Should return model's built-in dimension, not custom
1710+
// Should return user-configured dimension, not model's built-in
1711+
expect(configManager.currentModelDimension).toBe(2048)
1712+
// getModelDimension should not be called when custom dimension is set
1713+
expect(mockedGetModelDimension).not.toHaveBeenCalled()
1714+
})
1715+
1716+
it("should fall back to model's built-in dimension when no custom dimension is set", async () => {
1717+
// Mock getModelDimension to return a built-in dimension
1718+
mockedGetModelDimension.mockReturnValue(1536)
1719+
1720+
mockContextProxy.getGlobalState.mockReturnValue({
1721+
codebaseIndexEnabled: true,
1722+
codebaseIndexEmbedderProvider: "openai",
1723+
codebaseIndexEmbedderModelId: "text-embedding-3-small",
1724+
// No custom dimension set
1725+
codebaseIndexQdrantUrl: "http://localhost:6333",
1726+
})
1727+
mockContextProxy.getSecret.mockImplementation((key: string) => {
1728+
if (key === "codeIndexOpenAiKey") return "test-key"
1729+
return undefined
1730+
})
1731+
1732+
configManager = new CodeIndexConfigManager(mockContextProxy)
1733+
await configManager.loadConfiguration()
1734+
1735+
// Should fall back to model's built-in dimension
17111736
expect(configManager.currentModelDimension).toBe(1536)
17121737
expect(mockedGetModelDimension).toHaveBeenCalledWith("openai", "text-embedding-3-small")
17131738
})
17141739

1715-
it("should use custom dimension only when model has no built-in dimension", async () => {
1716-
// Mock getModelDimension to return undefined (no built-in dimension)
1717-
mockedGetModelDimension.mockReturnValue(undefined)
1740+
it("should use user-configured dimension (1536) even when model default is different (1024)", async () => {
1741+
// This test specifically addresses the issue reported in #8102
1742+
// Mock getModelDimension to return 1024 (model default)
1743+
mockedGetModelDimension.mockReturnValue(1024)
17181744

17191745
mockContextProxy.getGlobalState.mockReturnValue({
17201746
codebaseIndexEnabled: true,
17211747
codebaseIndexEmbedderProvider: "openai-compatible",
1722-
codebaseIndexEmbedderModelId: "custom-model",
1723-
codebaseIndexEmbedderModelDimension: 2048, // Custom dimension should be used
1748+
codebaseIndexEmbedderModelId: "some-model-with-1024-default",
1749+
codebaseIndexEmbedderModelDimension: 1536, // User explicitly sets 1536
17241750
codebaseIndexQdrantUrl: "http://localhost:6333",
17251751
})
17261752
mockContextProxy.getSecret.mockImplementation((key: string) => {
@@ -1731,9 +1757,10 @@ describe("CodeIndexConfigManager", () => {
17311757
configManager = new CodeIndexConfigManager(mockContextProxy)
17321758
await configManager.loadConfiguration()
17331759

1734-
// Should use custom dimension as fallback
1735-
expect(configManager.currentModelDimension).toBe(2048)
1736-
expect(mockedGetModelDimension).toHaveBeenCalledWith("openai-compatible", "custom-model")
1760+
// Should use user-configured dimension (1536), not model default (1024)
1761+
expect(configManager.currentModelDimension).toBe(1536)
1762+
// getModelDimension should not be called when custom dimension is set
1763+
expect(mockedGetModelDimension).not.toHaveBeenCalled()
17371764
})
17381765

17391766
it("should return undefined when neither model dimension nor custom dimension is available", async () => {

src/services/code-index/config-manager.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -442,19 +442,17 @@ export class CodeIndexConfigManager {
442442

443443
/**
444444
* Gets the current model dimension being used for embeddings.
445-
* Returns the model's built-in dimension if available, otherwise falls back to custom dimension.
445+
* Returns the user-configured custom dimension if set, otherwise falls back to model's built-in dimension.
446446
*/
447447
public get currentModelDimension(): number | undefined {
448-
// First try to get the model-specific dimension
449-
const modelId = this.modelId ?? getDefaultModelId(this.embedderProvider)
450-
const modelDimension = getModelDimension(this.embedderProvider, modelId)
451-
452-
// Only use custom dimension if model doesn't have a built-in dimension
453-
if (!modelDimension && this.modelDimension && this.modelDimension > 0) {
448+
// Prioritize user-configured custom dimension if explicitly set
449+
if (this.modelDimension && this.modelDimension > 0) {
454450
return this.modelDimension
455451
}
456452

457-
return modelDimension
453+
// Fall back to model-specific dimension from profiles
454+
const modelId = this.modelId ?? getDefaultModelId(this.embedderProvider)
455+
return getModelDimension(this.embedderProvider, modelId)
458456
}
459457

460458
/**

src/services/code-index/service-factory.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,12 @@ export class CodeIndexServiceFactory {
123123

124124
let vectorSize: number | undefined
125125

126-
// First try to get the model-specific dimension from profiles
127-
vectorSize = getModelDimension(provider, modelId)
128-
129-
// Only use manual dimension if model doesn't have a built-in dimension
130-
if (!vectorSize && config.modelDimension && config.modelDimension > 0) {
126+
// Prioritize user-configured custom dimension if explicitly set
127+
if (config.modelDimension && config.modelDimension > 0) {
131128
vectorSize = config.modelDimension
129+
} else {
130+
// Fall back to model-specific dimension from profiles
131+
vectorSize = getModelDimension(provider, modelId)
132132
}
133133

134134
if (vectorSize === undefined || vectorSize <= 0) {

0 commit comments

Comments
 (0)