diff --git a/.changeset/indexing-kilo-model-select.md b/.changeset/indexing-kilo-model-select.md new file mode 100644 index 00000000000..3d29381864c --- /dev/null +++ b/.changeset/indexing-kilo-model-select.md @@ -0,0 +1,8 @@ +--- +"kilo-code": patch +"@kilocode/kilo-indexing": patch +--- + +Fix the codebase indexing settings to show a model dropdown for the Kilo provider instead of a free-text "Embedding model" input. The Kilo embedding catalog is server-managed, so users should pick from the list rather than typing model ids by hand. While the catalog is loading the dropdown shows "Loading models…" and stays disabled instead of falling back to a placeholder text field. + +Show detailed embedding provider and Qdrant vector-store errors during indexing initialization failures, so failures include the exact response or dimension mismatch instead of only "Bad Request". diff --git a/packages/kilo-indexing/src/indexing/embedders/openai-compatible.ts b/packages/kilo-indexing/src/indexing/embedders/openai-compatible.ts index 5eb7500a6ae..57b770d383e 100644 --- a/packages/kilo-indexing/src/indexing/embedders/openai-compatible.ts +++ b/packages/kilo-indexing/src/indexing/embedders/openai-compatible.ts @@ -9,7 +9,7 @@ import { REMOTE_EMBEDDER_VALIDATION_TIMEOUT_MS, } from "../constants" import { getDefaultModelId, getModelQueryPrefix } from "../model-registry" -import { withValidationErrorHandling, type HttpError, formatEmbeddingError } from "../shared/validation-helpers" +import { withValidationErrorHandling, type HttpError } from "../shared/validation-helpers" import { Mutex } from "async-mutex" import { Log } from "../../util/log" @@ -28,6 +28,36 @@ interface OpenAIEmbeddingResponse { } } +type EmbeddingErrorDetail = { + status?: number + message: string + response?: unknown +} + +function json(value: unknown): string | undefined { + try { + return JSON.stringify(value) + } catch { + return undefined + } +} + +export function getEmbeddingErrorDetail(error: unknown): EmbeddingErrorDetail { + const err = error as { + status?: number + response?: { status?: number; data?: unknown; body?: unknown } + error?: unknown + message?: string + } + const response = err?.response?.data ?? err?.response?.body ?? err?.error + const msg = response ? json(response) : undefined + return { + status: err?.status ?? err?.response?.status, + message: msg ?? (error instanceof Error ? error.message : String(error)), + ...(response === undefined ? {} : { response }), + } +} + type OpenAICompatibleOptions = { headers?: Record dimensions?: number @@ -319,8 +349,14 @@ export class OpenAICompatibleEmbedder implements IEmbedder { }, } } catch (error) { + const detail = getEmbeddingErrorDetail(error) log.error("OpenAI Compatible embedder batch error", { - err: error instanceof Error ? error.message : String(error), + err: detail.message, + response: detail.response, + status: detail.status, + baseUrl: this.baseUrl, + model, + dimensions: this.dimensions, location: "OpenAICompatibleEmbedder:_embedBatchWithRetries", attempt: attempts + 1, }) @@ -345,8 +381,11 @@ export class OpenAICompatibleEmbedder implements IEmbedder { } } - // Format and throw the error - throw formatEmbeddingError(error, MAX_RETRIES) + throw new Error( + detail.status + ? `Embedding request failed after ${MAX_RETRIES} attempts with status ${detail.status}: ${detail.message}` + : `Embedding request failed after ${MAX_RETRIES} attempts: ${detail.message}`, + ) } } diff --git a/packages/kilo-indexing/src/indexing/vector-store/qdrant-client.ts b/packages/kilo-indexing/src/indexing/vector-store/qdrant-client.ts index 44b45240966..749d0dad0c6 100644 --- a/packages/kilo-indexing/src/indexing/vector-store/qdrant-client.ts +++ b/packages/kilo-indexing/src/indexing/vector-store/qdrant-client.ts @@ -18,6 +18,31 @@ const KEY = { const METADATA_ID = "f946a536-9af4-4f1f-9f95-7d6efb4647d5" +function json(value: unknown): string | undefined { + try { + return JSON.stringify(value) + } catch { + return undefined + } +} + +function qdrantErrorDetail(error: unknown) { + const err = error as { + status?: number + statusCode?: number + response?: { status?: number; data?: unknown; body?: unknown } + data?: unknown + body?: unknown + message?: string + } + const response = err?.response?.data ?? err?.response?.body ?? err?.data ?? err?.body + return { + status: err?.status ?? err?.statusCode ?? err?.response?.status, + message: json(response) ?? (error instanceof Error ? error.message : String(error)), + response, + } +} + /** * Qdrant implementation of the vector store interface */ @@ -416,6 +441,13 @@ export class QdrantVectorStore implements IVectorStore { }>, ): Promise { try { + const mismatch = points.find((point) => point.vector.length !== this.vectorSize) + if (mismatch) { + throw new Error( + `Qdrant vector dimension mismatch before upsert: expected ${this.vectorSize}, got ${mismatch.vector.length}`, + ) + } + const processedPoints = points.map((point) => { if (point.payload?.filePath) { const segments = point.payload.filePath.split(path.sep).filter(Boolean) @@ -439,8 +471,17 @@ export class QdrantVectorStore implements IVectorStore { wait: true, }) } catch (error) { - log.error("Failed to upsert points", { error }) - throw error + const detail = qdrantErrorDetail(error) + log.error("Failed to upsert points", { + error: detail.message, + response: detail.response, + status: detail.status, + collection: this.collectionName, + expectedVectorSize: this.vectorSize, + firstVectorSize: points[0]?.vector.length, + pointCount: points.length, + }) + throw new Error(`Qdrant upsert failed: ${detail.message}`) } } @@ -473,6 +514,12 @@ export class QdrantVectorStore implements IVectorStore { maxResults?: number, ): Promise { try { + if (queryVector.length !== this.vectorSize) { + throw new Error( + `Qdrant query vector dimension mismatch before search: expected ${this.vectorSize}, got ${queryVector.length}`, + ) + } + let filter: | { must: Array<{ key: string; match: { value: string } }> @@ -532,8 +579,17 @@ export class QdrantVectorStore implements IVectorStore { return filteredPoints as VectorStoreSearchResult[] } catch (error) { - log.error("Failed to search points", { error }) - throw error + const detail = qdrantErrorDetail(error) + log.error("Failed to search points", { + error: detail.message, + response: detail.response, + status: detail.status, + collection: this.collectionName, + expectedVectorSize: this.vectorSize, + queryVectorSize: queryVector.length, + directoryPrefix, + }) + throw new Error(`Qdrant search failed: ${detail.message}`) } } diff --git a/packages/kilo-indexing/test/kilocode/indexing/embedders/openai-compatible.test.ts b/packages/kilo-indexing/test/kilocode/indexing/embedders/openai-compatible.test.ts index 4bed6ea72ed..b87955c26f8 100644 --- a/packages/kilo-indexing/test/kilocode/indexing/embedders/openai-compatible.test.ts +++ b/packages/kilo-indexing/test/kilocode/indexing/embedders/openai-compatible.test.ts @@ -467,6 +467,26 @@ describe("OpenAICompatibleEmbedder", () => { ) }) + test("should surface exact OpenAI SDK response payloads for HTTP errors", async () => { + const testTexts = ["Hello world"] + const httpError = new Error("Bad Request") + ;(httpError as any).status = 400 + ;(httpError as any).response = { + status: 400, + data: { + object: "error", + message: "Provider returned error", + metadata: { + raw: '{"error":{"message":"encoding_format is not supported"}}', + }, + }, + } + + mockEmbeddingsCreate.mockRejectedValue(httpError) + + await expect(embedder.createEmbeddings(testTexts)).rejects.toThrow("encoding_format is not supported") + }) + test("should handle errors without status codes", async () => { const testTexts = ["Hello world"] const networkError = new Error("Network timeout") diff --git a/packages/kilo-indexing/test/kilocode/indexing/vector-store/qdrant-client.test.ts b/packages/kilo-indexing/test/kilocode/indexing/vector-store/qdrant-client.test.ts index 8a580bcde83..b6b77f71f6c 100644 --- a/packages/kilo-indexing/test/kilocode/indexing/vector-store/qdrant-client.test.ts +++ b/packages/kilo-indexing/test/kilocode/indexing/vector-store/qdrant-client.test.ts @@ -1092,6 +1092,10 @@ describe("QdrantVectorStore", () => { }) describe("upsertPoints", () => { + beforeEach(() => { + vectorStore = new QdrantVectorStore(mockWorkspacePath, mockQdrantUrl, 3, mockApiKey) + }) + test("should correctly call qdrantClient.upsert with processed points", async () => { const mockPoints = [ { @@ -1261,13 +1265,67 @@ describe("QdrantVectorStore", () => { const upsertError = new Error("Upsert failed") mockUpsert.mockRejectedValue(upsertError) - await expect(vectorStore.upsertPoints(mockPoints)).rejects.toThrow(upsertError) + await expect(vectorStore.upsertPoints(mockPoints)).rejects.toThrow("Qdrant upsert failed: Upsert failed") expect(mockUpsert).toHaveBeenCalledTimes(1) }) + + test("should fail before upsert when vectors do not match the configured dimension", async () => { + const mockPoints = [ + { + id: "test-id-1", + vector: [0.1, 0.2, 0.3, 0.4], + payload: { + filePath: "src/test.ts", + content: "test content", + startLine: 1, + endLine: 1, + }, + }, + ] + + await expect(vectorStore.upsertPoints(mockPoints)).rejects.toThrow( + "Qdrant vector dimension mismatch before upsert: expected 3, got 4", + ) + + expect(mockUpsert).not.toHaveBeenCalled() + }) + + test("should include Qdrant response details when upsert fails", async () => { + const mockPoints = [ + { + id: "test-id-1", + vector: [0.1, 0.2, 0.3], + payload: { + filePath: "src/test.ts", + content: "test content", + startLine: 1, + endLine: 1, + }, + }, + ] + const upsertError = new Error("Bad Request") as Error & { + status?: number + response?: { status: number; data: unknown } + } + upsertError.status = 400 + upsertError.response = { + status: 400, + data: { status: { error: "Wrong input: Vector dimension error: expected dim: 256, got 1536" } }, + } + mockUpsert.mockRejectedValue(upsertError) + + await expect(vectorStore.upsertPoints(mockPoints)).rejects.toThrow( + "Wrong input: Vector dimension error: expected dim: 256, got 1536", + ) + }) }) describe("search", () => { + beforeEach(() => { + vectorStore = new QdrantVectorStore(mockWorkspacePath, mockQdrantUrl, 3, mockApiKey) + }) + test("should correctly call qdrantClient.query and transform results", async () => { const queryVector = [0.1, 0.2, 0.3] const mockQdrantResults = { @@ -1557,11 +1615,39 @@ describe("QdrantVectorStore", () => { const queryError = new Error("Query failed") mockQuery.mockRejectedValue(queryError) - await expect(vectorStore.search(queryVector)).rejects.toThrow(queryError) + await expect(vectorStore.search(queryVector)).rejects.toThrow("Qdrant search failed: Query failed") expect(mockQuery).toHaveBeenCalledTimes(1) }) + test("should fail before search when query vector does not match the configured dimension", async () => { + const queryVector = [0.1, 0.2, 0.3, 0.4] + + await expect(vectorStore.search(queryVector)).rejects.toThrow( + "Qdrant query vector dimension mismatch before search: expected 3, got 4", + ) + + expect(mockQuery).not.toHaveBeenCalled() + }) + + test("should include Qdrant response details when query fails", async () => { + const queryVector = [0.1, 0.2, 0.3] + const queryError = new Error("Bad Request") as Error & { + status?: number + response?: { status: number; data: unknown } + } + queryError.status = 400 + queryError.response = { + status: 400, + data: { status: { error: "Wrong input: Vector dimension error: expected dim: 256, got 1536" } }, + } + mockQuery.mockRejectedValue(queryError) + + await expect(vectorStore.search(queryVector)).rejects.toThrow( + "Wrong input: Vector dimension error: expected dim: 256, got 1536", + ) + }) + test("should use constants DEFAULT_MAX_SEARCH_RESULTS and DEFAULT_SEARCH_MIN_SCORE correctly", async () => { const queryVector = [0.1, 0.2, 0.3] const mockQdrantResults = { points: [] } diff --git a/packages/kilo-vscode/src/KiloProvider.ts b/packages/kilo-vscode/src/KiloProvider.ts index cf3c33479e2..987d1188b0d 100644 --- a/packages/kilo-vscode/src/KiloProvider.ts +++ b/packages/kilo-vscode/src/KiloProvider.ts @@ -2152,10 +2152,29 @@ export class KiloProvider implements vscode.WebviewViewProvider, TelemetryProper } private async fetchAndSendKiloEmbeddingModels(): Promise { + // Serve a previously-cached non-empty catalog immediately so the webview + // never regresses to the empty fallback if a fresh fetch fails. + if (this.cachedKiloEmbeddingModelsMessage) { + this.postMessage(this.cachedKiloEmbeddingModelsMessage) + } const catalog = await fetchKiloEmbeddingModelCatalog() const message = { type: "kiloEmbeddingModelsLoaded", catalog } - this.cachedKiloEmbeddingModelsMessage = message - this.postMessage(message) + // Only cache when we got a real catalog. Caching an empty result poisons + // the cache and causes the webview to render the "provider/model" + // placeholder until a full reload, even though the next fetch would + // succeed. Webview-side retries will re-trigger this method until a real + // catalog arrives. + if (catalog.defaultModel && catalog.models.length > 0) { + this.cachedKiloEmbeddingModelsMessage = message + this.postMessage(message) + return + } + // No cache yet: still post the empty result so the webview can decide to + // retry. If we already had a non-empty cache, we already posted it above + // and don't want to clobber it with an empty payload. + if (!this.cachedKiloEmbeddingModelsMessage) { + this.postMessage(message) + } } /** diff --git a/packages/kilo-vscode/tests/unit/indexing-tab-kilo-no-textfield.test.ts b/packages/kilo-vscode/tests/unit/indexing-tab-kilo-no-textfield.test.ts new file mode 100644 index 00000000000..3dbf4a88eed --- /dev/null +++ b/packages/kilo-vscode/tests/unit/indexing-tab-kilo-no-textfield.test.ts @@ -0,0 +1,70 @@ +/** + * Regression: when provider is `kilo`, the IndexingTab must NOT show a + * free-text "Embedding model" override or a "Dimension" field. The Kilo + * embedding catalog is Cloud-managed: users pick a model from the Select + * (which determines the dimension server-side). A free-text TextField + * leaked through previously and rendered the literal placeholder + * "provider/model" whenever the catalog hadn't loaded yet. + * + * Source-level assertions (rather than full Solid render) keep this test + * cheap and stable across UI library upgrades. + */ + +import { describe, expect, it } from "bun:test" +import fs from "node:fs" +import path from "node:path" + +const SOURCE = fs.readFileSync( + path.resolve(import.meta.dir, "../../webview-ui/src/components/settings/IndexingTab.tsx"), + "utf8", +) + +describe("IndexingTab kilo branch", () => { + it("does not contain the legacy 'provider/model' placeholder fallback", () => { + expect(SOURCE).not.toContain('"provider/model"') + }) + + it("renders the Kilo model preset Select inside the kilo branch", () => { + // The Select for the catalog must reference kiloModels(). The kilo branch + // is the body of , so the + // Select must appear after that line. + const showIdx = SOURCE.indexOf('selectedProvider() === "kilo"') + expect(showIdx).toBeGreaterThan(-1) + + const after = SOURCE.slice(showIdx) + expect(after).toContain("options={kiloModels()}") + expect(after).toContain("settings.indexing.kiloModel.title") + }) + + it("places the free-text Embedding model TextField in the non-kilo fallback only", () => { + // The "Embedding model" row uses settings.indexing.model.title. It must + // appear only inside the `fallback={...}` of the kilo Show, not in the + // main branch. + const fallbackIdx = SOURCE.indexOf("fallback={") + expect(fallbackIdx).toBeGreaterThan(-1) + + const fallbackEnd = SOURCE.indexOf("}\n >", fallbackIdx) + expect(fallbackEnd).toBeGreaterThan(fallbackIdx) + + const fallbackBlock = SOURCE.slice(fallbackIdx, fallbackEnd) + expect(fallbackBlock).toContain("settings.indexing.model.title") + expect(fallbackBlock).toContain("settings.indexing.dimension.title") + expect(fallbackBlock).toContain("text-embedding-3-small") + + // The kilo branch (after the fallback) must NOT reference the override + // model or dimension rows. + const kiloBranch = SOURCE.slice(fallbackEnd) + expect(kiloBranch).not.toContain("settings.indexing.model.title") + expect(kiloBranch).not.toContain("settings.indexing.dimension.title") + }) + + it("disables the kilo Select while the catalog is empty so users cannot select stale state", () => { + expect(SOURCE).toContain("disabled={kiloModels().length === 0}") + }) + + it("does not send invalid null dimensions in the typed config payload", () => { + expect(SOURCE).toContain('if (next.provider === "kilo") delete next.dimension') + expect(SOURCE).not.toContain("dimension: null") + expect(SOURCE).not.toContain("dimension: undefined") + }) +}) diff --git a/packages/kilo-vscode/tests/unit/kilo-embedding-models-retry.test.ts b/packages/kilo-vscode/tests/unit/kilo-embedding-models-retry.test.ts new file mode 100644 index 00000000000..1f9510c0bd8 --- /dev/null +++ b/packages/kilo-vscode/tests/unit/kilo-embedding-models-retry.test.ts @@ -0,0 +1,204 @@ +/** + * Regression: when the Kilo gateway returns an empty embedding-model catalog + * (network/auth race on first webview boot), the IndexingTab fell back to the + * literal "provider/model" placeholder until a full webview reload. The fix + * is two-layered: + * + * 1. Webview-side retry: re-request the catalog while it stays empty, + * mirroring the indexing.tsx retry shape. + * 2. Extension-side: never replace a non-empty catalog with an empty one. + * + * These tests cover both layers via the pure helpers exposed for testing. + */ + +import { describe, expect, it } from "bun:test" +import { + EMPTY_KILO_EMBEDDING_MODEL_CATALOG, + type KiloEmbeddingModelCatalog, +} from "@kilocode/kilo-indexing/embedding-models" +import { + isEmptyKiloEmbeddingCatalog, + KILO_EMBEDDING_MAX_RETRIES, + subscribeKiloEmbeddingModels, +} from "../../webview-ui/src/context/kilo-embedding-models-subscribe" +import type { ExtensionMessage, WebviewMessage } from "../../webview-ui/src/types/messages" + +const realCatalog: KiloEmbeddingModelCatalog = { + defaultModel: "kilo/code-embedding-4k", + models: [{ id: "kilo/code-embedding-4k", name: "Code Embedding 4k", dimension: 1024, scoreThreshold: 0.4 }], + aliases: {}, +} + +type FakeTimer = { fn: () => void; ms: number } + +function createFakeTimers() { + const timers = new Map() + let next = 1 + const setIntervalFn = ((fn: () => void, ms: number) => { + const id = next++ + timers.set(id, { fn, ms }) + return id as unknown as ReturnType + }) as typeof setInterval + const clearIntervalFn = ((id: ReturnType) => { + timers.delete(id as unknown as number) + }) as typeof clearInterval + return { + setIntervalFn, + clearIntervalFn, + tick: () => { + // Snapshot to avoid mutation during iteration. + for (const t of [...timers.values()]) t.fn() + }, + pending: () => timers.size, + } +} + +function createHarness() { + const posted: WebviewMessage[] = [] + const handlers = new Set<(m: ExtensionMessage) => void>() + let stored: KiloEmbeddingModelCatalog = EMPTY_KILO_EMBEDDING_MODEL_CATALOG + return { + posted, + deliver: (m: ExtensionMessage) => handlers.forEach((h) => h(m)), + getCatalog: () => stored, + setCatalog: (next: KiloEmbeddingModelCatalog) => { + stored = next + }, + postMessage: (m: WebviewMessage) => { + posted.push(m) + }, + onMessage: (h: (m: ExtensionMessage) => void) => { + handlers.add(h) + return () => handlers.delete(h) + }, + } +} + +describe("isEmptyKiloEmbeddingCatalog", () => { + it("treats EMPTY_KILO_EMBEDDING_MODEL_CATALOG as empty", () => { + expect(isEmptyKiloEmbeddingCatalog(EMPTY_KILO_EMBEDDING_MODEL_CATALOG)).toBe(true) + }) + + it("treats a defaultModel-only payload as empty (models[] missing)", () => { + expect(isEmptyKiloEmbeddingCatalog({ defaultModel: "x", models: [], aliases: {} })).toBe(true) + }) + + it("treats a catalog with models but no defaultModel as empty", () => { + expect( + isEmptyKiloEmbeddingCatalog({ + defaultModel: "", + models: realCatalog.models, + aliases: {}, + }), + ).toBe(true) + }) + + it("recognises a real catalog as non-empty", () => { + expect(isEmptyKiloEmbeddingCatalog(realCatalog)).toBe(false) + }) +}) + +describe("subscribeKiloEmbeddingModels (webview retry)", () => { + it("posts the initial request immediately on subscribe", () => { + const h = createHarness() + const timers = createFakeTimers() + const cleanup = subscribeKiloEmbeddingModels({ + ...h, + setInterval: timers.setIntervalFn, + clearInterval: timers.clearIntervalFn, + }) + + expect(h.posted).toEqual([{ type: "requestKiloEmbeddingModels" }]) + cleanup() + }) + + it("retries while the catalog stays empty", () => { + const h = createHarness() + const timers = createFakeTimers() + const cleanup = subscribeKiloEmbeddingModels({ + ...h, + setInterval: timers.setIntervalFn, + clearInterval: timers.clearIntervalFn, + }) + + // Empty catalog "received" — should keep retrying. + h.deliver({ type: "kiloEmbeddingModelsLoaded", catalog: EMPTY_KILO_EMBEDDING_MODEL_CATALOG }) + + timers.tick() + timers.tick() + + expect(h.posted.length).toBe(3) // initial + 2 retries + cleanup() + }) + + it("stops retrying once a non-empty catalog arrives", () => { + const h = createHarness() + const timers = createFakeTimers() + const cleanup = subscribeKiloEmbeddingModels({ + ...h, + setInterval: timers.setIntervalFn, + clearInterval: timers.clearIntervalFn, + }) + + h.deliver({ type: "kiloEmbeddingModelsLoaded", catalog: realCatalog }) + + timers.tick() + timers.tick() + timers.tick() + + // Only the initial request — retries must short-circuit once we have a catalog. + expect(h.posted.length).toBe(1) + expect(h.getCatalog()).toEqual(realCatalog) + cleanup() + }) + + it("does not exceed the retry cap", () => { + const h = createHarness() + const timers = createFakeTimers() + const cleanup = subscribeKiloEmbeddingModels({ + ...h, + setInterval: timers.setIntervalFn, + clearInterval: timers.clearIntervalFn, + }) + + for (let i = 0; i < KILO_EMBEDDING_MAX_RETRIES + 5; i++) timers.tick() + + // Initial request + exactly MAX_RETRIES re-posts. + expect(h.posted.length).toBe(1 + KILO_EMBEDDING_MAX_RETRIES) + cleanup() + }) + + it("ignores empty catalogs delivered after a non-empty one (no regression to placeholder)", () => { + const h = createHarness() + const timers = createFakeTimers() + const cleanup = subscribeKiloEmbeddingModels({ + ...h, + setInterval: timers.setIntervalFn, + clearInterval: timers.clearIntervalFn, + }) + + h.deliver({ type: "kiloEmbeddingModelsLoaded", catalog: realCatalog }) + expect(h.getCatalog()).toEqual(realCatalog) + + // Late empty payload (e.g. another extension push). Must NOT clobber the + // good catalog or IndexingTab will fall back to "provider/model". + h.deliver({ type: "kiloEmbeddingModelsLoaded", catalog: EMPTY_KILO_EMBEDDING_MODEL_CATALOG }) + + expect(h.getCatalog()).toEqual(realCatalog) + cleanup() + }) + + it("clears the retry timer on cleanup", () => { + const h = createHarness() + const timers = createFakeTimers() + const cleanup = subscribeKiloEmbeddingModels({ + ...h, + setInterval: timers.setIntervalFn, + clearInterval: timers.clearIntervalFn, + }) + + expect(timers.pending()).toBe(1) + cleanup() + expect(timers.pending()).toBe(0) + }) +}) diff --git a/packages/kilo-vscode/tests/unit/kilo-provider-embedding-models-cache.test.ts b/packages/kilo-vscode/tests/unit/kilo-provider-embedding-models-cache.test.ts new file mode 100644 index 00000000000..99f39ff0a6a --- /dev/null +++ b/packages/kilo-vscode/tests/unit/kilo-provider-embedding-models-cache.test.ts @@ -0,0 +1,134 @@ +/** + * Regression: KiloProvider used to cache and post any catalog returned by the + * gateway, including the empty fallback that `fetchKiloEmbeddingModelCatalog` + * returns on transient failures. That poisoned the cache so subsequent + * `requestKiloEmbeddingModels` calls replayed the empty payload and + * IndexingTab kept showing the literal "provider/model" placeholder until a + * full webview reload. + * + * The fix: + * - Never cache an empty catalog. + * - When a non-empty catalog is already cached, replay it before re-fetching + * so a transient failure cannot regress the UI. + */ + +import { describe, expect, it } from "bun:test" + +// vscode mock is provided by the shared preload (tests/setup/vscode-mock.ts) +const { KiloProvider } = await import("../../src/KiloProvider") + +type Internals = { + webview: { postMessage: (message: unknown) => Promise } | null + cachedKiloEmbeddingModelsMessage: unknown + fetchAndSendKiloEmbeddingModels: () => Promise +} + +type Catalog = { + defaultModel: string + models: Array<{ id: string; name: string; dimension: number; scoreThreshold: number }> + aliases: Record +} + +const REAL: Catalog = { + defaultModel: "kilo/code-embedding-4k", + models: [{ id: "kilo/code-embedding-4k", name: "Code Embedding 4k", dimension: 1024, scoreThreshold: 0.4 }], + aliases: {}, +} + +const EMPTY: Catalog = { defaultModel: "", models: [], aliases: {} } + +function createProvider() { + const sent: Array<{ type: string; catalog?: Catalog }> = [] + const provider = new KiloProvider({} as never, {} as never) + const internal = provider as unknown as Internals + internal.webview = { + postMessage: async (message) => { + sent.push(message as { type: string; catalog?: Catalog }) + return true + }, + } + return { provider, internal, sent } +} + +function mockGatewayFetch(responses: Array) { + const original = globalThis.fetch + let i = 0 + globalThis.fetch = (async () => { + const next = responses[i++] ?? responses[responses.length - 1] + if (next === "fail") return new Response("nope", { status: 500 }) + return new Response(JSON.stringify(next), { status: 200, headers: { "content-type": "application/json" } }) + }) as typeof fetch + return () => { + globalThis.fetch = original + } +} + +describe("KiloProvider.fetchAndSendKiloEmbeddingModels", () => { + it("caches a real catalog and posts it to the webview", async () => { + const { internal, sent } = createProvider() + const restore = mockGatewayFetch([REAL]) + try { + await internal.fetchAndSendKiloEmbeddingModels() + } finally { + restore() + } + + expect(sent.length).toBe(1) + expect(sent[0]?.type).toBe("kiloEmbeddingModelsLoaded") + expect(sent[0]?.catalog).toEqual(REAL) + expect(internal.cachedKiloEmbeddingModelsMessage).toEqual({ type: "kiloEmbeddingModelsLoaded", catalog: REAL }) + }) + + it("does NOT cache an empty catalog (transient failure must not poison the cache)", async () => { + const { internal, sent } = createProvider() + const restore = mockGatewayFetch(["fail"]) + try { + await internal.fetchAndSendKiloEmbeddingModels() + } finally { + restore() + } + + // Empty payload still posted so the webview can decide to retry. + expect(sent.length).toBe(1) + expect(sent[0]?.catalog).toEqual(EMPTY) + // ...but the cache must stay null so the next request triggers a fresh + // fetch instead of replaying the empty response. + expect(internal.cachedKiloEmbeddingModelsMessage).toBeNull() + }) + + it("recovers on retry: empty fetch followed by a real fetch produces a cached real catalog", async () => { + const { internal, sent } = createProvider() + const restore = mockGatewayFetch(["fail", REAL]) + try { + await internal.fetchAndSendKiloEmbeddingModels() + await internal.fetchAndSendKiloEmbeddingModels() + } finally { + restore() + } + + // First call posts empty; second call posts real. + expect(sent.length).toBe(2) + expect(sent[0]?.catalog).toEqual(EMPTY) + expect(sent[1]?.catalog).toEqual(REAL) + expect(internal.cachedKiloEmbeddingModelsMessage).toEqual({ type: "kiloEmbeddingModelsLoaded", catalog: REAL }) + }) + + it("replays the cached real catalog before re-fetching, so a later transient failure cannot regress", async () => { + const { internal, sent } = createProvider() + const restore = mockGatewayFetch([REAL, "fail"]) + try { + await internal.fetchAndSendKiloEmbeddingModels() + sent.length = 0 // reset to inspect the second call only + await internal.fetchAndSendKiloEmbeddingModels() + } finally { + restore() + } + + // Second call: cached real catalog is replayed first. The fresh fetch + // returns empty but must NOT be posted (would clobber the good catalog + // in webview state) and must NOT overwrite the cache. + expect(sent.length).toBe(1) + expect(sent[0]?.catalog).toEqual(REAL) + expect(internal.cachedKiloEmbeddingModelsMessage).toEqual({ type: "kiloEmbeddingModelsLoaded", catalog: REAL }) + }) +}) diff --git a/packages/kilo-vscode/webview-ui/src/components/settings/IndexingTab.tsx b/packages/kilo-vscode/webview-ui/src/components/settings/IndexingTab.tsx index 60cd1199fbc..fe2825775de 100644 --- a/packages/kilo-vscode/webview-ui/src/components/settings/IndexingTab.tsx +++ b/packages/kilo-vscode/webview-ui/src/components/settings/IndexingTab.tsx @@ -92,7 +92,9 @@ const IndexingTab: Component = () => { const globalOn = createMemo(() => globalCfg().enabled === true) const updateIndexing = (partial: IndexingConfig) => { - updateConfig({ indexing: { ...cfg(), ...partial } }) + const next = { ...cfg(), ...partial } + if (next.provider === "kilo") delete next.dimension + updateConfig({ indexing: next }) } const vectorStore = () => cfg().vectorStore ?? "qdrant" @@ -117,11 +119,10 @@ const IndexingTab: Component = () => { updateIndexing({ provider: next, model, - dimension: undefined, }) return } - updateIndexing({ provider: next, model: undefined, dimension: undefined }) + updateIndexing({ provider: next, model: undefined }) } const saveEnabled = (enabled: boolean) => { @@ -264,47 +265,52 @@ const IndexingTab: Component = () => { placeholder={language.t("settings.providers.notSet")} /> - - 0}> - - item.value === knownKiloModel(cfg().model))} + value={(item) => item.value} + label={(item) => item.label} + onSelect={(item) => updateIndexing({ model: item?.value ?? kiloDefault() })} + variant="secondary" + size="small" + triggerVariant="settings" + disabled={kiloModels().length === 0} + placeholder={kiloModels().length === 0 ? "Loading models…" : "Custom model"} + /> + + + !cat.defaultModel || cat.models.length === 0 + +type SubscribeOptions = { + postMessage: (message: WebviewMessage) => void + onMessage: (handler: (message: ExtensionMessage) => void) => () => void + getCatalog: () => KiloEmbeddingModelCatalog + setCatalog: (next: KiloEmbeddingModelCatalog) => void + setInterval?: typeof globalThis.setInterval + clearInterval?: typeof globalThis.clearInterval + maxRetries?: number + retryMs?: number +} + +/** + * Pure orchestration: subscribes to `kiloEmbeddingModelsLoaded` messages, + * sends the initial request, and retries while the catalog is empty. Returned + * function cleans up the subscription and timer. + * + * Lives in a JSX-free module so unit tests can import it without pulling in + * the SolidJS render pipeline. + */ +export function subscribeKiloEmbeddingModels(opts: SubscribeOptions): () => void { + const setIntervalFn = opts.setInterval ?? globalThis.setInterval + const clearIntervalFn = opts.clearInterval ?? globalThis.clearInterval + const maxRetries = opts.maxRetries ?? KILO_EMBEDDING_MAX_RETRIES + const retryMs = opts.retryMs ?? KILO_EMBEDDING_RETRY_MS + + const unsubscribe = opts.onMessage((message: ExtensionMessage) => { + if (message.type !== "kiloEmbeddingModelsLoaded") return + // Never let an empty catalog overwrite a non-empty one that arrived + // earlier (e.g. cached message replayed after a transient failure). + if (isEmptyKiloEmbeddingCatalog(message.catalog) && !isEmptyKiloEmbeddingCatalog(opts.getCatalog())) return + opts.setCatalog(message.catalog) + }) + + opts.postMessage({ type: "requestKiloEmbeddingModels" }) + + let retries = 0 + const timer = setIntervalFn(() => { + if (!isEmptyKiloEmbeddingCatalog(opts.getCatalog()) || retries >= maxRetries) { + clearIntervalFn(timer) + return + } + retries++ + opts.postMessage({ type: "requestKiloEmbeddingModels" }) + if (retries >= maxRetries) clearIntervalFn(timer) + }, retryMs) + + return () => { + clearIntervalFn(timer) + unsubscribe() + } +} diff --git a/packages/kilo-vscode/webview-ui/src/context/kilo-embedding-models.tsx b/packages/kilo-vscode/webview-ui/src/context/kilo-embedding-models.tsx index 7403a049165..eba0663d682 100644 --- a/packages/kilo-vscode/webview-ui/src/context/kilo-embedding-models.tsx +++ b/packages/kilo-vscode/webview-ui/src/context/kilo-embedding-models.tsx @@ -4,7 +4,7 @@ import { type KiloEmbeddingModelCatalog, } from "@kilocode/kilo-indexing/embedding-models" import { useVSCode } from "./vscode" -import type { ExtensionMessage } from "../types/messages" +import { subscribeKiloEmbeddingModels } from "./kilo-embedding-models-subscribe" type KiloEmbeddingModelsContextValue = { catalog: Accessor @@ -16,14 +16,14 @@ export const KiloEmbeddingModelsProvider: ParentComponent = (props) => { const vscode = useVSCode() const [catalog, setCatalog] = createSignal(EMPTY_KILO_EMBEDDING_MODEL_CATALOG) - const unsubscribe = vscode.onMessage((message: ExtensionMessage) => { - if (message.type !== "kiloEmbeddingModelsLoaded") return - setCatalog(message.catalog) + const cleanup = subscribeKiloEmbeddingModels({ + postMessage: vscode.postMessage, + onMessage: vscode.onMessage, + getCatalog: catalog, + setCatalog, }) - vscode.postMessage({ type: "requestKiloEmbeddingModels" }) - - onCleanup(unsubscribe) + onCleanup(cleanup) return {props.children} }