mirror of
https://github.com/giancarloerra/socraticode.git
synced 2026-07-03 14:05:21 +02:00
fix: only call ensureOllamaReady when using Ollama provider (#8)
ensureOllamaReady() was called unconditionally on every search and context operation, causing failures for OpenAI and Google embedding users. Now only called when embeddingProvider is ollama; otherwise the configured provider is initialized via getEmbeddingProvider(). Fixes #7 Co-authored-by: pineapplestrikesback <pineapplestrikesback@users.noreply.github.com>
This commit is contained in:
@@ -11,6 +11,8 @@ import {
|
||||
searchArtifacts,
|
||||
} from "../services/context-artifacts.js";
|
||||
import { ensureQdrantReady } from "../services/docker.js";
|
||||
import { getEmbeddingConfig } from "../services/embedding-config.js";
|
||||
import { getEmbeddingProvider } from "../services/embedding-provider.js";
|
||||
import { isIndexingInProgress } from "../services/indexer.js";
|
||||
import { logger } from "../services/logger.js";
|
||||
import { ensureOllamaReady } from "../services/ollama.js";
|
||||
@@ -87,7 +89,13 @@ export async function handleContextTool(
|
||||
|
||||
case "codebase_context_index": {
|
||||
await ensureQdrantReady();
|
||||
await ensureOllamaReady();
|
||||
// Only ensure Ollama infrastructure when using the Ollama embedding provider.
|
||||
// For OpenAI/Google providers, just ensure the provider is initialized.
|
||||
if (getEmbeddingConfig().embeddingProvider === "ollama") {
|
||||
await ensureOllamaReady();
|
||||
} else {
|
||||
await getEmbeddingProvider();
|
||||
}
|
||||
|
||||
const config = await loadConfig(resolvedPath);
|
||||
if (!config?.artifacts?.length) {
|
||||
@@ -121,7 +129,13 @@ export async function handleContextTool(
|
||||
|
||||
case "codebase_context_search": {
|
||||
await ensureQdrantReady();
|
||||
await ensureOllamaReady();
|
||||
// Only ensure Ollama infrastructure when using the Ollama embedding provider.
|
||||
// For OpenAI/Google providers, just ensure the provider is initialized.
|
||||
if (getEmbeddingConfig().embeddingProvider === "ollama") {
|
||||
await ensureOllamaReady();
|
||||
} else {
|
||||
await getEmbeddingProvider();
|
||||
}
|
||||
|
||||
const query = args.query as string;
|
||||
if (!query?.trim()) {
|
||||
|
||||
@@ -6,6 +6,8 @@ import { SEARCH_DEFAULT_LIMIT, SEARCH_MIN_SCORE } from "../constants.js";
|
||||
import { getGraphStatus } from "../services/code-graph.js";
|
||||
import { getArtifactStatusSummary } from "../services/context-artifacts.js";
|
||||
import { ensureQdrantReady } from "../services/docker.js";
|
||||
import { getEmbeddingConfig } from "../services/embedding-config.js";
|
||||
import { getEmbeddingProvider } from "../services/embedding-provider.js";
|
||||
import type { IndexingProgress } from "../services/indexer.js";
|
||||
import { getIndexingProgress, getLastCompleted, isIndexingInProgress } from "../services/indexer.js";
|
||||
import { getLockHolderPid, } from "../services/lock.js";
|
||||
@@ -57,7 +59,13 @@ export async function handleQueryTool(
|
||||
switch (name) {
|
||||
case "codebase_search": {
|
||||
await ensureQdrantReady();
|
||||
await ensureOllamaReady();
|
||||
// Only ensure Ollama infrastructure when using the Ollama embedding provider.
|
||||
// For OpenAI/Google providers, just ensure the provider is initialized.
|
||||
if (getEmbeddingConfig().embeddingProvider === "ollama") {
|
||||
await ensureOllamaReady();
|
||||
} else {
|
||||
await getEmbeddingProvider();
|
||||
}
|
||||
|
||||
const query = args.query as string;
|
||||
const limit = (args.limit as number) || SEARCH_DEFAULT_LIMIT;
|
||||
|
||||
@@ -0,0 +1,192 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-only
|
||||
// Copyright (C) 2026 Giancarlo Erra - Altaire Limited
|
||||
|
||||
/**
|
||||
* Unit tests for the ensureOllamaReady conditional guard in context-tools.ts.
|
||||
* Verifies that codebase_context_index and codebase_context_search only call
|
||||
* ensureOllamaReady() for the Ollama provider, and use getEmbeddingProvider()
|
||||
* for OpenAI/Google.
|
||||
*/
|
||||
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// ── Mocks ────────────────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/logger.js", () => ({
|
||||
logger: {
|
||||
debug: vi.fn(),
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
// ── embedding-config.js mock ─────────────────────────────────────────────
|
||||
|
||||
const mockGetEmbeddingConfig = vi.fn(() => ({
|
||||
embeddingProvider: "ollama" as string,
|
||||
embeddingModel: "test-model",
|
||||
}));
|
||||
|
||||
vi.mock("../../src/services/embedding-config.js", () => ({
|
||||
getEmbeddingConfig: (...args: unknown[]) => mockGetEmbeddingConfig(...(args as [])),
|
||||
}));
|
||||
|
||||
// ── embedding-provider.js mock ───────────────────────────────────────────
|
||||
|
||||
const mockGetEmbeddingProvider = vi.fn(async () => ({
|
||||
embed: vi.fn(),
|
||||
ensureReady: vi.fn(async () => ({ imagePulled: false, containerStarted: false, modelPulled: false })),
|
||||
health: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../../src/services/embedding-provider.js", () => ({
|
||||
getEmbeddingProvider: (...args: unknown[]) => mockGetEmbeddingProvider(...(args as [])),
|
||||
}));
|
||||
|
||||
// ── ollama.js mock ───────────────────────────────────────────────────────
|
||||
|
||||
const mockEnsureOllamaReady = vi.fn(async () => ({
|
||||
modelPulled: false,
|
||||
containerStarted: false,
|
||||
imagePulled: false,
|
||||
}));
|
||||
|
||||
vi.mock("../../src/services/ollama.js", () => ({
|
||||
ensureOllamaReady: (...args: unknown[]) => mockEnsureOllamaReady(...(args as [])),
|
||||
}));
|
||||
|
||||
// ── docker.js mock ───────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/docker.js", () => ({
|
||||
ensureQdrantReady: vi.fn(async () => ({ pulled: false, started: false })),
|
||||
isDockerAvailable: vi.fn(async () => true),
|
||||
}));
|
||||
|
||||
// ── qdrant.js mock ───────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/qdrant.js", () => ({
|
||||
getCollectionInfo: vi.fn(async () => null),
|
||||
loadContextMetadata: vi.fn(async () => null),
|
||||
}));
|
||||
|
||||
// ── config.js mock ───────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/config.js", () => ({
|
||||
projectIdFromPath: vi.fn(() => "test-project-id"),
|
||||
contextCollectionName: vi.fn(() => "context_test"),
|
||||
}));
|
||||
|
||||
// ── indexer.js mock ──────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/indexer.js", () => ({
|
||||
isIndexingInProgress: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
// ── context-artifacts.js mock ───────────────────────────────────────────
|
||||
|
||||
const mockIndexAllArtifacts = vi.fn(async (_path: string) => ({ indexed: [], errors: [] }));
|
||||
const mockSearchArtifacts = vi.fn(async (_collection: string, _query: string, _limit: number) => []);
|
||||
|
||||
vi.mock("../../src/services/context-artifacts.js", () => ({
|
||||
loadConfig: vi.fn(async () => ({
|
||||
artifacts: [{ name: "test", type: "file", path: "test.md" }],
|
||||
})),
|
||||
indexAllArtifacts: (...args: unknown[]) => mockIndexAllArtifacts(...(args as [string])),
|
||||
searchArtifacts: (...args: unknown[]) => mockSearchArtifacts(...(args as [string, string, number])),
|
||||
ensureArtifactsIndexed: vi.fn(async () => ({ reindexed: [], upToDate: [], errors: [] })),
|
||||
removeAllArtifacts: vi.fn(async () => {}),
|
||||
}));
|
||||
|
||||
// ── Imports (after mocks) ────────────────────────────────────────────────
|
||||
|
||||
import { handleContextTool } from "../../src/tools/context-tools.js";
|
||||
|
||||
// ── Tests ────────────────────────────────────────────────────────────────
|
||||
|
||||
const TEST_PATH = "/tmp/test-project";
|
||||
|
||||
describe("codebase_context_index — embedding provider readiness guard", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("calls ensureOllamaReady when embeddingProvider is ollama", async () => {
|
||||
mockGetEmbeddingConfig.mockReturnValue({
|
||||
embeddingProvider: "ollama",
|
||||
embeddingModel: "test-model",
|
||||
});
|
||||
|
||||
await handleContextTool("codebase_context_index", {
|
||||
projectPath: TEST_PATH,
|
||||
});
|
||||
|
||||
expect(mockEnsureOllamaReady).toHaveBeenCalledOnce();
|
||||
expect(mockGetEmbeddingProvider).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("calls getEmbeddingProvider (not ensureOllamaReady) when embeddingProvider is openai", async () => {
|
||||
mockGetEmbeddingConfig.mockReturnValue({
|
||||
embeddingProvider: "openai",
|
||||
embeddingModel: "text-embedding-3-small",
|
||||
});
|
||||
|
||||
await handleContextTool("codebase_context_index", {
|
||||
projectPath: TEST_PATH,
|
||||
});
|
||||
|
||||
expect(mockEnsureOllamaReady).not.toHaveBeenCalled();
|
||||
expect(mockGetEmbeddingProvider).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
||||
describe("codebase_context_search — embedding provider readiness guard", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("calls ensureOllamaReady when embeddingProvider is ollama", async () => {
|
||||
mockGetEmbeddingConfig.mockReturnValue({
|
||||
embeddingProvider: "ollama",
|
||||
embeddingModel: "test-model",
|
||||
});
|
||||
|
||||
await handleContextTool("codebase_context_search", {
|
||||
projectPath: TEST_PATH,
|
||||
query: "test query",
|
||||
});
|
||||
|
||||
expect(mockEnsureOllamaReady).toHaveBeenCalledOnce();
|
||||
expect(mockGetEmbeddingProvider).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("calls getEmbeddingProvider (not ensureOllamaReady) when embeddingProvider is openai", async () => {
|
||||
mockGetEmbeddingConfig.mockReturnValue({
|
||||
embeddingProvider: "openai",
|
||||
embeddingModel: "text-embedding-3-small",
|
||||
});
|
||||
|
||||
await handleContextTool("codebase_context_search", {
|
||||
projectPath: TEST_PATH,
|
||||
query: "test query",
|
||||
});
|
||||
|
||||
expect(mockEnsureOllamaReady).not.toHaveBeenCalled();
|
||||
expect(mockGetEmbeddingProvider).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("calls getEmbeddingProvider (not ensureOllamaReady) when embeddingProvider is google", async () => {
|
||||
mockGetEmbeddingConfig.mockReturnValue({
|
||||
embeddingProvider: "google",
|
||||
embeddingModel: "gemini-embedding-001",
|
||||
});
|
||||
|
||||
await handleContextTool("codebase_context_search", {
|
||||
projectPath: TEST_PATH,
|
||||
query: "test query",
|
||||
});
|
||||
|
||||
expect(mockEnsureOllamaReady).not.toHaveBeenCalled();
|
||||
expect(mockGetEmbeddingProvider).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,180 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-only
|
||||
// Copyright (C) 2026 Giancarlo Erra - Altaire Limited
|
||||
|
||||
/**
|
||||
* Unit tests for the ensureOllamaReady conditional guard in query-tools.ts.
|
||||
* Verifies that codebase_search only calls ensureOllamaReady() for the Ollama
|
||||
* provider, and uses getEmbeddingProvider() for OpenAI/Google.
|
||||
*/
|
||||
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
// ── Mocks ────────────────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/logger.js", () => ({
|
||||
logger: {
|
||||
debug: vi.fn(),
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
// ── embedding-config.js mock ─────────────────────────────────────────────
|
||||
|
||||
const mockGetEmbeddingConfig = vi.fn(() => ({
|
||||
embeddingProvider: "ollama" as string,
|
||||
embeddingModel: "test-model",
|
||||
}));
|
||||
|
||||
vi.mock("../../src/services/embedding-config.js", () => ({
|
||||
getEmbeddingConfig: (...args: unknown[]) => mockGetEmbeddingConfig(...(args as [])),
|
||||
}));
|
||||
|
||||
// ── embedding-provider.js mock ───────────────────────────────────────────
|
||||
|
||||
const mockGetEmbeddingProvider = vi.fn(async () => ({
|
||||
embed: vi.fn(),
|
||||
ensureReady: vi.fn(async () => ({ imagePulled: false, containerStarted: false, modelPulled: false })),
|
||||
health: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("../../src/services/embedding-provider.js", () => ({
|
||||
getEmbeddingProvider: (...args: unknown[]) => mockGetEmbeddingProvider(...(args as [])),
|
||||
}));
|
||||
|
||||
// ── ollama.js mock ───────────────────────────────────────────────────────
|
||||
|
||||
const mockEnsureOllamaReady = vi.fn(async () => ({
|
||||
modelPulled: false,
|
||||
containerStarted: false,
|
||||
imagePulled: false,
|
||||
}));
|
||||
|
||||
vi.mock("../../src/services/ollama.js", () => ({
|
||||
ensureOllamaReady: (...args: unknown[]) => mockEnsureOllamaReady(...(args as [])),
|
||||
}));
|
||||
|
||||
// ── docker.js mock ───────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/docker.js", () => ({
|
||||
ensureQdrantReady: vi.fn(async () => ({ pulled: false, started: false })),
|
||||
isDockerAvailable: vi.fn(async () => true),
|
||||
}));
|
||||
|
||||
// ── qdrant.js mock ───────────────────────────────────────────────────────
|
||||
|
||||
const mockSearchChunks = vi.fn(async (_collection: string, _query: string, _limit: number) => []);
|
||||
|
||||
vi.mock("../../src/services/qdrant.js", () => ({
|
||||
searchChunks: (...args: unknown[]) => mockSearchChunks(...(args as [string, string, number])),
|
||||
getCollectionInfo: vi.fn(async () => ({ points_count: 0 })),
|
||||
getProjectMetadata: vi.fn(async () => null),
|
||||
}));
|
||||
|
||||
// ── config.js mock ───────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/config.js", () => ({
|
||||
collectionName: vi.fn(() => "test-collection"),
|
||||
projectIdFromPath: vi.fn(() => "test-project-id"),
|
||||
}));
|
||||
|
||||
// ── indexer.js mock ──────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/indexer.js", () => ({
|
||||
isIndexingInProgress: vi.fn(() => false),
|
||||
getIndexingProgress: vi.fn(() => null),
|
||||
getLastCompleted: vi.fn(() => null),
|
||||
}));
|
||||
|
||||
// ── code-graph.js mock ──────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/code-graph.js", () => ({
|
||||
getGraphStatus: vi.fn(async () => null),
|
||||
}));
|
||||
|
||||
// ── context-artifacts.js mock ───────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/context-artifacts.js", () => ({
|
||||
getArtifactStatusSummary: vi.fn(async () => null),
|
||||
}));
|
||||
|
||||
// ── watcher.js mock ──────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/watcher.js", () => ({
|
||||
ensureWatcherStarted: vi.fn(),
|
||||
isWatching: vi.fn(() => false),
|
||||
isWatchedByAnyProcess: vi.fn(async () => false),
|
||||
}));
|
||||
|
||||
// ── lock.js mock ─────────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/services/lock.js", () => ({
|
||||
getLockHolderPid: vi.fn(async () => null),
|
||||
}));
|
||||
|
||||
// ── constants.js mock ────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("../../src/constants.js", async (importOriginal) => {
|
||||
const original = await importOriginal<Record<string, unknown>>();
|
||||
return { ...original };
|
||||
});
|
||||
|
||||
// ── Imports (after mocks) ────────────────────────────────────────────────
|
||||
|
||||
import { handleQueryTool } from "../../src/tools/query-tools.js";
|
||||
|
||||
// ── Tests ────────────────────────────────────────────────────────────────
|
||||
|
||||
const TEST_PATH = "/tmp/test-project";
|
||||
|
||||
describe("codebase_search — embedding provider readiness guard", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("calls ensureOllamaReady when embeddingProvider is ollama", async () => {
|
||||
mockGetEmbeddingConfig.mockReturnValue({
|
||||
embeddingProvider: "ollama",
|
||||
embeddingModel: "test-model",
|
||||
});
|
||||
|
||||
await handleQueryTool("codebase_search", {
|
||||
projectPath: TEST_PATH,
|
||||
query: "test query",
|
||||
});
|
||||
|
||||
expect(mockEnsureOllamaReady).toHaveBeenCalledOnce();
|
||||
expect(mockGetEmbeddingProvider).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("calls getEmbeddingProvider (not ensureOllamaReady) when embeddingProvider is openai", async () => {
|
||||
mockGetEmbeddingConfig.mockReturnValue({
|
||||
embeddingProvider: "openai",
|
||||
embeddingModel: "text-embedding-3-small",
|
||||
});
|
||||
|
||||
await handleQueryTool("codebase_search", {
|
||||
projectPath: TEST_PATH,
|
||||
query: "test query",
|
||||
});
|
||||
|
||||
expect(mockEnsureOllamaReady).not.toHaveBeenCalled();
|
||||
expect(mockGetEmbeddingProvider).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("calls getEmbeddingProvider (not ensureOllamaReady) when embeddingProvider is google", async () => {
|
||||
mockGetEmbeddingConfig.mockReturnValue({
|
||||
embeddingProvider: "google",
|
||||
embeddingModel: "gemini-embedding-001",
|
||||
});
|
||||
|
||||
await handleQueryTool("codebase_search", {
|
||||
projectPath: TEST_PATH,
|
||||
query: "test query",
|
||||
});
|
||||
|
||||
expect(mockEnsureOllamaReady).not.toHaveBeenCalled();
|
||||
expect(mockGetEmbeddingProvider).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user