From cee166d71cbf2e0674c9101273fb3a947ab9ccb3 Mon Sep 17 00:00:00 2001 From: Tony Casey Date: Sun, 15 Feb 2026 22:18:15 +0000 Subject: [PATCH 1/6] feat: add multi-provider LLM enrichment support (GIT-110) Extract BaseLLMClient abstract class with shared logic (message building, response parsing, fact validation, enrichCommit orchestration). Add OpenAI, Gemini, and Ollama provider implementations alongside existing Anthropic. - Add ILLMCaller interface for provider-agnostic simple completions - Refactor IntentExtractor to use ILLMCaller instead of Anthropic SDK directly - Expand LLMClientFactory with auto-detection priority chain - Add ILLMConfig to hook config for YAML-based provider configuration - Wire config-derived LLM options through DI container - Add openai and @google/generative-ai as optional peer dependencies Co-Authored-By: Claude Opus 4.6 AI-Agent: Claude-Code/2.1.42 --- .env.example | 24 +- CLAUDE.md | 17 +- package-lock.json | 4 +- package.json | 8 + src/domain/interfaces/IHookConfig.ts | 17 +- src/domain/interfaces/ILLMCaller.ts | 25 ++ src/hooks/utils/config.ts | 27 +- src/infrastructure/di/container.ts | 11 +- src/infrastructure/di/types.ts | 2 + src/infrastructure/llm/AnthropicLLMClient.ts | 194 ++--------- src/infrastructure/llm/BaseLLMClient.ts | 205 +++++++++++ src/infrastructure/llm/GeminiLLMClient.ts | 78 +++++ src/infrastructure/llm/IntentExtractor.ts | 62 ++-- src/infrastructure/llm/LLMClientFactory.ts | 79 ++++- src/infrastructure/llm/OllamaLLMClient.ts | 107 ++++++ src/infrastructure/llm/OpenAILLMClient.ts | 79 +++++ tests/unit/hooks/utils/config.test.ts | 68 ++++ .../llm/AnthropicLLMClient.test.ts | 230 +------------ .../infrastructure/llm/BaseLLMClient.test.ts | 321 ++++++++++++++++++ .../llm/GeminiLLMClient.test.ts | 56 +++ .../llm/IntentExtractor.test.ts | 218 ++++++++---- .../llm/LLMClientFactory.test.ts | 152 ++++++++- .../llm/OllamaLLMClient.test.ts | 44 +++ .../llm/OpenAILLMClient.test.ts | 38 +++ 24 files changed, 1538 insertions(+), 528 deletions(-) create mode 100644 src/domain/interfaces/ILLMCaller.ts create mode 100644 src/infrastructure/llm/BaseLLMClient.ts create mode 100644 src/infrastructure/llm/GeminiLLMClient.ts create mode 100644 src/infrastructure/llm/OllamaLLMClient.ts create mode 100644 src/infrastructure/llm/OpenAILLMClient.ts create mode 100644 tests/unit/infrastructure/llm/BaseLLMClient.test.ts create mode 100644 tests/unit/infrastructure/llm/GeminiLLMClient.test.ts create mode 100644 tests/unit/infrastructure/llm/OllamaLLMClient.test.ts create mode 100644 tests/unit/infrastructure/llm/OpenAILLMClient.test.ts diff --git a/.env.example b/.env.example index 411a1429..dfba7229 100644 --- a/.env.example +++ b/.env.example @@ -1,4 +1,24 @@ -# LLM enrichment for extract (optional) -# Required only when using `git mem extract --enrich` +# LLM enrichment for extract and intent extraction (optional) +# git-mem auto-detects the provider from whichever key is set. +# Only one provider is needed. + +# Anthropic (Claude) — default provider # Get your key at: https://console.anthropic.com/ ANTHROPIC_API_KEY= + +# OpenAI (GPT) — requires: npm install openai +# Get your key at: https://platform.openai.com/api-keys +OPENAI_API_KEY= + +# Google Gemini — requires: npm install @google/generative-ai +# Get your key at: https://aistudio.google.com/apikey +GOOGLE_API_KEY= +# Alternative env var name: +# GEMINI_API_KEY= + +# Ollama (local) — no API key needed, no extra package +# Just set the host if not using the default localhost:11434 +# OLLAMA_HOST=http://localhost:11434 + +# Force a specific provider (overrides auto-detection) +# GIT_MEM_LLM_PROVIDER=anthropic diff --git a/CLAUDE.md b/CLAUDE.md index da13e8a7..095ec479 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,7 +65,22 @@ Uses **`node:test`** (native Node.js test runner) with **`tsx`** for TypeScript, ## Environment Variables -- `ANTHROPIC_API_KEY` — Required only for `git mem extract --enrich` (LLM enrichment). Without it, `--enrich` falls back to heuristic extraction with a warning. See `.env.example`. +- `ANTHROPIC_API_KEY` — Anthropic (Claude) API key for LLM enrichment and intent extraction. +- `OPENAI_API_KEY` — OpenAI API key (requires `npm install openai`). +- `GOOGLE_API_KEY` / `GEMINI_API_KEY` — Google Gemini API key (requires `npm install @google/generative-ai`). +- `OLLAMA_HOST` — Ollama server URL (default: `http://localhost:11434`). No extra package needed. +- `GIT_MEM_LLM_PROVIDER` — Force a specific provider: `anthropic`, `openai`, `gemini`, or `ollama`. Auto-detected from API keys if omitted. + +Only one provider is needed. Without any LLM key, `--enrich` falls back to heuristic extraction with a warning. See `.env.example`. + +**LLM config in `.git-mem/.git-mem.yaml`:** +```yaml +llm: + provider: openai # auto-detected if omitted + model: gpt-4o # provider default if omitted + intentModel: gpt-4o-mini + baseUrl: http://localhost:11434 # for ollama +``` ## Key Technical Details diff --git a/package-lock.json b/package-lock.json index e27f4060..3b349ea5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "git-mem", - "version": "0.3.0", + "version": "0.4.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "git-mem", - "version": "0.3.0", + "version": "0.4.0", "license": "MIT", "dependencies": { "@anthropic-ai/sdk": "^0.73.0", diff --git a/package.json b/package.json index f6f51931..cd5e74e1 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,14 @@ "prompts": "^2.4.2", "yaml": "^2.8.2" }, + "peerDependencies": { + "openai": ">=4.0.0", + "@google/generative-ai": ">=0.21.0" + }, + "peerDependenciesMeta": { + "openai": { "optional": true }, + "@google/generative-ai": { "optional": true } + }, "devDependencies": { "@semantic-release/changelog": "^6.0.3", "@semantic-release/git": "^10.0.1", diff --git a/src/domain/interfaces/IHookConfig.ts b/src/domain/interfaces/IHookConfig.ts index 7b767db1..3003523c 100644 --- a/src/domain/interfaces/IHookConfig.ts +++ b/src/domain/interfaces/IHookConfig.ts @@ -55,7 +55,7 @@ export interface ICommitMsgConfig { readonly requireType: boolean; /** Default memory lifecycle. */ readonly defaultLifecycle: 'permanent' | 'project' | 'session'; - /** Enable LLM enrichment for richer trailer content. Requires ANTHROPIC_API_KEY. */ + /** Enable LLM enrichment for richer trailer content. Requires an LLM provider API key. */ readonly enrich: boolean; /** Timeout in ms for LLM enrichment call. Default: 8000. Must be under hook timeout (10s). */ readonly enrichTimeout: number; @@ -70,6 +70,21 @@ export interface IHooksConfig { readonly commitMsg: ICommitMsgConfig; } +/** Supported LLM providers. */ +export type LLMProvider = 'anthropic' | 'openai' | 'gemini' | 'ollama'; + +export interface ILLMConfig { + /** Explicit provider selection. Auto-detected from env if omitted. */ + readonly provider?: LLMProvider; + /** Model for enrichment. Provider default if omitted. */ + readonly model?: string; + /** Lighter model for intent extraction. Falls back to model if omitted. */ + readonly intentModel?: string; + /** Base URL override (e.g., for Ollama). */ + readonly baseUrl?: string; +} + export interface IHookConfig { readonly hooks: IHooksConfig; + readonly llm?: ILLMConfig; } diff --git a/src/domain/interfaces/ILLMCaller.ts b/src/domain/interfaces/ILLMCaller.ts new file mode 100644 index 00000000..ff7dc23a --- /dev/null +++ b/src/domain/interfaces/ILLMCaller.ts @@ -0,0 +1,25 @@ +/** + * ILLMCaller + * + * Generic LLM completion interface used by services that need simple + * text-in/text-out LLM calls (e.g., IntentExtractor). + * Implemented by BaseLLMClient so any provider can serve as a caller. + */ + +export interface ILLMCallerOptions { + /** System prompt for the LLM. */ + readonly system: string; + /** User message to send. */ + readonly userMessage: string; + /** Maximum tokens for the response. */ + readonly maxTokens: number; +} + +export interface ILLMCaller { + /** + * Send a simple completion request to the LLM. + * @param options - System prompt, user message, and token limit. + * @returns The LLM's text response. + */ + complete(options: ILLMCallerOptions): Promise; +} diff --git a/src/hooks/utils/config.ts b/src/hooks/utils/config.ts index 125a9f6c..6ed8cf5c 100644 --- a/src/hooks/utils/config.ts +++ b/src/hooks/utils/config.ts @@ -9,7 +9,7 @@ import { existsSync, readFileSync } from 'fs'; import { join } from 'path'; import { parse as parseYaml } from 'yaml'; -import type { IHookConfig, IHooksConfig } from '../../domain/interfaces/IHookConfig'; +import type { IHookConfig, IHooksConfig, ILLMConfig, LLMProvider } from '../../domain/interfaces/IHookConfig'; /** Directory containing git-mem configuration */ export const CONFIG_DIR = '.git-mem'; @@ -78,7 +78,7 @@ export function loadHookConfig(cwd?: string): IHookConfig { const rawStop = (rawHooks.sessionStop ?? {}) as Record; - return { + const result: IHookConfig = { hooks: { enabled: rawHooks.enabled ?? DEFAULTS.hooks.enabled, sessionStart: { @@ -103,7 +103,30 @@ export function loadHookConfig(cwd?: string): IHookConfig { }, }, }; + + // Parse llm section if present + const rawLlm = raw.llm as Record | undefined; + if (rawLlm && typeof rawLlm === 'object') { + const llmConfig: ILLMConfig = { + ...(isValidProvider(rawLlm.provider) ? { provider: rawLlm.provider } : {}), + ...(typeof rawLlm.model === 'string' ? { model: rawLlm.model } : {}), + ...(typeof rawLlm.intentModel === 'string' ? { intentModel: rawLlm.intentModel } : {}), + ...(typeof rawLlm.baseUrl === 'string' ? { baseUrl: rawLlm.baseUrl } : {}), + }; + // Only attach if at least one field was parsed + if (Object.keys(llmConfig).length > 0) { + return { ...result, llm: llmConfig }; + } + } + + return result; } catch { return DEFAULTS; } } + +const VALID_PROVIDERS: readonly LLMProvider[] = ['anthropic', 'openai', 'gemini', 'ollama']; + +function isValidProvider(value: unknown): value is LLMProvider { + return typeof value === 'string' && VALID_PROVIDERS.includes(value as LLMProvider); +} diff --git a/src/infrastructure/di/container.ts b/src/infrastructure/di/container.ts index 0aa43be5..f917e3d2 100644 --- a/src/infrastructure/di/container.ts +++ b/src/infrastructure/di/container.ts @@ -110,18 +110,19 @@ export function createContainer(options?: IContainerOptions): AwilixContainer { - // Intent extraction requires an API key. Return null for graceful degradation. - const apiKey = process.env.ANTHROPIC_API_KEY; - if (!apiKey) { + // Intent extraction uses the LLM client as ILLMCaller. + // If no LLM client is available, return null for graceful degradation. + const client = container.cradle.llmClient; + if (!client) { return null; } try { return new IntentExtractor({ - apiKey, + caller: client as unknown as import('../../domain/interfaces/ILLMCaller').ILLMCaller, logger: container.cradle.logger, }); } catch { diff --git a/src/infrastructure/di/types.ts b/src/infrastructure/di/types.ts index f16ab6ce..73fc31de 100644 --- a/src/infrastructure/di/types.ts +++ b/src/infrastructure/di/types.ts @@ -64,4 +64,6 @@ export interface IContainerOptions { enrich?: boolean; /** Scope label for child logger (e.g., 'remember', 'mcp:recall'). */ scope?: string; + /** LLM provider configuration from hook config or CLI options. */ + llm?: import('../../domain/interfaces/IHookConfig').ILLMConfig; } diff --git a/src/infrastructure/llm/AnthropicLLMClient.ts b/src/infrastructure/llm/AnthropicLLMClient.ts index 932415cd..abc92bfc 100644 --- a/src/infrastructure/llm/AnthropicLLMClient.ts +++ b/src/infrastructure/llm/AnthropicLLMClient.ts @@ -1,21 +1,13 @@ /** * AnthropicLLMClient * - * Infrastructure implementation of ILLMClient using the Anthropic SDK. - * Sends commit message + diff to Claude for structured memory extraction. + * Anthropic provider implementation extending BaseLLMClient. + * Uses the Anthropic SDK for Claude API calls. */ import Anthropic from '@anthropic-ai/sdk'; -import type { - ILLMClient, - ILLMEnrichmentInput, - ILLMEnrichmentResult, - ILLMExtractedFact, -} from '../../domain/interfaces/ILLMClient'; -import type { MemoryType } from '../../domain/entities/IMemoryEntity'; -import { MEMORY_TYPE_VALUES } from '../../domain/entities/IMemoryEntity'; -import type { ConfidenceLevel } from '../../domain/types/IMemoryQuality'; -import { CONFIDENCE_VALUES } from '../../domain/types/IMemoryQuality'; +import { BaseLLMClient } from './BaseLLMClient'; +import type { IAPICallResult } from './BaseLLMClient'; import { LLMError } from '../../domain/errors/LLMError'; export interface IAnthropicLLMClientOptions { @@ -30,35 +22,8 @@ export interface IAnthropicLLMClientOptions { const DEFAULT_MODEL = 'claude-sonnet-4-20250514'; const DEFAULT_MAX_TOKENS = 2048; -const SYSTEM_PROMPT = `You are a code archaeology assistant. Given a git commit message and its diff, extract structured memories (decisions, gotchas, conventions, and facts) that would be valuable for future AI coding tools working in this codebase. - -Return a JSON array of extracted facts. Each fact should have: -- "content": A clear, concise description of the insight (1-2 sentences) -- "type": One of "decision", "gotcha", "convention", "fact" - - decision: An architectural or design choice and its rationale - - gotcha: A pitfall, workaround, or non-obvious behavior - - convention: A coding pattern or standard established - - fact: A factual observation about the codebase -- "confidence": One of "verified", "high", "medium", "low" - - verified: Explicitly stated in the commit message - - high: Strongly implied by the changes - - medium: Reasonable inference from the diff - - low: Weak inference, may need verification -- "tags": An array of relevant tags (e.g., file paths, technologies, concepts) - -Guidelines: -- Focus on insights that would help future developers understand WHY changes were made -- Skip trivial commits (version bumps, formatting, typo fixes) — return an empty array [] -- Prefer fewer high-quality facts over many low-quality ones -- Do not extract facts that are obvious from reading the code itself -- Tags should be lowercase, hyphenated (e.g., "error-handling", "api-design") - -Return ONLY a JSON array, no other text.`; - -export class AnthropicLLMClient implements ILLMClient { +export class AnthropicLLMClient extends BaseLLMClient { private readonly client: Anthropic; - private readonly model: string; - private readonly maxTokens: number; constructor(options?: IAnthropicLLMClientOptions) { const apiKey = options?.apiKey || process.env.ANTHROPIC_API_KEY; @@ -66,141 +31,30 @@ export class AnthropicLLMClient implements ILLMClient { throw new LLMError('Anthropic API key required. Set ANTHROPIC_API_KEY or pass apiKey option.'); } + super(options?.model || DEFAULT_MODEL, options?.maxTokens || DEFAULT_MAX_TOKENS); this.client = new Anthropic({ apiKey }); - this.model = options?.model || DEFAULT_MODEL; - this.maxTokens = options?.maxTokens || DEFAULT_MAX_TOKENS; - } - - async enrichCommit(input: ILLMEnrichmentInput): Promise { - const userMessage = this.buildUserMessage(input); - - try { - const response = await this.client.messages.create({ - model: this.model, - max_tokens: this.maxTokens, - system: SYSTEM_PROMPT, - messages: [{ role: 'user', content: userMessage }], - }); - - const text = response.content - .filter((block): block is Anthropic.TextBlock => block.type === 'text') - .map(block => block.text) - .join(''); - - const facts = this.parseResponse(text); - - return { - facts, - usage: { - inputTokens: response.usage.input_tokens, - outputTokens: response.usage.output_tokens, - }, - }; - } catch (error) { - if (error instanceof LLMError) throw error; - const message = error instanceof Error ? error.message : String(error); - throw new LLMError(`Anthropic API call failed: ${message}`, { - sha: input.sha, - model: this.model, - }); - } - } - - /** - * Build the user message from commit data. - */ - buildUserMessage(input: ILLMEnrichmentInput): string { - const parts: string[] = [ - `## Commit: ${input.sha.slice(0, 8)}`, - '', - `**Subject:** ${input.subject}`, - ]; - - if (input.body) { - parts.push('', `**Body:**`, input.body); - } - - if (input.filesChanged.length > 0) { - parts.push('', `**Files changed:** ${input.filesChanged.join(', ')}`); - } - - if (input.diff) { - parts.push('', '**Diff:**', '```', input.diff, '```'); - } - - return parts.join('\n'); } - /** - * Parse the LLM response into structured facts. - * Defensive: strips markdown fences, validates each entry, drops malformed. - */ - parseResponse(text: string): ILLMExtractedFact[] { - let cleaned = text.trim(); - - // Strip markdown code fences if present - if (cleaned.startsWith('```')) { - cleaned = cleaned.replace(/^```(?:json)?\s*\n?/, '').replace(/\n?```\s*$/, ''); - } - - let parsed: unknown; - try { - parsed = JSON.parse(cleaned); - } catch { - return []; - } - - if (!Array.isArray(parsed)) { - return []; - } - - const facts: ILLMExtractedFact[] = []; - for (const entry of parsed) { - const fact = this.validateFact(entry); - if (fact) { - facts.push(fact); - } - } - - return facts; - } - - /** - * Validate a single parsed fact entry. - * Returns null if the entry is malformed. - */ - private validateFact(entry: unknown): ILLMExtractedFact | null { - if (!entry || typeof entry !== 'object') return null; - - const obj = entry as Record; - - // content is required and must be a non-empty string - if (typeof obj.content !== 'string' || obj.content.trim().length === 0) { - return null; - } - - // type must be a valid MemoryType - if (typeof obj.type !== 'string' || !MEMORY_TYPE_VALUES.includes(obj.type as MemoryType)) { - return null; - } - - // confidence: default to 'medium' if missing/invalid - let confidence: ConfidenceLevel = 'medium'; - if (typeof obj.confidence === 'string' && CONFIDENCE_VALUES.includes(obj.confidence as ConfidenceLevel)) { - confidence = obj.confidence as ConfidenceLevel; - } - - // tags: default to empty array if missing/invalid - let tags: string[] = []; - if (Array.isArray(obj.tags)) { - tags = obj.tags.filter((t): t is string => typeof t === 'string'); - } + protected async callAPI( + systemPrompt: string, + userMessage: string, + ): Promise { + const response = await this.client.messages.create({ + model: this.model, + max_tokens: this.maxTokens, + system: systemPrompt, + messages: [{ role: 'user', content: userMessage }], + }); + + const text = response.content + .filter((block): block is Anthropic.TextBlock => block.type === 'text') + .map(block => block.text) + .join(''); return { - content: obj.content.trim(), - type: obj.type as MemoryType, - confidence, - tags, + text, + inputTokens: response.usage.input_tokens, + outputTokens: response.usage.output_tokens, }; } } diff --git a/src/infrastructure/llm/BaseLLMClient.ts b/src/infrastructure/llm/BaseLLMClient.ts new file mode 100644 index 00000000..2f9712f4 --- /dev/null +++ b/src/infrastructure/llm/BaseLLMClient.ts @@ -0,0 +1,205 @@ +/** + * BaseLLMClient + * + * Abstract base class for LLM provider implementations. + * Contains provider-agnostic logic: message building, response parsing, + * fact validation, and enrichCommit orchestration. + * + * Concrete providers implement only `callAPI()` and `callComplete()`. + */ + +import type { + ILLMClient, + ILLMEnrichmentInput, + ILLMEnrichmentResult, + ILLMExtractedFact, +} from '../../domain/interfaces/ILLMClient'; +import type { ILLMCaller, ILLMCallerOptions } from '../../domain/interfaces/ILLMCaller'; +import type { MemoryType } from '../../domain/entities/IMemoryEntity'; +import { MEMORY_TYPE_VALUES } from '../../domain/entities/IMemoryEntity'; +import type { ConfidenceLevel } from '../../domain/types/IMemoryQuality'; +import { CONFIDENCE_VALUES } from '../../domain/types/IMemoryQuality'; +import { LLMError } from '../../domain/errors/LLMError'; + +/** + * Result returned by the provider-specific API call. + */ +export interface IAPICallResult { + readonly text: string; + readonly inputTokens: number; + readonly outputTokens: number; +} + +export const ENRICHMENT_SYSTEM_PROMPT = `You are a code archaeology assistant. Given a git commit message and its diff, extract structured memories (decisions, gotchas, conventions, and facts) that would be valuable for future AI coding tools working in this codebase. + +Return a JSON array of extracted facts. Each fact should have: +- "content": A clear, concise description of the insight (1-2 sentences) +- "type": One of "decision", "gotcha", "convention", "fact" + - decision: An architectural or design choice and its rationale + - gotcha: A pitfall, workaround, or non-obvious behavior + - convention: A coding pattern or standard established + - fact: A factual observation about the codebase +- "confidence": One of "verified", "high", "medium", "low" + - verified: Explicitly stated in the commit message + - high: Strongly implied by the changes + - medium: Reasonable inference from the diff + - low: Weak inference, may need verification +- "tags": An array of relevant tags (e.g., file paths, technologies, concepts) + +Guidelines: +- Focus on insights that would help future developers understand WHY changes were made +- Skip trivial commits (version bumps, formatting, typo fixes) — return an empty array [] +- Prefer fewer high-quality facts over many low-quality ones +- Do not extract facts that are obvious from reading the code itself +- Tags should be lowercase, hyphenated (e.g., "error-handling", "api-design") + +Return ONLY a JSON array, no other text.`; + +export abstract class BaseLLMClient implements ILLMClient, ILLMCaller { + protected readonly model: string; + protected readonly maxTokens: number; + + constructor(model: string, maxTokens: number) { + this.model = model; + this.maxTokens = maxTokens; + } + + /** + * Provider-specific API call for enrichment. + */ + protected abstract callAPI( + systemPrompt: string, + userMessage: string, + ): Promise; + + /** + * Provider-specific API call for simple completions (used by ILLMCaller). + * Default implementation delegates to callAPI and returns just the text. + */ + async complete(options: ILLMCallerOptions): Promise { + const result = await this.callAPI(options.system, options.userMessage); + return result.text; + } + + async enrichCommit(input: ILLMEnrichmentInput): Promise { + const userMessage = this.buildUserMessage(input); + + try { + const result = await this.callAPI(ENRICHMENT_SYSTEM_PROMPT, userMessage); + const facts = this.parseResponse(result.text); + + return { + facts, + usage: { + inputTokens: result.inputTokens, + outputTokens: result.outputTokens, + }, + }; + } catch (error) { + if (error instanceof LLMError) throw error; + const message = error instanceof Error ? error.message : String(error); + throw new LLMError(`LLM API call failed: ${message}`, { + sha: input.sha, + model: this.model, + }); + } + } + + /** + * Build the user message from commit data. + */ + buildUserMessage(input: ILLMEnrichmentInput): string { + const parts: string[] = [ + `## Commit: ${input.sha.slice(0, 8)}`, + '', + `**Subject:** ${input.subject}`, + ]; + + if (input.body) { + parts.push('', '**Body:**', input.body); + } + + if (input.filesChanged.length > 0) { + parts.push('', `**Files changed:** ${input.filesChanged.join(', ')}`); + } + + if (input.diff) { + parts.push('', '**Diff:**', '```', input.diff, '```'); + } + + return parts.join('\n'); + } + + /** + * Parse the LLM response into structured facts. + * Defensive: strips markdown fences, validates each entry, drops malformed. + */ + parseResponse(text: string): ILLMExtractedFact[] { + let cleaned = text.trim(); + + // Strip markdown code fences if present + if (cleaned.startsWith('```')) { + cleaned = cleaned.replace(/^```(?:json)?\s*\n?/, '').replace(/\n?```\s*$/, ''); + } + + let parsed: unknown; + try { + parsed = JSON.parse(cleaned); + } catch { + return []; + } + + if (!Array.isArray(parsed)) { + return []; + } + + const facts: ILLMExtractedFact[] = []; + for (const entry of parsed) { + const fact = this.validateFact(entry); + if (fact) { + facts.push(fact); + } + } + + return facts; + } + + /** + * Validate a single parsed fact entry. + * Returns null if the entry is malformed. + */ + validateFact(entry: unknown): ILLMExtractedFact | null { + if (!entry || typeof entry !== 'object') return null; + + const obj = entry as Record; + + // content is required and must be a non-empty string + if (typeof obj.content !== 'string' || obj.content.trim().length === 0) { + return null; + } + + // type must be a valid MemoryType + if (typeof obj.type !== 'string' || !MEMORY_TYPE_VALUES.includes(obj.type as MemoryType)) { + return null; + } + + // confidence: default to 'medium' if missing/invalid + let confidence: ConfidenceLevel = 'medium'; + if (typeof obj.confidence === 'string' && CONFIDENCE_VALUES.includes(obj.confidence as ConfidenceLevel)) { + confidence = obj.confidence as ConfidenceLevel; + } + + // tags: default to empty array if missing/invalid + let tags: string[] = []; + if (Array.isArray(obj.tags)) { + tags = obj.tags.filter((t): t is string => typeof t === 'string'); + } + + return { + content: obj.content.trim(), + type: obj.type as MemoryType, + confidence, + tags, + }; + } +} diff --git a/src/infrastructure/llm/GeminiLLMClient.ts b/src/infrastructure/llm/GeminiLLMClient.ts new file mode 100644 index 00000000..517b2a24 --- /dev/null +++ b/src/infrastructure/llm/GeminiLLMClient.ts @@ -0,0 +1,78 @@ +/** + * GeminiLLMClient + * + * Google Gemini provider implementation extending BaseLLMClient. + * Uses the `@google/generative-ai` npm package. + */ + +import { BaseLLMClient } from './BaseLLMClient'; +import type { IAPICallResult } from './BaseLLMClient'; +import { LLMError } from '../../domain/errors/LLMError'; + +export interface IGeminiLLMClientOptions { + /** Google API key. Falls back to GOOGLE_API_KEY or GEMINI_API_KEY env var. */ + readonly apiKey?: string; + /** Model to use. Default: gemini-2.0-flash. */ + readonly model?: string; + /** Max tokens for response. Default: 2048. */ + readonly maxTokens?: number; +} + +const DEFAULT_MODEL = 'gemini-2.0-flash'; +const DEFAULT_MAX_TOKENS = 2048; + +export class GeminiLLMClient extends BaseLLMClient { + private readonly apiKey: string; + + constructor(options?: IGeminiLLMClientOptions) { + const apiKey = options?.apiKey || process.env.GOOGLE_API_KEY || process.env.GEMINI_API_KEY; + if (!apiKey) { + throw new LLMError('Google API key required. Set GOOGLE_API_KEY or GEMINI_API_KEY or pass apiKey option.'); + } + + super(options?.model || DEFAULT_MODEL, options?.maxTokens || DEFAULT_MAX_TOKENS); + this.apiKey = apiKey; + } + + protected async callAPI( + systemPrompt: string, + userMessage: string, + ): Promise { + let GoogleGenerativeAI: any; + try { + const mod = require('@google/generative-ai'); + GoogleGenerativeAI = mod.GoogleGenerativeAI; + } catch { + throw new LLMError( + 'Google Generative AI SDK not installed. Run: npm install @google/generative-ai', + ); + } + + const genAI = new GoogleGenerativeAI(this.apiKey); + const model = genAI.getGenerativeModel({ + model: this.model, + systemInstruction: systemPrompt, + }); + + try { + const result = await model.generateContent({ + contents: [{ role: 'user', parts: [{ text: userMessage }] }], + generationConfig: { maxOutputTokens: this.maxTokens }, + }); + + const response = result.response; + const text = response.text() ?? ''; + const usage = response.usageMetadata; + + return { + text, + inputTokens: usage?.promptTokenCount ?? 0, + outputTokens: usage?.candidatesTokenCount ?? 0, + }; + } catch (error) { + if (error instanceof LLMError) throw error; + const message = error instanceof Error ? error.message : String(error); + throw new LLMError(`Gemini API call failed: ${message}`); + } + } +} diff --git a/src/infrastructure/llm/IntentExtractor.ts b/src/infrastructure/llm/IntentExtractor.ts index 5c9e972c..5881c974 100644 --- a/src/infrastructure/llm/IntentExtractor.ts +++ b/src/infrastructure/llm/IntentExtractor.ts @@ -1,22 +1,22 @@ /** * IntentExtractor * - * Infrastructure implementation of IIntentExtractor using the Anthropic SDK. + * Infrastructure implementation of IIntentExtractor. * Extracts searchable keywords from user prompts for memory retrieval. - * Uses Claude Haiku for fast, cheap keyword extraction. + * Uses ILLMCaller for provider-agnostic LLM calls. */ -import Anthropic from '@anthropic-ai/sdk'; import type { IIntentExtractor, IIntentExtractorInput, IIntentExtractorResult, } from '../../domain/interfaces/IIntentExtractor'; +import type { ILLMCaller } from '../../domain/interfaces/ILLMCaller'; import type { ILogger } from '../../domain/interfaces/ILogger'; export interface IIntentExtractorOptions { - /** Anthropic API key. Falls back to ANTHROPIC_API_KEY env var. */ - readonly apiKey?: string; + /** LLM caller for completions. */ + readonly caller: ILLMCaller; /** Timeout in ms for LLM call. Default: 3000. */ readonly timeout?: number; /** Minimum word count to trigger extraction. Default: 5. */ @@ -25,7 +25,6 @@ export interface IIntentExtractorOptions { readonly logger?: ILogger; } -const HAIKU_MODEL = 'claude-haiku-4-5-20251001'; const MAX_TOKENS = 100; // Keywords are short const SYSTEM_PROMPT = `Extract searchable keywords from this user prompt. @@ -37,18 +36,13 @@ If the prompt is a simple confirmation or has no extractable keywords, respond w const CONFIRMATION_PATTERN = /^(yes|no|ok|okay|go|sure|proceed|continue|done|y|n|yep|nope|thanks|thank you|\d+)$/i; export class IntentExtractor implements IIntentExtractor { - private readonly client: Anthropic; + private readonly caller: ILLMCaller; private readonly timeout: number; private readonly minWords: number; private readonly logger?: ILogger; constructor(options: IIntentExtractorOptions) { - const apiKey = options.apiKey || process.env.ANTHROPIC_API_KEY; - if (!apiKey) { - throw new Error('Anthropic API key required for IntentExtractor'); - } - - this.client = new Anthropic({ apiKey }); + this.caller = options.caller; this.timeout = options.timeout ?? 3000; this.minWords = options.minWords ?? 5; this.logger = options.logger; @@ -99,32 +93,26 @@ export class IntentExtractor implements IIntentExtractor { * Call LLM with timeout enforcement. */ private async extractWithTimeout(prompt: string): Promise { + let timer: ReturnType | undefined; const timeoutPromise = new Promise((_, reject) => { - setTimeout(() => reject(new Error('Intent extraction timed out')), this.timeout); + timer = setTimeout(() => reject(new Error('Intent extraction timed out')), this.timeout); }); - const extractPromise = this.callLLM(prompt); - - return Promise.race([extractPromise, timeoutPromise]); - } - - /** - * Make the actual LLM call. - */ - private async callLLM(prompt: string): Promise { - const response = await this.client.messages.create({ - model: HAIKU_MODEL, - max_tokens: MAX_TOKENS, - system: SYSTEM_PROMPT, - messages: [{ role: 'user', content: `User prompt: "${prompt}"` }], - }); - - const text = response.content - .filter((block): block is Anthropic.TextBlock => block.type === 'text') - .map(block => block.text) - .join('') - .trim(); - - return text || null; + try { + const text = await Promise.race([ + this.caller.complete({ + system: SYSTEM_PROMPT, + userMessage: `User prompt: "${prompt}"`, + maxTokens: MAX_TOKENS, + }), + timeoutPromise, + ]); + + return text?.trim() || null; + } finally { + if (timer !== undefined) { + clearTimeout(timer); + } + } } } diff --git a/src/infrastructure/llm/LLMClientFactory.ts b/src/infrastructure/llm/LLMClientFactory.ts index 0778ecab..916cd31b 100644 --- a/src/infrastructure/llm/LLMClientFactory.ts +++ b/src/infrastructure/llm/LLMClientFactory.ts @@ -2,24 +2,27 @@ * LLMClientFactory * * Creates an ILLMClient instance based on available configuration. - * Returns null if no API key is found (graceful degradation). + * Returns null if no API key/provider is found (graceful degradation). + * Supports Anthropic, OpenAI, Gemini, and Ollama providers. */ import type { ILLMClient } from '../../domain/interfaces/ILLMClient'; -import { AnthropicLLMClient } from './AnthropicLLMClient'; +import type { LLMProvider } from '../../domain/interfaces/IHookConfig'; export interface ILLMClientFactoryOptions { /** Explicit provider selection. Auto-detected from env if omitted. */ - readonly provider?: 'anthropic'; + readonly provider?: LLMProvider; /** API key override. Falls back to env var. */ readonly apiKey?: string; /** Model override. */ readonly model?: string; + /** Base URL override (e.g., for Ollama host). */ + readonly baseUrl?: string; } /** * Create an LLM client from options or environment. - * Returns null if no API key is available (no throw). + * Returns null if no API key/provider is available (no throw). */ export function createLLMClient(options?: ILLMClientFactoryOptions): ILLMClient | null { const provider = options?.provider || detectProvider(options?.apiKey); @@ -32,11 +35,53 @@ export function createLLMClient(options?: ILLMClientFactoryOptions): ILLMClient case 'anthropic': { const apiKey = options?.apiKey || process.env.ANTHROPIC_API_KEY; if (!apiKey) return null; + const { AnthropicLLMClient } = require('./AnthropicLLMClient'); return new AnthropicLLMClient({ apiKey, model: options?.model, }); } + + case 'openai': { + const apiKey = options?.apiKey || process.env.OPENAI_API_KEY; + if (!apiKey) return null; + try { + const { OpenAILLMClient } = require('./OpenAILLMClient'); + return new OpenAILLMClient({ + apiKey, + model: options?.model, + }); + } catch { + return null; + } + } + + case 'gemini': { + const apiKey = options?.apiKey || process.env.GOOGLE_API_KEY || process.env.GEMINI_API_KEY; + if (!apiKey) return null; + try { + const { GeminiLLMClient } = require('./GeminiLLMClient'); + return new GeminiLLMClient({ + apiKey, + model: options?.model, + }); + } catch { + return null; + } + } + + case 'ollama': { + try { + const { OllamaLLMClient } = require('./OllamaLLMClient'); + return new OllamaLLMClient({ + model: options?.model, + baseUrl: options?.baseUrl, + }); + } catch { + return null; + } + } + default: return null; } @@ -44,10 +89,34 @@ export function createLLMClient(options?: ILLMClientFactoryOptions): ILLMClient /** * Detect which LLM provider to use from environment variables. + * Priority: explicit override > Anthropic > OpenAI > Gemini > Ollama. */ -function detectProvider(explicitApiKey?: string): 'anthropic' | null { +export function detectProvider(explicitApiKey?: string): LLMProvider | null { + // Explicit override via environment variable + const envProvider = process.env.GIT_MEM_LLM_PROVIDER; + if (envProvider && isKnownProvider(envProvider)) { + return envProvider; + } + if (explicitApiKey || process.env.ANTHROPIC_API_KEY) { return 'anthropic'; } + + if (process.env.OPENAI_API_KEY) { + return 'openai'; + } + + if (process.env.GOOGLE_API_KEY || process.env.GEMINI_API_KEY) { + return 'gemini'; + } + + if (process.env.OLLAMA_HOST) { + return 'ollama'; + } + return null; } + +function isKnownProvider(value: string): value is LLMProvider { + return ['anthropic', 'openai', 'gemini', 'ollama'].includes(value); +} diff --git a/src/infrastructure/llm/OllamaLLMClient.ts b/src/infrastructure/llm/OllamaLLMClient.ts new file mode 100644 index 00000000..3f1aa914 --- /dev/null +++ b/src/infrastructure/llm/OllamaLLMClient.ts @@ -0,0 +1,107 @@ +/** + * OllamaLLMClient + * + * Ollama provider implementation extending BaseLLMClient. + * Uses native fetch (no npm dependency) to call the Ollama HTTP API. + */ + +import { BaseLLMClient } from './BaseLLMClient'; +import type { IAPICallResult } from './BaseLLMClient'; +import { LLMError } from '../../domain/errors/LLMError'; + +export interface IOllamaLLMClientOptions { + /** Ollama host URL. Falls back to OLLAMA_HOST env var. Default: http://localhost:11434. */ + readonly baseUrl?: string; + /** Model to use. Default: llama3.2. */ + readonly model?: string; + /** Max tokens for response. Default: 2048. */ + readonly maxTokens?: number; + /** Request timeout in ms. Default: 60000 (Ollama is slower than cloud APIs). */ + readonly timeout?: number; +} + +const DEFAULT_BASE_URL = 'http://localhost:11434'; +const DEFAULT_MODEL = 'llama3.2'; +const DEFAULT_MAX_TOKENS = 2048; +const DEFAULT_TIMEOUT = 60000; + +export class OllamaLLMClient extends BaseLLMClient { + private readonly baseUrl: string; + private readonly timeout: number; + + constructor(options?: IOllamaLLMClientOptions) { + super( + options?.model || DEFAULT_MODEL, + options?.maxTokens || DEFAULT_MAX_TOKENS, + ); + this.baseUrl = (options?.baseUrl || process.env.OLLAMA_HOST || DEFAULT_BASE_URL).replace(/\/$/, ''); + this.timeout = options?.timeout || DEFAULT_TIMEOUT; + } + + protected async callAPI( + systemPrompt: string, + userMessage: string, + ): Promise { + const url = `${this.baseUrl}/api/chat`; + const body = JSON.stringify({ + model: this.model, + messages: [ + { role: 'system', content: systemPrompt }, + { role: 'user', content: userMessage }, + ], + stream: false, + options: { + num_predict: this.maxTokens, + }, + }); + + try { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), this.timeout); + + let response: Response; + try { + response = await fetch(url, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body, + signal: controller.signal, + }); + } finally { + clearTimeout(timer); + } + + if (!response.ok) { + const errorText = await response.text().catch(() => 'unknown error'); + throw new LLMError(`Ollama API returned ${response.status}: ${errorText}`); + } + + const data = await response.json() as { + message?: { content?: string }; + prompt_eval_count?: number; + eval_count?: number; + }; + + return { + text: data.message?.content ?? '', + inputTokens: data.prompt_eval_count ?? 0, + outputTokens: data.eval_count ?? 0, + }; + } catch (error) { + if (error instanceof LLMError) throw error; + + // Friendly message for connection errors + if (error instanceof TypeError && (error as NodeJS.ErrnoException).cause) { + const cause = (error as NodeJS.ErrnoException).cause as NodeJS.ErrnoException; + if (cause.code === 'ECONNREFUSED') { + throw new LLMError( + `Cannot connect to Ollama at ${this.baseUrl}. Is Ollama running?`, + ); + } + } + + const message = error instanceof Error ? error.message : String(error); + throw new LLMError(`Ollama API call failed: ${message}`); + } + } +} diff --git a/src/infrastructure/llm/OpenAILLMClient.ts b/src/infrastructure/llm/OpenAILLMClient.ts new file mode 100644 index 00000000..bd21b318 --- /dev/null +++ b/src/infrastructure/llm/OpenAILLMClient.ts @@ -0,0 +1,79 @@ +/** + * OpenAILLMClient + * + * OpenAI provider implementation extending BaseLLMClient. + * Uses the `openai` npm package for GPT API calls. + */ + +import { BaseLLMClient } from './BaseLLMClient'; +import type { IAPICallResult } from './BaseLLMClient'; +import { LLMError } from '../../domain/errors/LLMError'; + +// OpenAI SDK is an optional peer dependency — no static import. +// eslint-disable-next-line @typescript-eslint/no-require-imports + +export interface IOpenAILLMClientOptions { + /** OpenAI API key. Falls back to OPENAI_API_KEY env var. */ + readonly apiKey?: string; + /** Model to use. Default: gpt-4o. */ + readonly model?: string; + /** Max tokens for response. Default: 2048. */ + readonly maxTokens?: number; +} + +const DEFAULT_MODEL = 'gpt-4o'; +const DEFAULT_MAX_TOKENS = 2048; + +export class OpenAILLMClient extends BaseLLMClient { + private readonly apiKey: string; + + constructor(options?: IOpenAILLMClientOptions) { + const apiKey = options?.apiKey || process.env.OPENAI_API_KEY; + if (!apiKey) { + throw new LLMError('OpenAI API key required. Set OPENAI_API_KEY or pass apiKey option.'); + } + + super(options?.model || DEFAULT_MODEL, options?.maxTokens || DEFAULT_MAX_TOKENS); + this.apiKey = apiKey; + } + + protected async callAPI( + systemPrompt: string, + userMessage: string, + ): Promise { + let OpenAI: any; + try { + const mod = require('openai'); + OpenAI = mod.default ?? mod; + } catch { + throw new LLMError( + 'OpenAI SDK not installed. Run: npm install openai', + ); + } + + const client = new OpenAI({ apiKey: this.apiKey }); + + try { + const response = await client.chat.completions.create({ + model: this.model, + max_tokens: this.maxTokens, + messages: [ + { role: 'system', content: systemPrompt }, + { role: 'user', content: userMessage }, + ], + }); + + const text = response.choices[0]?.message?.content ?? ''; + + return { + text, + inputTokens: response.usage?.prompt_tokens ?? 0, + outputTokens: response.usage?.completion_tokens ?? 0, + }; + } catch (error) { + if (error instanceof LLMError) throw error; + const message = error instanceof Error ? error.message : String(error); + throw new LLMError(`OpenAI API call failed: ${message}`); + } + } +} diff --git a/tests/unit/hooks/utils/config.test.ts b/tests/unit/hooks/utils/config.test.ts index 4639a073..2212d3d8 100644 --- a/tests/unit/hooks/utils/config.test.ts +++ b/tests/unit/hooks/utils/config.test.ts @@ -162,5 +162,73 @@ describe('config', () => { assert.equal(config.hooks.commitMsg.autoAnalyze, true); assert.equal(config.hooks.commitMsg.inferTags, true); }); + + it('should parse llm configuration section', () => { + const testDir = createTestDir(); + writeConfig(testDir, { + hooks: { enabled: true }, + llm: { + provider: 'openai', + model: 'gpt-4o', + intentModel: 'gpt-4o-mini', + baseUrl: 'http://localhost:11434', + }, + }); + + const config = loadHookConfig(testDir); + + assert.ok(config.llm); + assert.equal(config.llm!.provider, 'openai'); + assert.equal(config.llm!.model, 'gpt-4o'); + assert.equal(config.llm!.intentModel, 'gpt-4o-mini'); + assert.equal(config.llm!.baseUrl, 'http://localhost:11434'); + }); + + it('should accept all valid providers in llm config', () => { + for (const provider of ['anthropic', 'openai', 'gemini', 'ollama']) { + const testDir = createTestDir(); + writeConfig(testDir, { + hooks: { enabled: true }, + llm: { provider }, + }); + + const config = loadHookConfig(testDir); + assert.ok(config.llm, `llm should be present for provider ${provider}`); + assert.equal(config.llm!.provider, provider); + } + }); + + it('should ignore invalid provider in llm config', () => { + const testDir = createTestDir(); + writeConfig(testDir, { + hooks: { enabled: true }, + llm: { provider: 'invalid-provider' }, + }); + + const config = loadHookConfig(testDir); + // No valid fields parsed, so llm should not be attached + assert.equal(config.llm, undefined); + }); + + it('should not include llm when section is absent', () => { + const testDir = createTestDir(); + writeConfig(testDir, { hooks: { enabled: true } }); + + const config = loadHookConfig(testDir); + assert.equal(config.llm, undefined); + }); + + it('should parse partial llm config (only model)', () => { + const testDir = createTestDir(); + writeConfig(testDir, { + hooks: { enabled: true }, + llm: { model: 'custom-model' }, + }); + + const config = loadHookConfig(testDir); + assert.ok(config.llm); + assert.equal(config.llm!.model, 'custom-model'); + assert.equal(config.llm!.provider, undefined); + }); }); }); diff --git a/tests/unit/infrastructure/llm/AnthropicLLMClient.test.ts b/tests/unit/infrastructure/llm/AnthropicLLMClient.test.ts index c225c19d..d1591312 100644 --- a/tests/unit/infrastructure/llm/AnthropicLLMClient.test.ts +++ b/tests/unit/infrastructure/llm/AnthropicLLMClient.test.ts @@ -1,12 +1,12 @@ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; import { AnthropicLLMClient } from '../../../../src/infrastructure/llm/AnthropicLLMClient'; +import { BaseLLMClient } from '../../../../src/infrastructure/llm/BaseLLMClient'; import { LLMError } from '../../../../src/domain/errors/LLMError'; describe('AnthropicLLMClient', () => { describe('constructor', () => { it('should throw LLMError when no API key is provided', () => { - // Save and clear env const saved = process.env.ANTHROPIC_API_KEY; delete process.env.ANTHROPIC_API_KEY; @@ -17,10 +17,9 @@ describe('AnthropicLLMClient', () => { assert.ok(err instanceof LLMError); assert.ok(err.message.includes('API key')); return true; - } + }, ); } finally { - // Restore env if (saved !== undefined) { process.env.ANTHROPIC_API_KEY = saved; } @@ -31,227 +30,18 @@ describe('AnthropicLLMClient', () => { const client = new AnthropicLLMClient({ apiKey: 'sk-ant-test-key' }); assert.ok(client); }); - }); - - describe('buildUserMessage', () => { - let client: AnthropicLLMClient; - - // Use a dummy API key for non-API tests - const dummyKey = 'sk-ant-test-dummy'; - - it('should include subject and SHA', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const msg = client.buildUserMessage({ - sha: 'abc12345def67890', - subject: 'feat: add login page', - body: '', - diff: '', - filesChanged: [], - }); - - assert.ok(msg.includes('abc12345')); - assert.ok(msg.includes('feat: add login page')); - }); - - it('should include body when present', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const msg = client.buildUserMessage({ - sha: 'abc12345', - subject: 'feat: something', - body: 'Decided to use Redis for caching because it supports TTL.', - diff: '', - filesChanged: [], - }); - assert.ok(msg.includes('Decided to use Redis')); - assert.ok(msg.includes('**Body:**')); - }); - - it('should omit body section when body is empty', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const msg = client.buildUserMessage({ - sha: 'abc12345', - subject: 'feat: something', - body: '', - diff: 'some diff', - filesChanged: [], - }); - - assert.ok(!msg.includes('**Body:**')); - }); - - it('should include files changed', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const msg = client.buildUserMessage({ - sha: 'abc12345', - subject: 'feat: something', - body: '', - diff: '', - filesChanged: ['src/auth.ts', 'src/login.ts'], - }); - - assert.ok(msg.includes('src/auth.ts')); - assert.ok(msg.includes('src/login.ts')); + it('should extend BaseLLMClient', () => { + const client = new AnthropicLLMClient({ apiKey: 'sk-ant-test-key' }); + assert.ok(client instanceof BaseLLMClient); }); - it('should include diff in code block', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const msg = client.buildUserMessage({ - sha: 'abc12345', - subject: 'feat: something', - body: '', - diff: '+const x = 1;\n-const y = 2;', - filesChanged: [], - }); - - assert.ok(msg.includes('```')); - assert.ok(msg.includes('+const x = 1;')); + it('should implement ILLMCaller (complete method)', () => { + const client = new AnthropicLLMClient({ apiKey: 'sk-ant-test-key' }); + assert.equal(typeof client.complete, 'function'); }); }); - describe('parseResponse', () => { - let client: AnthropicLLMClient; - const dummyKey = 'sk-ant-test-dummy'; - - it('should parse valid JSON array', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse(JSON.stringify([ - { - content: 'Using JWT for stateless auth', - type: 'decision', - confidence: 'high', - tags: ['authentication', 'jwt'], - }, - ])); - - assert.equal(facts.length, 1); - assert.equal(facts[0]!.content, 'Using JWT for stateless auth'); - assert.equal(facts[0]!.type, 'decision'); - assert.equal(facts[0]!.confidence, 'high'); - assert.deepEqual(facts[0]!.tags, ['authentication', 'jwt']); - }); - - it('should return empty array for empty JSON array', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse('[]'); - assert.equal(facts.length, 0); - }); - - it('should strip markdown code fences', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const wrapped = '```json\n[\n {\n "content": "test fact",\n "type": "fact",\n "confidence": "medium",\n "tags": []\n }\n]\n```'; - const facts = client.parseResponse(wrapped); - assert.equal(facts.length, 1); - assert.equal(facts[0]!.content, 'test fact'); - }); - - it('should return empty array for invalid JSON', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse('this is not json'); - assert.equal(facts.length, 0); - }); - - it('should return empty array for non-array JSON', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse('{"content": "not an array"}'); - assert.equal(facts.length, 0); - }); - - it('should drop entries with missing content', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse(JSON.stringify([ - { type: 'fact', confidence: 'high', tags: [] }, - { content: 'valid fact', type: 'fact', confidence: 'high', tags: [] }, - ])); - - assert.equal(facts.length, 1); - assert.equal(facts[0]!.content, 'valid fact'); - }); - - it('should drop entries with empty content', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse(JSON.stringify([ - { content: '', type: 'fact', confidence: 'high', tags: [] }, - { content: ' ', type: 'fact', confidence: 'high', tags: [] }, - ])); - - assert.equal(facts.length, 0); - }); - - it('should drop entries with invalid type', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse(JSON.stringify([ - { content: 'some insight', type: 'invalid-type', confidence: 'high', tags: [] }, - { content: 'valid fact', type: 'gotcha', confidence: 'high', tags: [] }, - ])); - - assert.equal(facts.length, 1); - assert.equal(facts[0]!.type, 'gotcha'); - }); - - it('should default confidence to medium when invalid', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse(JSON.stringify([ - { content: 'some fact', type: 'fact', confidence: 'super-high', tags: [] }, - ])); - - assert.equal(facts.length, 1); - assert.equal(facts[0]!.confidence, 'medium'); - }); - - it('should default tags to empty array when missing', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse(JSON.stringify([ - { content: 'some fact', type: 'fact', confidence: 'high' }, - ])); - - assert.equal(facts.length, 1); - assert.deepEqual(facts[0]!.tags, []); - }); - - it('should filter non-string tags', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse(JSON.stringify([ - { content: 'some fact', type: 'fact', confidence: 'high', tags: ['valid', 123, null, 'also-valid'] }, - ])); - - assert.equal(facts.length, 1); - assert.deepEqual(facts[0]!.tags, ['valid', 'also-valid']); - }); - - it('should handle multiple valid facts', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse(JSON.stringify([ - { content: 'decision one', type: 'decision', confidence: 'verified', tags: ['auth'] }, - { content: 'gotcha one', type: 'gotcha', confidence: 'high', tags: ['race-condition'] }, - { content: 'convention one', type: 'convention', confidence: 'medium', tags: [] }, - ])); - - assert.equal(facts.length, 3); - assert.equal(facts[0]!.type, 'decision'); - assert.equal(facts[1]!.type, 'gotcha'); - assert.equal(facts[2]!.type, 'convention'); - }); - - it('should drop null and non-object entries', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse(JSON.stringify([ - null, - 42, - 'string', - { content: 'valid', type: 'fact', confidence: 'high', tags: [] }, - ])); - - assert.equal(facts.length, 1); - }); - - it('should trim content whitespace', () => { - client = new AnthropicLLMClient({ apiKey: dummyKey }); - const facts = client.parseResponse(JSON.stringify([ - { content: ' padded content ', type: 'fact', confidence: 'high', tags: [] }, - ])); - - assert.equal(facts[0]!.content, 'padded content'); - }); - }); + // Note: buildUserMessage, parseResponse, validateFact, and enrichCommit + // are tested in BaseLLMClient.test.ts since they are shared logic. }); diff --git a/tests/unit/infrastructure/llm/BaseLLMClient.test.ts b/tests/unit/infrastructure/llm/BaseLLMClient.test.ts new file mode 100644 index 00000000..659b76cb --- /dev/null +++ b/tests/unit/infrastructure/llm/BaseLLMClient.test.ts @@ -0,0 +1,321 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { BaseLLMClient, ENRICHMENT_SYSTEM_PROMPT } from '../../../../src/infrastructure/llm/BaseLLMClient'; +import type { IAPICallResult } from '../../../../src/infrastructure/llm/BaseLLMClient'; +import type { ILLMEnrichmentInput } from '../../../../src/domain/interfaces/ILLMClient'; +import { LLMError } from '../../../../src/domain/errors/LLMError'; + +/** + * Concrete test implementation of BaseLLMClient. + */ +class TestLLMClient extends BaseLLMClient { + public callAPIFn: (system: string, user: string) => Promise; + + constructor( + callAPIFn?: (system: string, user: string) => Promise, + ) { + super('test-model', 2048); + this.callAPIFn = callAPIFn ?? (async () => ({ + text: '[]', + inputTokens: 10, + outputTokens: 5, + })); + } + + protected async callAPI(systemPrompt: string, userMessage: string): Promise { + return this.callAPIFn(systemPrompt, userMessage); + } +} + +describe('BaseLLMClient', () => { + describe('buildUserMessage', () => { + const client = new TestLLMClient(); + + it('should include subject and SHA', () => { + const msg = client.buildUserMessage({ + sha: 'abc12345def67890', + subject: 'feat: add login page', + body: '', + diff: '', + filesChanged: [], + }); + + assert.ok(msg.includes('abc12345')); + assert.ok(msg.includes('feat: add login page')); + }); + + it('should include body when present', () => { + const msg = client.buildUserMessage({ + sha: 'abc12345', + subject: 'feat: something', + body: 'Decided to use Redis for caching.', + diff: '', + filesChanged: [], + }); + + assert.ok(msg.includes('Decided to use Redis')); + assert.ok(msg.includes('**Body:**')); + }); + + it('should omit body section when body is empty', () => { + const msg = client.buildUserMessage({ + sha: 'abc12345', + subject: 'feat: something', + body: '', + diff: 'some diff', + filesChanged: [], + }); + + assert.ok(!msg.includes('**Body:**')); + }); + + it('should include files changed', () => { + const msg = client.buildUserMessage({ + sha: 'abc12345', + subject: 'feat: something', + body: '', + diff: '', + filesChanged: ['src/auth.ts', 'src/login.ts'], + }); + + assert.ok(msg.includes('src/auth.ts')); + assert.ok(msg.includes('src/login.ts')); + }); + + it('should include diff in code block', () => { + const msg = client.buildUserMessage({ + sha: 'abc12345', + subject: 'feat: something', + body: '', + diff: '+const x = 1;\n-const y = 2;', + filesChanged: [], + }); + + assert.ok(msg.includes('```')); + assert.ok(msg.includes('+const x = 1;')); + }); + }); + + describe('parseResponse', () => { + const client = new TestLLMClient(); + + it('should parse valid JSON array', () => { + const facts = client.parseResponse(JSON.stringify([ + { + content: 'Using JWT for stateless auth', + type: 'decision', + confidence: 'high', + tags: ['authentication', 'jwt'], + }, + ])); + + assert.equal(facts.length, 1); + assert.equal(facts[0]!.content, 'Using JWT for stateless auth'); + assert.equal(facts[0]!.type, 'decision'); + assert.equal(facts[0]!.confidence, 'high'); + assert.deepEqual(facts[0]!.tags, ['authentication', 'jwt']); + }); + + it('should return empty array for empty JSON array', () => { + assert.equal(client.parseResponse('[]').length, 0); + }); + + it('should strip markdown code fences', () => { + const wrapped = '```json\n[{"content":"test","type":"fact","confidence":"medium","tags":[]}]\n```'; + const facts = client.parseResponse(wrapped); + assert.equal(facts.length, 1); + assert.equal(facts[0]!.content, 'test'); + }); + + it('should return empty array for invalid JSON', () => { + assert.equal(client.parseResponse('not json').length, 0); + }); + + it('should return empty array for non-array JSON', () => { + assert.equal(client.parseResponse('{"content":"obj"}').length, 0); + }); + + it('should drop entries with missing content', () => { + const facts = client.parseResponse(JSON.stringify([ + { type: 'fact', confidence: 'high', tags: [] }, + { content: 'valid', type: 'fact', confidence: 'high', tags: [] }, + ])); + assert.equal(facts.length, 1); + }); + + it('should drop entries with empty content', () => { + const facts = client.parseResponse(JSON.stringify([ + { content: '', type: 'fact', confidence: 'high', tags: [] }, + { content: ' ', type: 'fact', confidence: 'high', tags: [] }, + ])); + assert.equal(facts.length, 0); + }); + + it('should drop entries with invalid type', () => { + const facts = client.parseResponse(JSON.stringify([ + { content: 'bad', type: 'invalid-type', confidence: 'high', tags: [] }, + { content: 'good', type: 'gotcha', confidence: 'high', tags: [] }, + ])); + assert.equal(facts.length, 1); + assert.equal(facts[0]!.type, 'gotcha'); + }); + + it('should default confidence to medium when invalid', () => { + const facts = client.parseResponse(JSON.stringify([ + { content: 'fact', type: 'fact', confidence: 'super-high', tags: [] }, + ])); + assert.equal(facts[0]!.confidence, 'medium'); + }); + + it('should default tags to empty array when missing', () => { + const facts = client.parseResponse(JSON.stringify([ + { content: 'fact', type: 'fact', confidence: 'high' }, + ])); + assert.deepEqual(facts[0]!.tags, []); + }); + + it('should filter non-string tags', () => { + const facts = client.parseResponse(JSON.stringify([ + { content: 'fact', type: 'fact', confidence: 'high', tags: ['ok', 123, null, 'fine'] }, + ])); + assert.deepEqual(facts[0]!.tags, ['ok', 'fine']); + }); + + it('should trim content whitespace', () => { + const facts = client.parseResponse(JSON.stringify([ + { content: ' padded ', type: 'fact', confidence: 'high', tags: [] }, + ])); + assert.equal(facts[0]!.content, 'padded'); + }); + + it('should drop null and non-object entries', () => { + const facts = client.parseResponse(JSON.stringify([ + null, 42, 'str', + { content: 'valid', type: 'fact', confidence: 'high', tags: [] }, + ])); + assert.equal(facts.length, 1); + }); + }); + + describe('validateFact', () => { + const client = new TestLLMClient(); + + it('should return null for null input', () => { + assert.equal(client.validateFact(null), null); + }); + + it('should return null for non-object input', () => { + assert.equal(client.validateFact('string'), null); + assert.equal(client.validateFact(42), null); + }); + + it('should accept all valid memory types', () => { + for (const type of ['decision', 'gotcha', 'convention', 'fact']) { + const result = client.validateFact({ content: 'test', type, confidence: 'high', tags: [] }); + assert.ok(result, `Should accept type: ${type}`); + assert.equal(result!.type, type); + } + }); + + it('should accept all valid confidence levels', () => { + for (const confidence of ['verified', 'high', 'medium', 'low']) { + const result = client.validateFact({ content: 'test', type: 'fact', confidence, tags: [] }); + assert.ok(result, `Should accept confidence: ${confidence}`); + assert.equal(result!.confidence, confidence); + } + }); + }); + + describe('enrichCommit', () => { + it('should orchestrate callAPI and return parsed facts', async () => { + const client = new TestLLMClient(async (_system, _user) => ({ + text: JSON.stringify([ + { content: 'A decision', type: 'decision', confidence: 'high', tags: ['arch'] }, + ]), + inputTokens: 100, + outputTokens: 50, + })); + + const input: ILLMEnrichmentInput = { + sha: 'abc12345', + subject: 'feat: add auth', + body: '', + diff: '+code', + filesChanged: ['auth.ts'], + }; + + const result = await client.enrichCommit(input); + + assert.equal(result.facts.length, 1); + assert.equal(result.facts[0]!.content, 'A decision'); + assert.equal(result.usage.inputTokens, 100); + assert.equal(result.usage.outputTokens, 50); + }); + + it('should pass ENRICHMENT_SYSTEM_PROMPT to callAPI', async () => { + let capturedSystem = ''; + const client = new TestLLMClient(async (system, _user) => { + capturedSystem = system; + return { text: '[]', inputTokens: 0, outputTokens: 0 }; + }); + + await client.enrichCommit({ + sha: 'abc', subject: 'test', body: '', diff: '', filesChanged: [], + }); + + assert.equal(capturedSystem, ENRICHMENT_SYSTEM_PROMPT); + }); + + it('should wrap non-LLMError exceptions', async () => { + const client = new TestLLMClient(async () => { + throw new Error('network down'); + }); + + await assert.rejects( + () => client.enrichCommit({ + sha: 'abc', subject: 'test', body: '', diff: '', filesChanged: [], + }), + (err: unknown) => { + assert.ok(err instanceof LLMError); + assert.ok(err.message.includes('network down')); + return true; + }, + ); + }); + + it('should re-throw LLMError directly', async () => { + const client = new TestLLMClient(async () => { + throw new LLMError('specific error'); + }); + + await assert.rejects( + () => client.enrichCommit({ + sha: 'abc', subject: 'test', body: '', diff: '', filesChanged: [], + }), + (err: unknown) => { + assert.ok(err instanceof LLMError); + assert.equal(err.message, 'specific error'); + return true; + }, + ); + }); + }); + + describe('complete (ILLMCaller)', () => { + it('should delegate to callAPI and return text', async () => { + const client = new TestLLMClient(async (_system, _user) => ({ + text: 'keyword1, keyword2', + inputTokens: 10, + outputTokens: 5, + })); + + const result = await client.complete({ + system: 'Extract keywords', + userMessage: 'some prompt', + maxTokens: 100, + }); + + assert.equal(result, 'keyword1, keyword2'); + }); + }); +}); diff --git a/tests/unit/infrastructure/llm/GeminiLLMClient.test.ts b/tests/unit/infrastructure/llm/GeminiLLMClient.test.ts new file mode 100644 index 00000000..77246a30 --- /dev/null +++ b/tests/unit/infrastructure/llm/GeminiLLMClient.test.ts @@ -0,0 +1,56 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { GeminiLLMClient } from '../../../../src/infrastructure/llm/GeminiLLMClient'; +import { LLMError } from '../../../../src/domain/errors/LLMError'; + +describe('GeminiLLMClient', () => { + describe('constructor', () => { + it('should throw LLMError when no API key is provided', () => { + const savedGoogle = process.env.GOOGLE_API_KEY; + const savedGemini = process.env.GEMINI_API_KEY; + delete process.env.GOOGLE_API_KEY; + delete process.env.GEMINI_API_KEY; + + try { + assert.throws( + () => new GeminiLLMClient(), + (err: unknown) => { + assert.ok(err instanceof LLMError); + assert.ok(err.message.includes('API key')); + return true; + }, + ); + } finally { + if (savedGoogle !== undefined) process.env.GOOGLE_API_KEY = savedGoogle; + if (savedGemini !== undefined) process.env.GEMINI_API_KEY = savedGemini; + } + }); + + it('should accept explicit apiKey option', () => { + const client = new GeminiLLMClient({ apiKey: 'AIza-test-key' }); + assert.ok(client); + }); + + it('should accept GEMINI_API_KEY env var', () => { + const savedGoogle = process.env.GOOGLE_API_KEY; + const savedGemini = process.env.GEMINI_API_KEY; + delete process.env.GOOGLE_API_KEY; + process.env.GEMINI_API_KEY = 'AIza-env-key'; + + try { + const client = new GeminiLLMClient(); + assert.ok(client); + } finally { + if (savedGoogle !== undefined) process.env.GOOGLE_API_KEY = savedGoogle; + else delete process.env.GOOGLE_API_KEY; + if (savedGemini !== undefined) process.env.GEMINI_API_KEY = savedGemini; + else delete process.env.GEMINI_API_KEY; + } + }); + + it('should accept custom model option', () => { + const client = new GeminiLLMClient({ apiKey: 'AIza-test', model: 'gemini-1.5-pro' }); + assert.ok(client); + }); + }); +}); diff --git a/tests/unit/infrastructure/llm/IntentExtractor.test.ts b/tests/unit/infrastructure/llm/IntentExtractor.test.ts index 14ff3a7e..9a704094 100644 --- a/tests/unit/infrastructure/llm/IntentExtractor.test.ts +++ b/tests/unit/infrastructure/llm/IntentExtractor.test.ts @@ -2,85 +2,61 @@ * IntentExtractor unit tests * * Tests for keyword extraction from user prompts. - * LLM calls are not mocked since we test filtering logic separately. + * Uses mock ILLMCaller instead of real LLM calls. */ import { describe, it } from 'node:test'; import assert from 'node:assert/strict'; +import { IntentExtractor } from '../../../../src/infrastructure/llm/IntentExtractor'; +import type { ILLMCaller, ILLMCallerOptions } from '../../../../src/domain/interfaces/ILLMCaller'; + +function createMockCaller(response: string): ILLMCaller { + return { + async complete(_options: ILLMCallerOptions): Promise { + return response; + }, + }; +} + +function createSlowCaller(delayMs: number, response: string): ILLMCaller { + return { + async complete(_options: ILLMCallerOptions): Promise { + await new Promise(resolve => setTimeout(resolve, delayMs)); + return response; + }, + }; +} + +function createFailingCaller(error: Error): ILLMCaller { + return { + async complete(_options: ILLMCallerOptions): Promise { + throw error; + }, + }; +} -// Test the confirmation pattern matching logic directly describe('IntentExtractor', () => { - // The CONFIRMATION_PATTERN from IntentExtractor.ts + // Confirmation pattern tests (unchanged — test the regex directly) const CONFIRMATION_PATTERN = /^(yes|no|ok|okay|go|sure|proceed|continue|done|y|n|yep|nope|thanks|thank you|\d+)$/i; describe('confirmation pattern', () => { - it('should match "yes"', () => { - assert.ok(CONFIRMATION_PATTERN.test('yes')); - }); - - it('should match "no"', () => { - assert.ok(CONFIRMATION_PATTERN.test('no')); - }); - - it('should match "ok"', () => { - assert.ok(CONFIRMATION_PATTERN.test('ok')); - }); - - it('should match "okay"', () => { - assert.ok(CONFIRMATION_PATTERN.test('okay')); - }); - - it('should match "go"', () => { - assert.ok(CONFIRMATION_PATTERN.test('go')); - }); - - it('should match "sure"', () => { - assert.ok(CONFIRMATION_PATTERN.test('sure')); - }); - - it('should match "proceed"', () => { - assert.ok(CONFIRMATION_PATTERN.test('proceed')); - }); - - it('should match "continue"', () => { - assert.ok(CONFIRMATION_PATTERN.test('continue')); - }); - - it('should match "done"', () => { - assert.ok(CONFIRMATION_PATTERN.test('done')); - }); - - it('should match "y"', () => { - assert.ok(CONFIRMATION_PATTERN.test('y')); - }); - - it('should match "n"', () => { - assert.ok(CONFIRMATION_PATTERN.test('n')); - }); - - it('should match "yep"', () => { - assert.ok(CONFIRMATION_PATTERN.test('yep')); - }); - - it('should match "nope"', () => { - assert.ok(CONFIRMATION_PATTERN.test('nope')); - }); - - it('should match "thanks"', () => { - assert.ok(CONFIRMATION_PATTERN.test('thanks')); - }); - - it('should match "thank you"', () => { - assert.ok(CONFIRMATION_PATTERN.test('thank you')); - }); - - it('should match single digit', () => { - assert.ok(CONFIRMATION_PATTERN.test('1')); - }); - - it('should match multi-digit number', () => { - assert.ok(CONFIRMATION_PATTERN.test('42')); - }); + it('should match "yes"', () => assert.ok(CONFIRMATION_PATTERN.test('yes'))); + it('should match "no"', () => assert.ok(CONFIRMATION_PATTERN.test('no'))); + it('should match "ok"', () => assert.ok(CONFIRMATION_PATTERN.test('ok'))); + it('should match "okay"', () => assert.ok(CONFIRMATION_PATTERN.test('okay'))); + it('should match "go"', () => assert.ok(CONFIRMATION_PATTERN.test('go'))); + it('should match "sure"', () => assert.ok(CONFIRMATION_PATTERN.test('sure'))); + it('should match "proceed"', () => assert.ok(CONFIRMATION_PATTERN.test('proceed'))); + it('should match "continue"', () => assert.ok(CONFIRMATION_PATTERN.test('continue'))); + it('should match "done"', () => assert.ok(CONFIRMATION_PATTERN.test('done'))); + it('should match "y"', () => assert.ok(CONFIRMATION_PATTERN.test('y'))); + it('should match "n"', () => assert.ok(CONFIRMATION_PATTERN.test('n'))); + it('should match "yep"', () => assert.ok(CONFIRMATION_PATTERN.test('yep'))); + it('should match "nope"', () => assert.ok(CONFIRMATION_PATTERN.test('nope'))); + it('should match "thanks"', () => assert.ok(CONFIRMATION_PATTERN.test('thanks'))); + it('should match "thank you"', () => assert.ok(CONFIRMATION_PATTERN.test('thank you'))); + it('should match single digit', () => assert.ok(CONFIRMATION_PATTERN.test('1'))); + it('should match multi-digit number', () => assert.ok(CONFIRMATION_PATTERN.test('42'))); it('should be case-insensitive', () => { assert.ok(CONFIRMATION_PATTERN.test('YES')); @@ -137,4 +113,106 @@ describe('IntentExtractor', () => { assert.ok(countWords('implement user authentication for the API') >= minWords); }); }); + + describe('extract with ILLMCaller', () => { + it('should return extracted keywords from LLM', async () => { + const caller = createMockCaller('GIT-95, authentication, login'); + const extractor = new IntentExtractor({ caller }); + + const result = await extractor.extract({ + prompt: 'fix the authentication bug in login flow please', + }); + + assert.equal(result.skipped, false); + assert.equal(result.intent, 'GIT-95, authentication, login'); + }); + + it('should skip when LLM returns SKIP', async () => { + const caller = createMockCaller('SKIP'); + const extractor = new IntentExtractor({ caller }); + + const result = await extractor.extract({ + prompt: 'can you help me with something in this project please', + }); + + assert.equal(result.skipped, true); + assert.equal(result.reason, 'llm_skip'); + }); + + it('should skip prompts with too few words', async () => { + const caller = createMockCaller('should not be called'); + const extractor = new IntentExtractor({ caller, minWords: 5 }); + + const result = await extractor.extract({ prompt: 'fix bug' }); + + assert.equal(result.skipped, true); + assert.equal(result.reason, 'too_short'); + }); + + it('should skip confirmations', async () => { + const caller = createMockCaller('should not be called'); + const extractor = new IntentExtractor({ caller, minWords: 1 }); + + const result = await extractor.extract({ prompt: 'yes' }); + + assert.equal(result.skipped, true); + assert.equal(result.reason, 'confirmation'); + }); + + it('should handle LLM errors gracefully', async () => { + const caller = createFailingCaller(new Error('API down')); + const extractor = new IntentExtractor({ caller }); + + const result = await extractor.extract({ + prompt: 'implement the new feature for user profiles please', + }); + + assert.equal(result.skipped, true); + assert.equal(result.reason, 'error'); + }); + + it('should handle timeout gracefully', async () => { + const caller = createSlowCaller(200, 'slow response'); + const extractor = new IntentExtractor({ caller, timeout: 50 }); + + const result = await extractor.extract({ + prompt: 'implement the new feature for user profiles please', + }); + + assert.equal(result.skipped, true); + assert.equal(result.reason, 'timeout'); + }); + + it('should return null intent for empty LLM response', async () => { + const caller = createMockCaller(''); + const extractor = new IntentExtractor({ caller }); + + const result = await extractor.extract({ + prompt: 'implement the new feature for user profiles please', + }); + + assert.equal(result.skipped, true); + assert.equal(result.reason, 'llm_skip'); + }); + + it('should pass system prompt and user message to caller', async () => { + let capturedOptions: ILLMCallerOptions | undefined; + const caller: ILLMCaller = { + async complete(options: ILLMCallerOptions): Promise { + capturedOptions = options; + return 'keywords'; + }, + }; + const extractor = new IntentExtractor({ caller }); + + await extractor.extract({ + prompt: 'fix the authentication bug in the login flow please', + }); + + assert.ok(capturedOptions); + assert.ok(capturedOptions!.system.includes('Extract searchable keywords')); + assert.ok(capturedOptions!.userMessage.includes('authentication')); + assert.equal(capturedOptions!.maxTokens, 100); + }); + }); }); diff --git a/tests/unit/infrastructure/llm/LLMClientFactory.test.ts b/tests/unit/infrastructure/llm/LLMClientFactory.test.ts index f151ba67..91c12c58 100644 --- a/tests/unit/infrastructure/llm/LLMClientFactory.test.ts +++ b/tests/unit/infrastructure/llm/LLMClientFactory.test.ts @@ -1,43 +1,139 @@ import { describe, it, before, after } from 'node:test'; import assert from 'node:assert/strict'; -import { createLLMClient } from '../../../../src/infrastructure/llm/LLMClientFactory'; +import { createLLMClient, detectProvider } from '../../../../src/infrastructure/llm/LLMClientFactory'; import { AnthropicLLMClient } from '../../../../src/infrastructure/llm/AnthropicLLMClient'; describe('LLMClientFactory', () => { - describe('createLLMClient', () => { - let savedKey: string | undefined; + // Save all env vars we'll modify + const envKeys = [ + 'ANTHROPIC_API_KEY', 'OPENAI_API_KEY', 'GOOGLE_API_KEY', + 'GEMINI_API_KEY', 'OLLAMA_HOST', 'GIT_MEM_LLM_PROVIDER', + ] as const; - before(() => { - savedKey = process.env.ANTHROPIC_API_KEY; - }); + let savedEnv: Record; + + function clearAllEnv(): void { + for (const key of envKeys) { + delete process.env[key]; + } + } - after(() => { - if (savedKey !== undefined) { - process.env.ANTHROPIC_API_KEY = savedKey; + before(() => { + savedEnv = {}; + for (const key of envKeys) { + savedEnv[key] = process.env[key]; + } + }); + + after(() => { + for (const key of envKeys) { + if (savedEnv[key] !== undefined) { + process.env[key] = savedEnv[key]; } else { - delete process.env.ANTHROPIC_API_KEY; + delete process.env[key]; } + } + }); + + describe('detectProvider', () => { + it('should return null when no env vars are set', () => { + clearAllEnv(); + assert.equal(detectProvider(), null); + }); + + it('should detect anthropic from ANTHROPIC_API_KEY', () => { + clearAllEnv(); + process.env.ANTHROPIC_API_KEY = 'sk-ant-test'; + assert.equal(detectProvider(), 'anthropic'); + }); + + it('should detect openai from OPENAI_API_KEY', () => { + clearAllEnv(); + process.env.OPENAI_API_KEY = 'sk-test'; + assert.equal(detectProvider(), 'openai'); + }); + + it('should detect gemini from GOOGLE_API_KEY', () => { + clearAllEnv(); + process.env.GOOGLE_API_KEY = 'AIza-test'; + assert.equal(detectProvider(), 'gemini'); + }); + + it('should detect gemini from GEMINI_API_KEY', () => { + clearAllEnv(); + process.env.GEMINI_API_KEY = 'AIza-test'; + assert.equal(detectProvider(), 'gemini'); + }); + + it('should detect ollama from OLLAMA_HOST', () => { + clearAllEnv(); + process.env.OLLAMA_HOST = 'http://localhost:11434'; + assert.equal(detectProvider(), 'ollama'); + }); + + it('should prioritize anthropic over openai', () => { + clearAllEnv(); + process.env.ANTHROPIC_API_KEY = 'sk-ant-test'; + process.env.OPENAI_API_KEY = 'sk-test'; + assert.equal(detectProvider(), 'anthropic'); + }); + + it('should prioritize openai over gemini', () => { + clearAllEnv(); + process.env.OPENAI_API_KEY = 'sk-test'; + process.env.GOOGLE_API_KEY = 'AIza-test'; + assert.equal(detectProvider(), 'openai'); }); + it('should prioritize gemini over ollama', () => { + clearAllEnv(); + process.env.GOOGLE_API_KEY = 'AIza-test'; + process.env.OLLAMA_HOST = 'http://localhost:11434'; + assert.equal(detectProvider(), 'gemini'); + }); + + it('should honor GIT_MEM_LLM_PROVIDER override', () => { + clearAllEnv(); + process.env.ANTHROPIC_API_KEY = 'sk-ant-test'; + process.env.GIT_MEM_LLM_PROVIDER = 'openai'; + assert.equal(detectProvider(), 'openai'); + }); + + it('should ignore invalid GIT_MEM_LLM_PROVIDER', () => { + clearAllEnv(); + process.env.ANTHROPIC_API_KEY = 'sk-ant-test'; + process.env.GIT_MEM_LLM_PROVIDER = 'invalid-provider'; + assert.equal(detectProvider(), 'anthropic'); + }); + + it('should use explicit API key to detect anthropic', () => { + clearAllEnv(); + assert.equal(detectProvider('sk-ant-explicit'), 'anthropic'); + }); + }); + + describe('createLLMClient', () => { it('should return null when no API key is available', () => { - delete process.env.ANTHROPIC_API_KEY; + clearAllEnv(); const client = createLLMClient(); assert.equal(client, null); }); it('should return AnthropicLLMClient when ANTHROPIC_API_KEY is set', () => { + clearAllEnv(); process.env.ANTHROPIC_API_KEY = 'sk-ant-test-key'; const client = createLLMClient(); assert.ok(client instanceof AnthropicLLMClient); }); it('should return AnthropicLLMClient when explicit apiKey is passed', () => { - delete process.env.ANTHROPIC_API_KEY; + clearAllEnv(); const client = createLLMClient({ apiKey: 'sk-ant-explicit-key' }); assert.ok(client instanceof AnthropicLLMClient); }); it('should return AnthropicLLMClient when provider is explicitly anthropic', () => { + clearAllEnv(); const client = createLLMClient({ provider: 'anthropic', apiKey: 'sk-ant-test-key', @@ -46,17 +142,47 @@ describe('LLMClientFactory', () => { }); it('should return null when provider is anthropic but no key', () => { - delete process.env.ANTHROPIC_API_KEY; + clearAllEnv(); const client = createLLMClient({ provider: 'anthropic' }); assert.equal(client, null); }); it('should pass model option through to client', () => { + clearAllEnv(); const client = createLLMClient({ apiKey: 'sk-ant-test-key', model: 'claude-haiku-4-5-20251001', }); assert.ok(client instanceof AnthropicLLMClient); }); + + it('should return null for openai without SDK installed', () => { + clearAllEnv(); + process.env.OPENAI_API_KEY = 'sk-test'; + // The openai SDK is not a dependency, so require will fail + // and the factory should return null gracefully + const client = createLLMClient({ provider: 'openai', apiKey: 'sk-test' }); + // Could be null if SDK not installed, or an instance if it is + // The test verifies it doesn't throw + assert.ok(client === null || typeof client.enrichCommit === 'function'); + }); + + it('should return null for gemini without SDK installed', () => { + clearAllEnv(); + const client = createLLMClient({ provider: 'gemini', apiKey: 'AIza-test' }); + assert.ok(client === null || typeof client.enrichCommit === 'function'); + }); + + it('should return null for openai when no key provided', () => { + clearAllEnv(); + const client = createLLMClient({ provider: 'openai' }); + assert.equal(client, null); + }); + + it('should return null for gemini when no key provided', () => { + clearAllEnv(); + const client = createLLMClient({ provider: 'gemini' }); + assert.equal(client, null); + }); }); }); diff --git a/tests/unit/infrastructure/llm/OllamaLLMClient.test.ts b/tests/unit/infrastructure/llm/OllamaLLMClient.test.ts new file mode 100644 index 00000000..95ee7ac6 --- /dev/null +++ b/tests/unit/infrastructure/llm/OllamaLLMClient.test.ts @@ -0,0 +1,44 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { OllamaLLMClient } from '../../../../src/infrastructure/llm/OllamaLLMClient'; + +describe('OllamaLLMClient', () => { + describe('constructor', () => { + it('should create client with default settings', () => { + const client = new OllamaLLMClient(); + assert.ok(client); + }); + + it('should accept custom baseUrl option', () => { + const client = new OllamaLLMClient({ baseUrl: 'http://custom:11434' }); + assert.ok(client); + }); + + it('should accept custom model option', () => { + const client = new OllamaLLMClient({ model: 'mistral' }); + assert.ok(client); + }); + + it('should use OLLAMA_HOST env var', () => { + const saved = process.env.OLLAMA_HOST; + process.env.OLLAMA_HOST = 'http://remote:11434'; + + try { + const client = new OllamaLLMClient(); + assert.ok(client); + } finally { + if (saved !== undefined) { + process.env.OLLAMA_HOST = saved; + } else { + delete process.env.OLLAMA_HOST; + } + } + }); + + it('should strip trailing slash from baseUrl', () => { + // Can't directly inspect the private field, but construction should succeed + const client = new OllamaLLMClient({ baseUrl: 'http://localhost:11434/' }); + assert.ok(client); + }); + }); +}); diff --git a/tests/unit/infrastructure/llm/OpenAILLMClient.test.ts b/tests/unit/infrastructure/llm/OpenAILLMClient.test.ts new file mode 100644 index 00000000..8d1e77aa --- /dev/null +++ b/tests/unit/infrastructure/llm/OpenAILLMClient.test.ts @@ -0,0 +1,38 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { OpenAILLMClient } from '../../../../src/infrastructure/llm/OpenAILLMClient'; +import { LLMError } from '../../../../src/domain/errors/LLMError'; + +describe('OpenAILLMClient', () => { + describe('constructor', () => { + it('should throw LLMError when no API key is provided', () => { + const saved = process.env.OPENAI_API_KEY; + delete process.env.OPENAI_API_KEY; + + try { + assert.throws( + () => new OpenAILLMClient(), + (err: unknown) => { + assert.ok(err instanceof LLMError); + assert.ok(err.message.includes('API key')); + return true; + }, + ); + } finally { + if (saved !== undefined) { + process.env.OPENAI_API_KEY = saved; + } + } + }); + + it('should accept explicit apiKey option', () => { + const client = new OpenAILLMClient({ apiKey: 'sk-test-key' }); + assert.ok(client); + }); + + it('should accept custom model option', () => { + const client = new OpenAILLMClient({ apiKey: 'sk-test-key', model: 'gpt-4o-mini' }); + assert.ok(client); + }); + }); +}); From 8f4b981246e50df2e59e8e018d6509957d84fe85 Mon Sep 17 00:00:00 2001 From: Tony Casey Date: Sun, 15 Feb 2026 22:26:31 +0000 Subject: [PATCH 2/6] =?UTF-8?q?fix:=20address=20review=20comments=20?= =?UTF-8?q?=E2=80=94=20fix=20JSDoc,=20remove=20unsafe=20cast=20(GIT-110)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix BaseLLMClient JSDoc: callComplete() → callAPI() with correct description - Improve ICommitMsgConfig.enrich JSDoc with all provider env var names - Make ILLMClient extend ILLMCaller, removing the `as unknown as` cast in DI container Co-Authored-By: Claude Opus 4.6 AI-Agent: Claude-Code/2.1.42 --- src/domain/interfaces/IHookConfig.ts | 2 +- src/domain/interfaces/ILLMClient.ts | 4 +++- src/infrastructure/di/container.ts | 2 +- src/infrastructure/llm/BaseLLMClient.ts | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/domain/interfaces/IHookConfig.ts b/src/domain/interfaces/IHookConfig.ts index 3003523c..7131277a 100644 --- a/src/domain/interfaces/IHookConfig.ts +++ b/src/domain/interfaces/IHookConfig.ts @@ -55,7 +55,7 @@ export interface ICommitMsgConfig { readonly requireType: boolean; /** Default memory lifecycle. */ readonly defaultLifecycle: 'permanent' | 'project' | 'session'; - /** Enable LLM enrichment for richer trailer content. Requires an LLM provider API key. */ + /** Enable LLM enrichment for richer trailer content. Requires a configured LLM provider (e.g., ANTHROPIC_API_KEY, OPENAI_API_KEY, GOOGLE_API_KEY, or OLLAMA_HOST). */ readonly enrich: boolean; /** Timeout in ms for LLM enrichment call. Default: 8000. Must be under hook timeout (10s). */ readonly enrichTimeout: number; diff --git a/src/domain/interfaces/ILLMClient.ts b/src/domain/interfaces/ILLMClient.ts index 1c60d55e..9c2cf80a 100644 --- a/src/domain/interfaces/ILLMClient.ts +++ b/src/domain/interfaces/ILLMClient.ts @@ -7,6 +7,7 @@ import type { MemoryType } from '../entities/IMemoryEntity'; import type { ConfidenceLevel } from '../types/IMemoryQuality'; +import type { ILLMCaller } from './ILLMCaller'; /** * Input to an LLM enrichment call for a single commit. @@ -53,8 +54,9 @@ export interface ILLMEnrichmentResult { /** * Provider-agnostic LLM client interface. + * Extends ILLMCaller for simple text completions (used by IntentExtractor). */ -export interface ILLMClient { +export interface ILLMClient extends ILLMCaller { /** * Extract structured memories from a commit's message and diff. * @param input - Commit data and diff to analyze. diff --git a/src/infrastructure/di/container.ts b/src/infrastructure/di/container.ts index f917e3d2..1df9dd00 100644 --- a/src/infrastructure/di/container.ts +++ b/src/infrastructure/di/container.ts @@ -122,7 +122,7 @@ export function createContainer(options?: IContainerOptions): AwilixContainer Date: Sun, 15 Feb 2026 22:31:14 +0000 Subject: [PATCH 3/6] =?UTF-8?q?fix:=20address=20CodeRabbit=20review=20?= =?UTF-8?q?=E2=80=94=20maxTokens=20propagation,=20ESLint,=20cleanup=20(GIT?= =?UTF-8?q?-110)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Pass maxTokens through callAPI() to all providers so ILLMCaller.complete() respects the caller's token limit (e.g., IntentExtractor uses 100) - Add eslint-disable comments for dynamic require() of optional peer deps - Fix env var restoration in OpenAI test cleanup Co-Authored-By: Claude Opus 4.6 AI-Agent: Claude-Code/2.1.42 --- src/infrastructure/llm/AnthropicLLMClient.ts | 3 ++- src/infrastructure/llm/BaseLLMClient.ts | 8 +++++--- src/infrastructure/llm/GeminiLLMClient.ts | 5 ++++- src/infrastructure/llm/LLMClientFactory.ts | 4 ++++ src/infrastructure/llm/OllamaLLMClient.ts | 3 ++- src/infrastructure/llm/OpenAILLMClient.ts | 5 ++++- tests/unit/infrastructure/llm/OpenAILLMClient.test.ts | 2 ++ 7 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/infrastructure/llm/AnthropicLLMClient.ts b/src/infrastructure/llm/AnthropicLLMClient.ts index abc92bfc..f58260a0 100644 --- a/src/infrastructure/llm/AnthropicLLMClient.ts +++ b/src/infrastructure/llm/AnthropicLLMClient.ts @@ -38,10 +38,11 @@ export class AnthropicLLMClient extends BaseLLMClient { protected async callAPI( systemPrompt: string, userMessage: string, + maxTokens?: number, ): Promise { const response = await this.client.messages.create({ model: this.model, - max_tokens: this.maxTokens, + max_tokens: maxTokens ?? this.maxTokens, system: systemPrompt, messages: [{ role: 'user', content: userMessage }], }); diff --git a/src/infrastructure/llm/BaseLLMClient.ts b/src/infrastructure/llm/BaseLLMClient.ts index a865665d..41257b1e 100644 --- a/src/infrastructure/llm/BaseLLMClient.ts +++ b/src/infrastructure/llm/BaseLLMClient.ts @@ -65,11 +65,13 @@ export abstract class BaseLLMClient implements ILLMClient, ILLMCaller { } /** - * Provider-specific API call for enrichment. + * Provider-specific API call. + * @param maxTokens - Optional override for max response tokens. */ protected abstract callAPI( systemPrompt: string, userMessage: string, + maxTokens?: number, ): Promise; /** @@ -77,7 +79,7 @@ export abstract class BaseLLMClient implements ILLMClient, ILLMCaller { * Default implementation delegates to callAPI and returns just the text. */ async complete(options: ILLMCallerOptions): Promise { - const result = await this.callAPI(options.system, options.userMessage); + const result = await this.callAPI(options.system, options.userMessage, options.maxTokens); return result.text; } @@ -85,7 +87,7 @@ export abstract class BaseLLMClient implements ILLMClient, ILLMCaller { const userMessage = this.buildUserMessage(input); try { - const result = await this.callAPI(ENRICHMENT_SYSTEM_PROMPT, userMessage); + const result = await this.callAPI(ENRICHMENT_SYSTEM_PROMPT, userMessage, this.maxTokens); const facts = this.parseResponse(result.text); return { diff --git a/src/infrastructure/llm/GeminiLLMClient.ts b/src/infrastructure/llm/GeminiLLMClient.ts index 517b2a24..b887e4e8 100644 --- a/src/infrastructure/llm/GeminiLLMClient.ts +++ b/src/infrastructure/llm/GeminiLLMClient.ts @@ -37,9 +37,12 @@ export class GeminiLLMClient extends BaseLLMClient { protected async callAPI( systemPrompt: string, userMessage: string, + maxTokens?: number, ): Promise { + // eslint-disable-next-line @typescript-eslint/no-explicit-any let GoogleGenerativeAI: any; try { + // eslint-disable-next-line @typescript-eslint/no-require-imports const mod = require('@google/generative-ai'); GoogleGenerativeAI = mod.GoogleGenerativeAI; } catch { @@ -57,7 +60,7 @@ export class GeminiLLMClient extends BaseLLMClient { try { const result = await model.generateContent({ contents: [{ role: 'user', parts: [{ text: userMessage }] }], - generationConfig: { maxOutputTokens: this.maxTokens }, + generationConfig: { maxOutputTokens: maxTokens ?? this.maxTokens }, }); const response = result.response; diff --git a/src/infrastructure/llm/LLMClientFactory.ts b/src/infrastructure/llm/LLMClientFactory.ts index 916cd31b..884d722f 100644 --- a/src/infrastructure/llm/LLMClientFactory.ts +++ b/src/infrastructure/llm/LLMClientFactory.ts @@ -35,6 +35,7 @@ export function createLLMClient(options?: ILLMClientFactoryOptions): ILLMClient case 'anthropic': { const apiKey = options?.apiKey || process.env.ANTHROPIC_API_KEY; if (!apiKey) return null; + // eslint-disable-next-line @typescript-eslint/no-require-imports const { AnthropicLLMClient } = require('./AnthropicLLMClient'); return new AnthropicLLMClient({ apiKey, @@ -46,6 +47,7 @@ export function createLLMClient(options?: ILLMClientFactoryOptions): ILLMClient const apiKey = options?.apiKey || process.env.OPENAI_API_KEY; if (!apiKey) return null; try { + // eslint-disable-next-line @typescript-eslint/no-require-imports const { OpenAILLMClient } = require('./OpenAILLMClient'); return new OpenAILLMClient({ apiKey, @@ -60,6 +62,7 @@ export function createLLMClient(options?: ILLMClientFactoryOptions): ILLMClient const apiKey = options?.apiKey || process.env.GOOGLE_API_KEY || process.env.GEMINI_API_KEY; if (!apiKey) return null; try { + // eslint-disable-next-line @typescript-eslint/no-require-imports const { GeminiLLMClient } = require('./GeminiLLMClient'); return new GeminiLLMClient({ apiKey, @@ -72,6 +75,7 @@ export function createLLMClient(options?: ILLMClientFactoryOptions): ILLMClient case 'ollama': { try { + // eslint-disable-next-line @typescript-eslint/no-require-imports const { OllamaLLMClient } = require('./OllamaLLMClient'); return new OllamaLLMClient({ model: options?.model, diff --git a/src/infrastructure/llm/OllamaLLMClient.ts b/src/infrastructure/llm/OllamaLLMClient.ts index 3f1aa914..e8c90a64 100644 --- a/src/infrastructure/llm/OllamaLLMClient.ts +++ b/src/infrastructure/llm/OllamaLLMClient.ts @@ -41,6 +41,7 @@ export class OllamaLLMClient extends BaseLLMClient { protected async callAPI( systemPrompt: string, userMessage: string, + maxTokens?: number, ): Promise { const url = `${this.baseUrl}/api/chat`; const body = JSON.stringify({ @@ -51,7 +52,7 @@ export class OllamaLLMClient extends BaseLLMClient { ], stream: false, options: { - num_predict: this.maxTokens, + num_predict: maxTokens ?? this.maxTokens, }, }); diff --git a/src/infrastructure/llm/OpenAILLMClient.ts b/src/infrastructure/llm/OpenAILLMClient.ts index bd21b318..898a3bae 100644 --- a/src/infrastructure/llm/OpenAILLMClient.ts +++ b/src/infrastructure/llm/OpenAILLMClient.ts @@ -40,9 +40,12 @@ export class OpenAILLMClient extends BaseLLMClient { protected async callAPI( systemPrompt: string, userMessage: string, + maxTokens?: number, ): Promise { + // eslint-disable-next-line @typescript-eslint/no-explicit-any let OpenAI: any; try { + // eslint-disable-next-line @typescript-eslint/no-require-imports const mod = require('openai'); OpenAI = mod.default ?? mod; } catch { @@ -56,7 +59,7 @@ export class OpenAILLMClient extends BaseLLMClient { try { const response = await client.chat.completions.create({ model: this.model, - max_tokens: this.maxTokens, + max_tokens: maxTokens ?? this.maxTokens, messages: [ { role: 'system', content: systemPrompt }, { role: 'user', content: userMessage }, diff --git a/tests/unit/infrastructure/llm/OpenAILLMClient.test.ts b/tests/unit/infrastructure/llm/OpenAILLMClient.test.ts index 8d1e77aa..bf2e876c 100644 --- a/tests/unit/infrastructure/llm/OpenAILLMClient.test.ts +++ b/tests/unit/infrastructure/llm/OpenAILLMClient.test.ts @@ -21,6 +21,8 @@ describe('OpenAILLMClient', () => { } finally { if (saved !== undefined) { process.env.OPENAI_API_KEY = saved; + } else { + delete process.env.OPENAI_API_KEY; } } }); From 9bf0a4a98898b5f9c3e210b2cc29ae8635f7dfc6 Mon Sep 17 00:00:00 2001 From: Tony Casey Date: Sun, 15 Feb 2026 22:50:34 +0000 Subject: [PATCH 4/6] =?UTF-8?q?fix:=20address=20round-2=20review=20?= =?UTF-8?q?=E2=80=94=20import=20style,=20factory=20consistency,=20docs=20(?= =?UTF-8?q?GIT-110)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove stale eslint-disable comment at top of OpenAILLMClient.ts - Use top-level import type for ILLMConfig in types.ts - Add try/catch to anthropic case in factory for consistency - Mark intentModel as reserved (not yet wired) - Fix CLAUDE.md markdown: blank line before fenced code block - Clarify test comments about optional SDK loading Co-Authored-By: Claude Opus 4.6 AI-Agent: Claude-Code/2.1.42 --- CLAUDE.md | 1 + src/domain/interfaces/IHookConfig.ts | 2 +- src/infrastructure/di/types.ts | 3 ++- src/infrastructure/llm/LLMClientFactory.ts | 16 ++++++++++------ src/infrastructure/llm/OpenAILLMClient.ts | 3 --- .../infrastructure/llm/LLMClientFactory.test.ts | 11 +++++------ 6 files changed, 19 insertions(+), 17 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 095ec479..e00b6a55 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,6 +74,7 @@ Uses **`node:test`** (native Node.js test runner) with **`tsx`** for TypeScript, Only one provider is needed. Without any LLM key, `--enrich` falls back to heuristic extraction with a warning. See `.env.example`. **LLM config in `.git-mem/.git-mem.yaml`:** + ```yaml llm: provider: openai # auto-detected if omitted diff --git a/src/domain/interfaces/IHookConfig.ts b/src/domain/interfaces/IHookConfig.ts index 7131277a..04ed1183 100644 --- a/src/domain/interfaces/IHookConfig.ts +++ b/src/domain/interfaces/IHookConfig.ts @@ -78,7 +78,7 @@ export interface ILLMConfig { readonly provider?: LLMProvider; /** Model for enrichment. Provider default if omitted. */ readonly model?: string; - /** Lighter model for intent extraction. Falls back to model if omitted. */ + /** Lighter model for intent extraction. Reserved — not yet wired to handler. */ readonly intentModel?: string; /** Base URL override (e.g., for Ollama). */ readonly baseUrl?: string; diff --git a/src/infrastructure/di/types.ts b/src/infrastructure/di/types.ts index 73fc31de..bcd7eb5d 100644 --- a/src/infrastructure/di/types.ts +++ b/src/infrastructure/di/types.ts @@ -28,6 +28,7 @@ import type { ITrailerService } from '../../domain/interfaces/ITrailerService'; import type { IAgentResolver } from '../../domain/interfaces/IAgentResolver'; import type { IHookConfigLoader } from '../../domain/interfaces/IHookConfigLoader'; import type { IIntentExtractor } from '../../domain/interfaces/IIntentExtractor'; +import type { ILLMConfig } from '../../domain/interfaces/IHookConfig'; export interface ICradle { // Infrastructure @@ -65,5 +66,5 @@ export interface IContainerOptions { /** Scope label for child logger (e.g., 'remember', 'mcp:recall'). */ scope?: string; /** LLM provider configuration from hook config or CLI options. */ - llm?: import('../../domain/interfaces/IHookConfig').ILLMConfig; + llm?: ILLMConfig; } diff --git a/src/infrastructure/llm/LLMClientFactory.ts b/src/infrastructure/llm/LLMClientFactory.ts index 884d722f..133da17c 100644 --- a/src/infrastructure/llm/LLMClientFactory.ts +++ b/src/infrastructure/llm/LLMClientFactory.ts @@ -35,12 +35,16 @@ export function createLLMClient(options?: ILLMClientFactoryOptions): ILLMClient case 'anthropic': { const apiKey = options?.apiKey || process.env.ANTHROPIC_API_KEY; if (!apiKey) return null; - // eslint-disable-next-line @typescript-eslint/no-require-imports - const { AnthropicLLMClient } = require('./AnthropicLLMClient'); - return new AnthropicLLMClient({ - apiKey, - model: options?.model, - }); + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { AnthropicLLMClient } = require('./AnthropicLLMClient'); + return new AnthropicLLMClient({ + apiKey, + model: options?.model, + }); + } catch { + return null; + } } case 'openai': { diff --git a/src/infrastructure/llm/OpenAILLMClient.ts b/src/infrastructure/llm/OpenAILLMClient.ts index 898a3bae..f994d41b 100644 --- a/src/infrastructure/llm/OpenAILLMClient.ts +++ b/src/infrastructure/llm/OpenAILLMClient.ts @@ -9,9 +9,6 @@ import { BaseLLMClient } from './BaseLLMClient'; import type { IAPICallResult } from './BaseLLMClient'; import { LLMError } from '../../domain/errors/LLMError'; -// OpenAI SDK is an optional peer dependency — no static import. -// eslint-disable-next-line @typescript-eslint/no-require-imports - export interface IOpenAILLMClientOptions { /** OpenAI API key. Falls back to OPENAI_API_KEY env var. */ readonly apiKey?: string; diff --git a/tests/unit/infrastructure/llm/LLMClientFactory.test.ts b/tests/unit/infrastructure/llm/LLMClientFactory.test.ts index 91c12c58..ee501948 100644 --- a/tests/unit/infrastructure/llm/LLMClientFactory.test.ts +++ b/tests/unit/infrastructure/llm/LLMClientFactory.test.ts @@ -156,19 +156,18 @@ describe('LLMClientFactory', () => { assert.ok(client instanceof AnthropicLLMClient); }); - it('should return null for openai without SDK installed', () => { + it('should handle openai provider gracefully', () => { clearAllEnv(); process.env.OPENAI_API_KEY = 'sk-test'; - // The openai SDK is not a dependency, so require will fail - // and the factory should return null gracefully + // Returns a client if openai SDK is installed, null if construction fails. + // The test verifies it doesn't throw. const client = createLLMClient({ provider: 'openai', apiKey: 'sk-test' }); - // Could be null if SDK not installed, or an instance if it is - // The test verifies it doesn't throw assert.ok(client === null || typeof client.enrichCommit === 'function'); }); - it('should return null for gemini without SDK installed', () => { + it('should handle gemini provider gracefully', () => { clearAllEnv(); + // Returns a client if @google/generative-ai is installed, null if construction fails. const client = createLLMClient({ provider: 'gemini', apiKey: 'AIza-test' }); assert.ok(client === null || typeof client.enrichCommit === 'function'); }); From cde16eb47ceb1cbd6646fa751d033b2a21f33ad3 Mon Sep 17 00:00:00 2001 From: Tony Casey Date: Sun, 15 Feb 2026 23:03:19 +0000 Subject: [PATCH 5/6] fix: add AbortError timeout detection and update test comments (GIT-110) - OllamaLLMClient: detect AbortError specifically for timeout messages - LLMClientFactory.test.ts: clarify test names and comments for SDK behavior Co-Authored-By: Claude Opus 4.6 AI-Agent: Claude-Code/2.1.42 --- src/infrastructure/llm/OllamaLLMClient.ts | 7 +++++++ .../unit/infrastructure/llm/LLMClientFactory.test.ts | 11 ++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/infrastructure/llm/OllamaLLMClient.ts b/src/infrastructure/llm/OllamaLLMClient.ts index e8c90a64..c78af0de 100644 --- a/src/infrastructure/llm/OllamaLLMClient.ts +++ b/src/infrastructure/llm/OllamaLLMClient.ts @@ -91,6 +91,13 @@ export class OllamaLLMClient extends BaseLLMClient { } catch (error) { if (error instanceof LLMError) throw error; + // Specific timeout detection from AbortController + if (error instanceof Error && error.name === 'AbortError') { + throw new LLMError( + `Ollama API request timed out after ${this.timeout}ms`, + ); + } + // Friendly message for connection errors if (error instanceof TypeError && (error as NodeJS.ErrnoException).cause) { const cause = (error as NodeJS.ErrnoException).cause as NodeJS.ErrnoException; diff --git a/tests/unit/infrastructure/llm/LLMClientFactory.test.ts b/tests/unit/infrastructure/llm/LLMClientFactory.test.ts index ee501948..afad5edb 100644 --- a/tests/unit/infrastructure/llm/LLMClientFactory.test.ts +++ b/tests/unit/infrastructure/llm/LLMClientFactory.test.ts @@ -156,18 +156,19 @@ describe('LLMClientFactory', () => { assert.ok(client instanceof AnthropicLLMClient); }); - it('should handle openai provider gracefully', () => { + it('should not throw for openai even if SDK is missing', () => { clearAllEnv(); process.env.OPENAI_API_KEY = 'sk-test'; - // Returns a client if openai SDK is installed, null if construction fails. - // The test verifies it doesn't throw. + // Factory creates client if openai SDK is installed, returns null if not. + // SDK availability is checked in the provider's callAPI(), not at construction. const client = createLLMClient({ provider: 'openai', apiKey: 'sk-test' }); assert.ok(client === null || typeof client.enrichCommit === 'function'); }); - it('should handle gemini provider gracefully', () => { + it('should not throw for gemini even if SDK is missing', () => { clearAllEnv(); - // Returns a client if @google/generative-ai is installed, null if construction fails. + // Factory creates client if @google/generative-ai is installed, returns null if not. + // SDK availability is checked in the provider's callAPI(), not at construction. const client = createLLMClient({ provider: 'gemini', apiKey: 'AIza-test' }); assert.ok(client === null || typeof client.enrichCommit === 'function'); }); From 2e4e7e1840d681298d690f4ef372c769cbc0250c Mon Sep 17 00:00:00 2001 From: Tony Casey Date: Sun, 15 Feb 2026 23:11:03 +0000 Subject: [PATCH 6/6] fix: replace any with structural types for optional SDK loading (GIT-110) - OpenAILLMClient: typed OpenAIConstructor replaces any - GeminiLLMClient: typed GenAIConstructor/GeminiModel replaces any - Both use unknown + cast instead of eslint-disable for no-explicit-any Co-Authored-By: Claude Opus 4.6 AI-Agent: Claude-Code/2.1.42 AI-Decision: When dynamically loading optional SDKs, use structural typing instead of 'any' to maintain type safety while allowing for optional dependencies. AI-Confidence: verified AI-Tags: typescript, type-safety, optional-dependencies, sdk-loading, eslint, type-casting, code-quality, api-design, dependency-management, structural-typing, dynamic-imports, require AI-Lifecycle: project AI-Memory-Id: ee99d9b6 AI-Source: llm-enrichment --- src/infrastructure/llm/GeminiLLMClient.ts | 21 +++++++++++++++++---- src/infrastructure/llm/OpenAILLMClient.ts | 21 +++++++++++++++++---- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/infrastructure/llm/GeminiLLMClient.ts b/src/infrastructure/llm/GeminiLLMClient.ts index b887e4e8..e239fa7b 100644 --- a/src/infrastructure/llm/GeminiLLMClient.ts +++ b/src/infrastructure/llm/GeminiLLMClient.ts @@ -39,12 +39,25 @@ export class GeminiLLMClient extends BaseLLMClient { userMessage: string, maxTokens?: number, ): Promise { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let GoogleGenerativeAI: any; + type GeminiModel = { + generateContent: (params: { + contents: Array<{ role: string; parts: Array<{ text: string }> }>; + generationConfig: { maxOutputTokens: number }; + }) => Promise<{ + response: { + text: () => string; + usageMetadata?: { promptTokenCount?: number; candidatesTokenCount?: number }; + }; + }>; + }; + type GenAIConstructor = new (apiKey: string) => { + getGenerativeModel: (opts: { model: string; systemInstruction: string }) => GeminiModel; + }; + let GoogleGenerativeAI: GenAIConstructor; try { // eslint-disable-next-line @typescript-eslint/no-require-imports - const mod = require('@google/generative-ai'); - GoogleGenerativeAI = mod.GoogleGenerativeAI; + const mod: unknown = require('@google/generative-ai'); + GoogleGenerativeAI = (mod as { GoogleGenerativeAI: GenAIConstructor }).GoogleGenerativeAI; } catch { throw new LLMError( 'Google Generative AI SDK not installed. Run: npm install @google/generative-ai', diff --git a/src/infrastructure/llm/OpenAILLMClient.ts b/src/infrastructure/llm/OpenAILLMClient.ts index f994d41b..044b86e9 100644 --- a/src/infrastructure/llm/OpenAILLMClient.ts +++ b/src/infrastructure/llm/OpenAILLMClient.ts @@ -39,12 +39,25 @@ export class OpenAILLMClient extends BaseLLMClient { userMessage: string, maxTokens?: number, ): Promise { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let OpenAI: any; + type OpenAIConstructor = new (opts: { apiKey: string }) => { + chat: { + completions: { + create: (params: { + model: string; + max_tokens: number; + messages: Array<{ role: 'system' | 'user'; content: string }>; + }) => Promise<{ + choices: Array<{ message?: { content?: string } }>; + usage?: { prompt_tokens?: number; completion_tokens?: number }; + }>; + }; + }; + }; + let OpenAI: OpenAIConstructor; try { // eslint-disable-next-line @typescript-eslint/no-require-imports - const mod = require('openai'); - OpenAI = mod.default ?? mod; + const mod: unknown = require('openai'); + OpenAI = ((mod as { default?: OpenAIConstructor }).default ?? mod) as OpenAIConstructor; } catch { throw new LLMError( 'OpenAI SDK not installed. Run: npm install openai',