diff --git a/apps/web/.env.example b/apps/web/.env.example index b15779a5..77ed9805 100644 --- a/apps/web/.env.example +++ b/apps/web/.env.example @@ -19,3 +19,9 @@ MARBLE_WORKSPACE_KEY=your_workspace_key_here FREESOUND_CLIENT_ID=your_client_id_here FREESOUND_API_KEY=your_api_key_here + +# LLM provider configuration (required for LLM agent) +LLM_PROVIDER=openai-compatible +LLM_API_KEY=your_openai_api_key_here +LLM_MODEL=gpt-4o +# LLM_BASE_URL=https://api.openai.com/v1 diff --git a/apps/web/package.json b/apps/web/package.json index 004ee5d7..0b8a704d 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -51,6 +51,7 @@ "motion": "^12.18.1", "nanoid": "^5.1.5", "next": "16.1.3", + "openai": "^5.1.0", "next-themes": "^0.4.4", "opencut-wasm": "^0.2.5", "pg": "^8.16.2", diff --git a/apps/web/src/agent/__tests__/orchestrator.test.ts b/apps/web/src/agent/__tests__/orchestrator.test.ts index 55bbed0b..fc70aaf1 100644 --- a/apps/web/src/agent/__tests__/orchestrator.test.ts +++ b/apps/web/src/agent/__tests__/orchestrator.test.ts @@ -3,12 +3,86 @@ import { beforeEach, describe, expect, + jest, test, } from "bun:test"; -import { run } from "@/agent/orchestrator"; import { useChatStore } from "@/stores/chat-store"; import { useAgentStore } from "@/stores/agent-store"; -import type { AgentContext, ChatMessage } from "@/agent/types"; +import { toolRegistry } from "@/agent/tools/registry"; +import type { AgentContext, ToolDefinition } from "@/agent/types"; + +// --------------------------------------------------------------------------- +// Mock the transitive WASM dependency chain before loading the orchestrator. +// +// The orchestrator imports transcribe-video.tool which imports @/agent/context +// which imports @/core → opencut-wasm (WASM binary that can't load in test env). +// +// We mock @/agent/context to cut the entire WASM chain. +// We also mock @/lib/media/audio and @/services/transcription/service to prevent +// their side effects from loading. +// --------------------------------------------------------------------------- + +jest.mock("@/agent/context", () => ({ + EditorContextAdapter: { + getContext: () => ({ + projectId: null, + activeSceneId: null, + mediaAssets: [], + playbackTimeMs: 0, + }), + resolveAssetFile: () => null, + getAssetHasAudio: () => undefined, + }, +})); + +jest.mock("@/lib/media/audio", () => ({ + decodeAudioToFloat32: jest.fn(), +})); + +jest.mock("@/services/transcription/service", () => ({ + transcriptionService: { + transcribe: jest.fn(), + }, +})); + +// Dynamic import so mocks take effect before the module graph is resolved +const { run } = await import("@/agent/orchestrator"); + +// --------------------------------------------------------------------------- +// Test tools +// --------------------------------------------------------------------------- + +const testTool: ToolDefinition = { + name: "_test_tool", + description: "A test tool with required params", + parameters: [ + { key: "input", type: "string", required: true }, + { key: "count", type: "number", required: false }, + { key: "flag", type: "boolean", required: true }, + ], + execute: async (args) => ({ echoed: args.input, flag: args.flag }), +}; + +/** Tool that intentionally throws during execution — used to prove the + * orchestrator's resolveToolCalls catch block works at runtime. */ +const throwingTool: ToolDefinition = { + name: "_test_throwing_tool", + description: "A tool that always throws", + parameters: [ + { key: "payload", type: "string", required: true }, + ], + execute: async () => { + throw new Error("Transcription service unavailable"); + }, +}; + +// Register once (registry is a singleton — re-registering overwrites) +toolRegistry.register("_test_tool", testTool); +toolRegistry.register("_test_throwing_tool", throwingTool); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- const MOCK_CONTEXT: AgentContext = { projectId: "proj-1", @@ -28,10 +102,16 @@ function mockFetchResponse(data: unknown, status = 200) { ); } -function setMockFetch(fn: (input: RequestInfo | URL, init?: RequestInit) => Promise) { +function setMockFetch( + fn: (input: RequestInfo | URL, init?: RequestInit) => Promise, +) { globalThis.fetch = fn as typeof fetch; } +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + describe("orchestrator", () => { beforeEach(() => { useChatStore.getState().clearMessages(); @@ -42,116 +122,292 @@ describe("orchestrator", () => { globalThis.fetch = originalFetch; }); - test("happy path — no tool calls", async () => { + // ----------------------------------------------------------------------- + // Happy path — no tool calls + // ----------------------------------------------------------------------- + + test("appends assistant message when response has no tool calls", async () => { setMockFetch(() => - mockFetchResponse({ - content: "Hello! How can I help?", - })); + mockFetchResponse({ content: "Hello! How can I help?" }), + ); - const messages: ChatMessage[] = [ - { id: "1", role: "user", content: "Hi", timestamp: Date.now() }, - ]; + await run( + [{ id: "1", role: "user", content: "Hi", timestamp: Date.now() }], + MOCK_CONTEXT, + ); - await run(messages, MOCK_CONTEXT); + const chat = useChatStore.getState(); + const agent = useAgentStore.getState(); - const chatState = useChatStore.getState(); - const agentState = useAgentStore.getState(); - - // Assistant message appended - expect(chatState.messages).toHaveLength(1); - expect(chatState.messages[0].role).toBe("assistant"); - expect(chatState.messages[0].content).toBe("Hello! How can I help?"); - - // Loading cleared - expect(chatState.loading).toBe(false); - expect(chatState.error).toBeNull(); - - // Agent state reset - expect(agentState.status).toBe("idle"); - expect(agentState.activeTool).toBeNull(); - expect(agentState.context).toEqual(MOCK_CONTEXT); + expect(chat.messages).toHaveLength(1); + expect(chat.messages[0].role).toBe("assistant"); + expect(chat.messages[0].content).toBe("Hello! How can I help?"); + expect(chat.loading).toBe(false); + expect(chat.error).toBeNull(); + expect(agent.status).toBe("idle"); + expect(agent.context).toEqual(MOCK_CONTEXT); }); - test("tool call resolution — echo_context executes", async () => { + // ----------------------------------------------------------------------- + // Tool call → resolve → loop → final answer + // ----------------------------------------------------------------------- + + test("resolves tool call and loops for final answer", async () => { + let callCount = 0; + setMockFetch(() => { + callCount++; + if (callCount === 1) { + return mockFetchResponse({ + content: "Let me run the tool.", + toolCalls: [ + { + id: "tc_1", + name: "_test_tool", + args: { input: "hello", flag: true }, + }, + ], + }); + } + return mockFetchResponse({ + content: "Here is the final answer.", + }); + }); + + await run( + [{ id: "1", role: "user", content: "Go", timestamp: Date.now() }], + MOCK_CONTEXT, + ); + + const chat = useChatStore.getState(); + const agent = useAgentStore.getState(); + + // assistant (with toolCalls) + tool_result + assistant (final) + expect(chat.messages).toHaveLength(3); + expect(chat.messages[0].role).toBe("assistant"); + expect(chat.messages[0].toolCalls).toHaveLength(1); + expect(chat.messages[1].role).toBe("tool_result"); + expect(chat.messages[1].content).toContain("echoed"); + expect(chat.messages[2].role).toBe("assistant"); + expect(chat.messages[2].content).toBe("Here is the final answer."); + + expect(agent.status).toBe("idle"); + expect(chat.loading).toBe(false); + }); + + // ----------------------------------------------------------------------- + // Max-iteration guard + // ----------------------------------------------------------------------- + + test("stops at MAX_ITERATIONS and appends limit message", async () => { + // Every fetch response returns a tool call → loop hits the cap setMockFetch(() => mockFetchResponse({ - content: "Let me check your editor context.", + content: "Thinking...", toolCalls: [ - { id: "tc_1", name: "echo_context", args: {} }, + { + id: "tc_loop", + name: "_test_tool", + args: { input: "loop", flag: true }, + }, ], - })); + }), + ); - const messages: ChatMessage[] = [ - { id: "1", role: "user", content: "Show me context", timestamp: Date.now() }, - ]; + await run( + [{ id: "1", role: "user", content: "Loop", timestamp: Date.now() }], + MOCK_CONTEXT, + ); - await run(messages, MOCK_CONTEXT); + const chat = useChatStore.getState(); + const agent = useAgentStore.getState(); - const chatState = useChatStore.getState(); - const agentState = useAgentStore.getState(); + // 8 iterations × (1 assistant + 1 tool_result) + 1 cap message = 17 + expect(chat.messages).toHaveLength(17); - // Assistant message + tool result - expect(chatState.messages).toHaveLength(2); - expect(chatState.messages[0].role).toBe("assistant"); - expect(chatState.messages[1].role).toBe("tool_result"); + // Last message is the cap message + const lastMsg = chat.messages[chat.messages.length - 1]; + expect(lastMsg.role).toBe("assistant"); + expect(lastMsg.content).toContain("maximum number of reasoning steps"); - // Tool result should contain echo_context output - const toolResult = JSON.parse(chatState.messages[1].content); - expect(toolResult.projectId).toBe("proj-1"); - - // States cleaned up - expect(chatState.loading).toBe(false); - expect(agentState.status).toBe("idle"); - expect(agentState.context).toEqual(MOCK_CONTEXT); + expect(agent.status).toBe("idle"); + expect(chat.loading).toBe(false); }); - test("error path — API failure sets error and loading cleared", async () => { - setMockFetch(() => - mockFetchResponse({ error: "fail" }, 500)); + // ----------------------------------------------------------------------- + // Arg validation — missing required arg + // ----------------------------------------------------------------------- - const messages: ChatMessage[] = [ - { id: "1", role: "user", content: "Hi", timestamp: Date.now() }, - ]; + test("returns error result when required arg is missing", async () => { + let callCount = 0; + setMockFetch(() => { + callCount++; + if (callCount === 1) { + // Missing "input" (required) and "flag" (required) + return mockFetchResponse({ + content: "Running tool", + toolCalls: [ + { + id: "tc_val", + name: "_test_tool", + args: { count: 5 }, + }, + ], + }); + } + return mockFetchResponse({ content: "Done" }); + }); - await run(messages, MOCK_CONTEXT); + await run( + [{ id: "1", role: "user", content: "Test", timestamp: Date.now() }], + MOCK_CONTEXT, + ); - const chatState = useChatStore.getState(); - const agentState = useAgentStore.getState(); + const chat = useChatStore.getState(); - // Error set - expect(chatState.error).toBeTruthy(); - expect(chatState.loading).toBe(false); + // assistant (toolCalls) + tool_result (error) + assistant (final) + expect(chat.messages).toHaveLength(3); - // Agent in error state - expect(agentState.status).toBe("error"); - - // No assistant message added - expect(chatState.messages).toHaveLength(0); + const toolResult = chat.messages[1]; + expect(toolResult.role).toBe("tool_result"); + expect(toolResult.content).toContain("Missing required argument"); + expect(toolResult.content).toContain("input"); }); - test("error path — network failure sets error", async () => { + // ----------------------------------------------------------------------- + // Tool execution throws inside resolveToolCalls — runtime behavioral proof + // ----------------------------------------------------------------------- + + test("recovers when tool.execute() throws, continues loop to final answer", async () => { + let callCount = 0; + setMockFetch(() => { + callCount++; + if (callCount === 1) { + return mockFetchResponse({ + content: "Running throwing tool.", + toolCalls: [ + { + id: "tc_throw", + name: "_test_throwing_tool", + args: { payload: "boom" }, + }, + ], + }); + } + return mockFetchResponse({ content: "Recovered successfully." }); + }); + + await run( + [{ id: "1", role: "user", content: "Throw test", timestamp: Date.now() }], + MOCK_CONTEXT, + ); + + const chat = useChatStore.getState(); + const agent = useAgentStore.getState(); + + // assistant (with toolCalls) + tool_result (error) + assistant (final) + expect(chat.messages).toHaveLength(3); + + // The tool_result should contain the thrown error message, NOT crash the orchestrator + const toolResult = chat.messages[1]; + expect(toolResult.role).toBe("tool_result"); + expect(toolResult.content).toContain("Error in _test_throwing_tool"); + expect(toolResult.content).toContain("Transcription service unavailable"); + + // Loop continues — final answer arrives + const finalMsg = chat.messages[2]; + expect(finalMsg.role).toBe("assistant"); + expect(finalMsg.content).toBe("Recovered successfully."); + + // Orchestrator ends cleanly — no global error state + expect(agent.status).toBe("idle"); + expect(chat.loading).toBe(false); + expect(chat.error).toBeNull(); + }); + + // ----------------------------------------------------------------------- + // Arg validation — wrong type + // ----------------------------------------------------------------------- + + test("returns error result when arg has wrong type", async () => { + let callCount = 0; + setMockFetch(() => { + callCount++; + if (callCount === 1) { + // flag is boolean required, but we pass a string + return mockFetchResponse({ + content: "Running tool", + toolCalls: [ + { + id: "tc_type", + name: "_test_tool", + args: { input: "hello", flag: "not-a-bool" }, + }, + ], + }); + } + return mockFetchResponse({ content: "Done" }); + }); + + await run( + [{ id: "1", role: "user", content: "Test", timestamp: Date.now() }], + MOCK_CONTEXT, + ); + + const chat = useChatStore.getState(); + const toolResult = chat.messages[1]; + expect(toolResult.role).toBe("tool_result"); + expect(toolResult.content).toContain("flag"); + expect(toolResult.content).toContain("must be a boolean"); + }); + + // ----------------------------------------------------------------------- + // Error paths + // ----------------------------------------------------------------------- + + test("sets error state on API failure", async () => { + setMockFetch(() => mockFetchResponse({ error: "fail" }, 500)); + + await run( + [{ id: "1", role: "user", content: "Hi", timestamp: Date.now() }], + MOCK_CONTEXT, + ); + + const chat = useChatStore.getState(); + const agent = useAgentStore.getState(); + + expect(chat.error).toBeTruthy(); + expect(chat.loading).toBe(false); + expect(agent.status).toBe("error"); + expect(chat.messages).toHaveLength(0); + }); + + test("sets error state on network failure", async () => { setMockFetch(() => Promise.reject(new Error("Network error"))); - const messages: ChatMessage[] = [ - { id: "1", role: "user", content: "Hi", timestamp: Date.now() }, - ]; + await run( + [{ id: "1", role: "user", content: "Hi", timestamp: Date.now() }], + MOCK_CONTEXT, + ); - await run(messages, MOCK_CONTEXT); - - const chatState = useChatStore.getState(); - expect(chatState.error).toBe("Network error"); - expect(chatState.loading).toBe(false); + const chat = useChatStore.getState(); + expect(chat.error).toBe("Network error"); + expect(chat.loading).toBe(false); expect(useAgentStore.getState().status).toBe("error"); }); - test("context is stored in agentStore", async () => { - setMockFetch(() => - mockFetchResponse({ content: "ok" })); + // ----------------------------------------------------------------------- + // Context storage + // ----------------------------------------------------------------------- + + test("stores context in agentStore", async () => { + setMockFetch(() => mockFetchResponse({ content: "ok" })); const context: AgentContext = { projectId: "special-proj", activeSceneId: "scene-X", - mediaAssets: [{ id: "m1", name: "a.mp4", type: "video", duration: 10 }], + mediaAssets: [ + { id: "m1", name: "a.mp4", type: "video", duration: 10 }, + ], playbackTimeMs: 1234, }; diff --git a/apps/web/src/agent/__tests__/system-prompt.test.ts b/apps/web/src/agent/__tests__/system-prompt.test.ts index 87956013..5c83fe82 100644 --- a/apps/web/src/agent/__tests__/system-prompt.test.ts +++ b/apps/web/src/agent/__tests__/system-prompt.test.ts @@ -2,7 +2,18 @@ import { describe, expect, test } from "bun:test"; import { buildSystemPrompt } from "@/agent/system-prompt"; import type { AgentContext } from "@/agent/types"; +const BASE_CONTEXT: AgentContext = { + projectId: "proj-1", + activeSceneId: "scene-A", + mediaAssets: [], + playbackTimeMs: 0, +}; + describe("buildSystemPrompt", () => { + // ----------------------------------------------------------------------- + // Media context + // ----------------------------------------------------------------------- + test("includes media assets when present", () => { const context: AgentContext = { projectId: "proj-1", @@ -56,4 +67,55 @@ describe("buildSystemPrompt", () => { expect(prompt).toContain("No active scene"); expect(prompt).toContain("No media assets loaded."); }); + + // ----------------------------------------------------------------------- + // Tool guidance + // ----------------------------------------------------------------------- + + test("includes tool guidance section when tools are provided", () => { + const tools = [ + { name: "transcribe_video", description: "Transcribes video audio" }, + ]; + + const prompt = buildSystemPrompt(BASE_CONTEXT, tools); + + expect(prompt).toContain("Available tools:"); + expect(prompt).toContain("transcribe_video"); + expect(prompt).toContain("Transcribes video audio"); + }); + + test("includes plain-text fallback instruction when tools are provided", () => { + const tools = [ + { name: "transcribe_video", description: "Transcribes video audio" }, + ]; + + const prompt = buildSystemPrompt(BASE_CONTEXT, tools); + + expect(prompt).toContain("plain text"); + }); + + test("lists multiple tools when provided", () => { + const tools = [ + { name: "transcribe_video", description: "Transcribes audio" }, + { name: "analyze_scene", description: "Analyzes scene content" }, + ]; + + const prompt = buildSystemPrompt(BASE_CONTEXT, tools); + + expect(prompt).toContain("transcribe_video"); + expect(prompt).toContain("analyze_scene"); + }); + + test("omits tool guidance when no tools are provided", () => { + const prompt = buildSystemPrompt(BASE_CONTEXT); + + expect(prompt).not.toContain("Available tools:"); + expect(prompt).not.toContain("plain text"); + }); + + test("omits tool guidance when tools array is empty", () => { + const prompt = buildSystemPrompt(BASE_CONTEXT, []); + + expect(prompt).not.toContain("Available tools:"); + }); }); diff --git a/apps/web/src/agent/orchestrator.ts b/apps/web/src/agent/orchestrator.ts index 54baf782..89625958 100644 --- a/apps/web/src/agent/orchestrator.ts +++ b/apps/web/src/agent/orchestrator.ts @@ -2,27 +2,32 @@ import type { AgentContext, ChatMessage, ToolCall, + ToolDefinition, ToolResult, } from "@/agent/types"; import { toolRegistry } from "@/agent/tools/registry"; -import "@/agent/tools/mock.tool"; import "@/agent/tools/transcribe-video.tool"; import { useChatStore } from "@/stores/chat-store"; import { useAgentStore } from "@/stores/agent-store"; +/** Hard cap on multi-turn iterations (user-specified). */ +const MAX_ITERATIONS = 8; + interface APIResponse { content: string; toolCalls?: ToolCall[]; } /** - * Client-side orchestrator — single-pass only (v1). + * Client-side orchestrator — multi-turn loop (v2). * - * 1. Sets agent status → sends - * 2. POST to /api/agent/chat - * 3. Resolves any tool calls via the registry - * 4. Appends assistant message + tool results to chatStore - * 5. Returns to idle (or error) + * 1. Sets agent status → sending + * 2. Loop (max MAX_ITERATIONS): + * a. POST current message history to /api/agent/chat + * b. If toolCalls → validate args, resolve via registry, append results → loop + * c. If no toolCalls → append final assistant message → break + * 3. On max-iteration cap → append limit message, idle + * 4. Error → chatStore.error + error status */ export async function run( messages: ChatMessage[], @@ -34,40 +39,88 @@ export async function run( agentStore.setContext(context); agentStore.setStatus("sending"); + // Working copy of messages that grows with each turn + const workingMessages = [...messages]; + try { - const response = await fetch("/api/agent/chat", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ messages, context }), - }); + let iterations = 0; + let hitCap = false; - if (!response.ok) { - throw new Error(`API error: ${response.status}`); - } + while (iterations < MAX_ITERATIONS) { + iterations++; - const data: APIResponse = await response.json(); + const response = await fetch("/api/agent/chat", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ messages: workingMessages, context }), + }); - // Resolve tool calls if present - let toolResults: ToolResult[] = []; - if (data.toolCalls && data.toolCalls.length > 0) { + if (!response.ok) { + throw new Error(`API error: ${response.status}`); + } + + const data: APIResponse = await response.json(); + + // No tool calls → final answer, done + if (!data.toolCalls || data.toolCalls.length === 0) { + chatStore.addMessage({ + role: "assistant", + content: data.content, + }); + break; + } + + // Tool calls present → validate, resolve, append, continue agentStore.setStatus("processing"); - toolResults = await resolveToolCalls(data.toolCalls, context); + + // Append assistant message (with toolCalls) to chat + working history + chatStore.addMessage({ + role: "assistant", + content: data.content, + toolCalls: data.toolCalls, + }); + workingMessages.push({ + id: "", + role: "assistant", + content: data.content, + toolCalls: data.toolCalls, + timestamp: Date.now(), + }); + + // Resolve each tool call (validates args before execution) + const toolResults = await resolveToolCalls(data.toolCalls, context); + + // Append tool results to chat + working history + for (const tr of toolResults) { + const content = tr.error + ? `Error in ${tr.name}: ${tr.error}` + : JSON.stringify(tr.result); + + chatStore.addMessage({ + role: "tool_result", + content, + toolCallId: tr.toolCallId, + }); + workingMessages.push({ + id: "", + role: "tool_result", + content, + toolCallId: tr.toolCallId, + timestamp: Date.now(), + }); + } + + // If this was the last allowed iteration, mark cap hit + if (iterations >= MAX_ITERATIONS) { + hitCap = true; + } } - // Append assistant response - chatStore.addMessage({ - role: "assistant", - content: data.content, - toolCalls: data.toolCalls, - }); - - // Append tool results as separate messages - for (const tr of toolResults) { + if (hitCap) { chatStore.addMessage({ - role: "tool_result", - content: tr.error - ? `Error in ${tr.name}: ${tr.error}` - : JSON.stringify(tr.result), + role: "assistant", + content: + "I've reached the maximum number of reasoning steps. Please continue the conversation for further assistance.", }); } @@ -81,6 +134,46 @@ export async function run( } } +// --------------------------------------------------------------------------- +// Arg validation +// --------------------------------------------------------------------------- + +/** + * Validates tool arguments against the tool's parameter schema. + * Returns an error string on failure, or null if args are valid. + */ +function validateToolArgs( + toolDef: ToolDefinition, + args: Record, +): string | null { + for (const param of toolDef.parameters) { + if (!param.required) continue; + + const value = args[param.key]; + + if (value === undefined || value === null) { + return `Missing required argument: ${param.key}`; + } + + const actualType = typeof value; + if (param.type === "string" && actualType !== "string") { + return `Argument "${param.key}" must be a string, got ${actualType}`; + } + if (param.type === "number" && actualType !== "number") { + return `Argument "${param.key}" must be a number, got ${actualType}`; + } + if (param.type === "boolean" && actualType !== "boolean") { + return `Argument "${param.key}" must be a boolean, got ${actualType}`; + } + } + + return null; +} + +// --------------------------------------------------------------------------- +// Tool resolution +// --------------------------------------------------------------------------- + async function resolveToolCalls( toolCalls: ToolCall[], context: AgentContext, @@ -93,6 +186,19 @@ async function resolveToolCalls( try { const tool = toolRegistry.get(tc.name); + + // Validate args BEFORE execution + const validationError = validateToolArgs(tool, tc.args); + if (validationError) { + results.push({ + toolCallId: tc.id, + name: tc.name, + result: null, + error: validationError, + }); + continue; + } + const result = await tool.execute(tc.args, context); results.push({ toolCallId: tc.id, name: tc.name, result }); } catch (error) { diff --git a/apps/web/src/agent/providers/__tests__/index.test.ts b/apps/web/src/agent/providers/__tests__/index.test.ts new file mode 100644 index 00000000..913475a5 --- /dev/null +++ b/apps/web/src/agent/providers/__tests__/index.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, test } from "bun:test"; +import { createProvider } from "@/agent/providers/index"; +import { OpenAICompatibleAdapter } from "@/agent/providers/openai-compatible"; +import type { ProviderConfig } from "@/agent/providers/types"; + +describe("createProvider", () => { + test("returns OpenAICompatibleAdapter for openai-compatible provider", () => { + const config: ProviderConfig = { + provider: "openai-compatible", + apiKey: "test-key", + model: "gpt-4o-mini", + }; + + const adapter = createProvider(config); + expect(adapter).toBeInstanceOf(OpenAICompatibleAdapter); + }); + + test("passes config through to adapter", () => { + const config: ProviderConfig = { + provider: "openai-compatible", + apiKey: "sk-test-123", + model: "gpt-4o", + baseUrl: "https://api.custom.com/v1", + }; + + const adapter = createProvider(config); + expect(adapter).toBeInstanceOf(OpenAICompatibleAdapter); + // Adapter was constructed without throwing — config accepted + }); + + test("throws Error for unknown provider", () => { + const config: ProviderConfig = { + provider: "unknown-provider", + apiKey: "test-key", + model: "test-model", + }; + + expect(() => createProvider(config)).toThrow( + "Unknown LLM provider: unknown-provider", + ); + }); + + test("throws for empty provider string", () => { + const config: ProviderConfig = { + provider: "", + apiKey: "test-key", + model: "test-model", + }; + + expect(() => createProvider(config)).toThrow("Unknown LLM provider: "); + }); +}); diff --git a/apps/web/src/agent/providers/__tests__/openai-compatible.test.ts b/apps/web/src/agent/providers/__tests__/openai-compatible.test.ts new file mode 100644 index 00000000..0139ffb0 --- /dev/null +++ b/apps/web/src/agent/providers/__tests__/openai-compatible.test.ts @@ -0,0 +1,295 @@ +import { beforeEach, describe, expect, jest, test } from "bun:test"; +import type { ProviderConfig } from "@/agent/providers/types"; +import type { ToolDefinition } from "@/agent/types"; + +// --------------------------------------------------------------------------- +// Mock the openai SDK before any module that imports it is loaded +// --------------------------------------------------------------------------- + +const mockCreate = jest.fn(); + +jest.mock("openai", () => ({ + default: class MockOpenAI { + chat = { + completions: { + create: mockCreate, + }, + }; + constructor(_opts: { apiKey: string; baseURL?: string }) {} + }, +})); + +import { OpenAICompatibleAdapter } from "@/agent/providers/openai-compatible"; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const TEST_CONFIG: ProviderConfig = { + provider: "openai-compatible", + apiKey: "sk-test", + model: "gpt-4o-mini", +}; + +const SIMPLE_TOOLS: ToolDefinition[] = [ + { + name: "test_tool", + description: "A test tool", + parameters: [ + { key: "input", type: "string", required: true }, + ], + execute: async () => ({}), + }, +]; + +function makeOpenAIResponse(overrides: { + content?: string; + toolCalls?: Array<{ id: string; name: string; arguments: string }>; +}) { + return { + choices: [ + { + message: { + content: overrides.content ?? "Hello!", + tool_calls: overrides.toolCalls?.map((tc) => ({ + id: tc.id, + type: "function", + function: { name: tc.name, arguments: tc.arguments }, + })), + }, + }, + ], + }; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("OpenAICompatibleAdapter.chat()", () => { + beforeEach(() => { + mockCreate.mockReset(); + }); + + test("returns content-only response", async () => { + mockCreate.mockResolvedValueOnce( + makeOpenAIResponse({ content: "Hi there!" }), + ); + + const adapter = new OpenAICompatibleAdapter(TEST_CONFIG); + const result = await adapter.chat({ + messages: [{ id: "1", role: "user", content: "Hey", timestamp: 0 }], + systemPrompt: "You are helpful.", + tools: [], + }); + + expect(result.content).toBe("Hi there!"); + expect(result.toolCalls).toBeUndefined(); + }); + + test("returns tool calls from provider response", async () => { + mockCreate.mockResolvedValueOnce( + makeOpenAIResponse({ + content: "", + toolCalls: [ + { + id: "tc_1", + name: "test_tool", + arguments: '{"input":"hello"}', + }, + ], + }), + ); + + const adapter = new OpenAICompatibleAdapter(TEST_CONFIG); + const result = await adapter.chat({ + messages: [{ id: "1", role: "user", content: "Run tool", timestamp: 0 }], + systemPrompt: "System", + tools: SIMPLE_TOOLS, + }); + + expect(result.content).toBe(""); + expect(result.toolCalls).toHaveLength(1); + expect(result.toolCalls?.[0]).toEqual({ + id: "tc_1", + name: "test_tool", + args: { input: "hello" }, + }); + }); + + test("prepends system prompt as first message", async () => { + mockCreate.mockResolvedValueOnce(makeOpenAIResponse({ content: "ok" })); + + const adapter = new OpenAICompatibleAdapter(TEST_CONFIG); + await adapter.chat({ + messages: [{ id: "1", role: "user", content: "Hi", timestamp: 0 }], + systemPrompt: "Custom system prompt", + tools: [], + }); + + const callArgs = mockCreate.mock.calls[0][0] as { + messages: Array>; + }; + expect(callArgs.messages[0]).toEqual({ + role: "system", + content: "Custom system prompt", + }); + }); + + test("converts user messages to OpenAI format", async () => { + mockCreate.mockResolvedValueOnce(makeOpenAIResponse({ content: "ok" })); + + const adapter = new OpenAICompatibleAdapter(TEST_CONFIG); + await adapter.chat({ + messages: [ + { id: "1", role: "user", content: "Hello", timestamp: 0 }, + ], + systemPrompt: "System", + tools: [], + }); + + const callArgs = mockCreate.mock.calls[0][0] as { + messages: Array>; + }; + // messages[0] is system, messages[1] is user + expect(callArgs.messages[1]).toEqual({ + role: "user", + content: "Hello", + }); + }); + + test("converts assistant messages with tool calls", async () => { + mockCreate.mockResolvedValueOnce(makeOpenAIResponse({ content: "ok" })); + + const adapter = new OpenAICompatibleAdapter(TEST_CONFIG); + await adapter.chat({ + messages: [ + { + id: "1", + role: "assistant", + content: "Let me check.", + toolCalls: [ + { id: "tc_1", name: "test_tool", args: { input: "x" } }, + ], + timestamp: 0, + }, + ], + systemPrompt: "System", + tools: [], + }); + + const callArgs = mockCreate.mock.calls[0][0] as { + messages: Array>; + }; + const assistantMsg = callArgs.messages[1]; + expect(assistantMsg.role).toBe("assistant"); + expect(assistantMsg.content).toBe("Let me check."); + expect(assistantMsg.tool_calls).toEqual([ + { + id: "tc_1", + type: "function", + function: { name: "test_tool", arguments: '{"input":"x"}' }, + }, + ]); + }); + + test("converts tool_result messages to OpenAI tool format", async () => { + mockCreate.mockResolvedValueOnce(makeOpenAIResponse({ content: "ok" })); + + const adapter = new OpenAICompatibleAdapter(TEST_CONFIG); + await adapter.chat({ + messages: [ + { + id: "1", + role: "tool_result", + content: '{"result":"data"}', + toolCallId: "tc_1", + timestamp: 0, + }, + ], + systemPrompt: "System", + tools: [], + }); + + const callArgs = mockCreate.mock.calls[0][0] as { + messages: Array>; + }; + expect(callArgs.messages[1]).toEqual({ + role: "tool", + tool_call_id: "tc_1", + content: '{"result":"data"}', + }); + }); + + test("converts tool definitions to OpenAI function format", async () => { + mockCreate.mockResolvedValueOnce(makeOpenAIResponse({ content: "ok" })); + + const adapter = new OpenAICompatibleAdapter(TEST_CONFIG); + await adapter.chat({ + messages: [{ id: "1", role: "user", content: "Go", timestamp: 0 }], + systemPrompt: "System", + tools: SIMPLE_TOOLS, + }); + + const callArgs = mockCreate.mock.calls[0][0] as { + tools: Array>; + }; + expect(callArgs.tools).toHaveLength(1); + expect(callArgs.tools[0]).toEqual({ + type: "function", + function: { + name: "test_tool", + description: "A test tool", + parameters: { + type: "object", + properties: { input: { type: "string" } }, + required: ["input"], + }, + }, + }); + }); + + test("throws when provider returns empty choices", async () => { + mockCreate.mockResolvedValueOnce({ choices: [] }); + + const adapter = new OpenAICompatibleAdapter(TEST_CONFIG); + + expect( + adapter.chat({ + messages: [{ id: "1", role: "user", content: "Hi", timestamp: 0 }], + systemPrompt: "System", + tools: [], + }), + ).rejects.toThrow("Empty response from AI provider"); + }); + + test("defaults content to empty string when null", async () => { + mockCreate.mockResolvedValueOnce({ + choices: [{ message: { content: null } }], + }); + + const adapter = new OpenAICompatibleAdapter(TEST_CONFIG); + const result = await adapter.chat({ + messages: [{ id: "1", role: "user", content: "Hi", timestamp: 0 }], + systemPrompt: "System", + tools: [], + }); + + expect(result.content).toBe(""); + expect(result.toolCalls).toBeUndefined(); + }); + + test("passes model from config to API call", async () => { + mockCreate.mockResolvedValueOnce(makeOpenAIResponse({ content: "ok" })); + + const adapter = new OpenAICompatibleAdapter(TEST_CONFIG); + await adapter.chat({ + messages: [{ id: "1", role: "user", content: "Hi", timestamp: 0 }], + systemPrompt: "System", + tools: [], + }); + + const callArgs = mockCreate.mock.calls[0][0] as { model: string }; + expect(callArgs.model).toBe("gpt-4o-mini"); + }); +}); diff --git a/apps/web/src/agent/providers/index.ts b/apps/web/src/agent/providers/index.ts new file mode 100644 index 00000000..5bbb2cdb --- /dev/null +++ b/apps/web/src/agent/providers/index.ts @@ -0,0 +1,20 @@ +import type { ProviderAdapter, ProviderConfig } from "./types"; +import { OpenAICompatibleAdapter } from "./openai-compatible"; + +/** + * Factory — resolves a ProviderConfig to a concrete adapter. + * + * Extend the switch when new provider families are added. + * Unknown providers throw immediately so mis-config is caught + * at route call time, not silently. + */ +export function createProvider(config: ProviderConfig): ProviderAdapter { + switch (config.provider) { + case "openai-compatible": + return new OpenAICompatibleAdapter(config); + default: + throw new Error(`Unknown LLM provider: ${config.provider}`); + } +} + +export type { ProviderAdapter, ProviderConfig, ProviderResponse } from "./types"; diff --git a/apps/web/src/agent/providers/openai-compatible.ts b/apps/web/src/agent/providers/openai-compatible.ts new file mode 100644 index 00000000..7a2423b6 --- /dev/null +++ b/apps/web/src/agent/providers/openai-compatible.ts @@ -0,0 +1,167 @@ +import OpenAI from "openai"; +import type { ChatMessage, ToolCall, ToolSchema } from "@/agent/types"; +import type { + ProviderAdapter, + ProviderConfig, + ProviderResponse, +} from "./types"; + +// --------------------------------------------------------------------------- +// OpenAI wire-format types (private to this adapter) +// --------------------------------------------------------------------------- + +interface OpenAIFunctionTool { + type: "function"; + function: { + name: string; + description: string; + parameters: { + type: "object"; + properties: Record; + required?: string[]; + }; + }; +} + +// --------------------------------------------------------------------------- +// Internal conversion helpers +// --------------------------------------------------------------------------- + +/** + * Converts internal ChatMessages into OpenAI chat-completion message format. + * Prepends the system prompt as the first message. + */ +function toOpenAIMessages( + messages: ChatMessage[], + systemPrompt: string, +): Array> { + const result: Array> = [ + { role: "system", content: systemPrompt }, + ]; + + for (const msg of messages) { + if (msg.role === "user") { + result.push({ role: "user", content: msg.content }); + } else if (msg.role === "assistant") { + const entry: Record = { + role: "assistant", + content: msg.content, + }; + if (msg.toolCalls && msg.toolCalls.length > 0) { + entry.tool_calls = msg.toolCalls.map((tc) => ({ + id: tc.id, + type: "function", + function: { + name: tc.name, + arguments: JSON.stringify(tc.args), + }, + })); + } + result.push(entry); + } else if (msg.role === "tool_result") { + result.push({ + role: "tool", + tool_call_id: msg.toolCallId ?? "", + content: msg.content, + }); + } + } + + return result; +} + +/** + * Converts provider-agnostic ToolDefinitions into OpenAI + * function-calling tool format. + */ +function toOpenAIFunctions(tools: ToolSchema[]): OpenAIFunctionTool[] { + return tools.map((tool) => ({ + type: "function" as const, + function: { + name: tool.name, + description: tool.description, + parameters: { + type: "object" as const, + properties: Object.fromEntries( + tool.parameters.map((p) => [p.key, { type: p.type }]), + ), + ...(tool.parameters.some((p) => p.required) && { + required: tool.parameters + .filter((p) => p.required) + .map((p) => p.key), + }), + }, + }, + })); +} + +// --------------------------------------------------------------------------- +// Adapter implementation +// --------------------------------------------------------------------------- + +/** + * OpenAI-compatible adapter. + * + * Covers OpenAI, Groq, local Ollama, or any baseUrl-driven provider + * that speaks the OpenAI chat-completions API. + */ +export class OpenAICompatibleAdapter implements ProviderAdapter { + private client: OpenAI; + private model: string; + + constructor(config: ProviderConfig) { + this.client = new OpenAI({ + apiKey: config.apiKey, + ...(config.baseUrl && { baseURL: config.baseUrl }), + }); + this.model = config.model; + } + + async chat(params: { + messages: ChatMessage[]; + systemPrompt: string; + tools: ToolSchema[]; + }): Promise { + const openaiMessages = toOpenAIMessages( + params.messages, + params.systemPrompt, + ); + const openaiTools = toOpenAIFunctions(params.tools); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const requestParams: any = { + model: this.model, + messages: openaiMessages, + }; + + if (openaiTools.length > 0) { + requestParams.tools = openaiTools; + } + + const response = + await this.client.chat.completions.create(requestParams); + + const choice = response.choices[0]; + if (!choice) { + throw new Error("Empty response from AI provider"); + } + + const content = choice.message.content ?? ""; + let toolCalls: ToolCall[] | undefined; + + if ( + choice.message.tool_calls && + choice.message.tool_calls.length > 0 + ) { + toolCalls = choice.message.tool_calls + .filter((tc): tc is Extract => tc.type === "function") + .map((tc) => ({ + id: tc.id, + name: tc.function.name, + args: JSON.parse(tc.function.arguments), + })); + } + + return { content, ...(toolCalls && { toolCalls }) }; + } +} diff --git a/apps/web/src/agent/providers/types.ts b/apps/web/src/agent/providers/types.ts new file mode 100644 index 00000000..fd885ddf --- /dev/null +++ b/apps/web/src/agent/providers/types.ts @@ -0,0 +1,36 @@ +import type { ChatMessage, ToolCall, ToolSchema } from "@/agent/types"; + +/** + * Provider-agnostic configuration — read from LLM_* env vars. + * Never exposed to the client. + */ +export interface ProviderConfig { + provider: string; + apiKey: string; + model: string; + baseUrl?: string; +} + +/** + * Canonical response shape — every adapter returns this. + * The route passes this straight through to the client. + */ +export interface ProviderResponse { + content: string; + toolCalls?: ToolCall[]; +} + +/** + * Adapter interface — one implementation per provider family. + * + * The adapter owns message-format conversion, tool-schema conversion, + * and the actual API call. Callers (the route) never touch the wire + * format directly. + */ +export interface ProviderAdapter { + chat(params: { + messages: ChatMessage[]; + systemPrompt: string; + tools: ToolSchema[]; + }): Promise; +} diff --git a/apps/web/src/agent/system-prompt.ts b/apps/web/src/agent/system-prompt.ts index e263cd86..26b4229c 100644 --- a/apps/web/src/agent/system-prompt.ts +++ b/apps/web/src/agent/system-prompt.ts @@ -1,16 +1,42 @@ import type { AgentContext } from "@/agent/types"; -export function buildSystemPrompt(context: AgentContext): string { +export interface ToolSummary { + name: string; + description: string; +} + +/** + * Builds the system prompt for the LLM from the editor context. + * + * Pure function — no client-only imports (no EditorCore, WASM, browser APIs). + * Consumable server-side in the API route. + */ +export function buildSystemPrompt( + context: AgentContext, + tools?: ToolSummary[], +): string { const mediaSection = context.mediaAssets.length > 0 ? `Active media assets:\n${context.mediaAssets.map((m) => `- ${m.name} (${m.type}, ${m.duration}s)`).join("\n")}` : "No media assets loaded."; - return [ + const parts = [ "You are an AI assistant embedded in the NeuralCut video editor.", `Project: ${context.projectId ?? "No project loaded"}`, `Active scene: ${context.activeSceneId ?? "No active scene"}`, `Playback position: ${context.playbackTimeMs}ms`, mediaSection, - ].join("\n"); + ]; + + if (tools && tools.length > 0) { + const toolList = tools + .map((t) => `- ${t.name}: ${t.description}`) + .join("\n"); + parts.push( + `Available tools:\n${toolList}`, + "When you need to perform an action matching one of these tools, call the appropriate tool. For all other requests, respond directly in plain text.", + ); + } + + return parts.join("\n"); } diff --git a/apps/web/src/agent/tools/__tests__/registry.test.ts b/apps/web/src/agent/tools/__tests__/registry.test.ts new file mode 100644 index 00000000..851ded78 --- /dev/null +++ b/apps/web/src/agent/tools/__tests__/registry.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, test } from "bun:test"; +import { toToolSchemas } from "@/agent/tools/registry"; +import type { ToolDefinition } from "@/agent/types"; + +describe("toToolSchemas()", () => { + test("strips execute from tool definitions, returning pure data schemas", () => { + const tools: ToolDefinition[] = [ + { + name: "test_tool", + description: "A test tool", + parameters: [ + { key: "input", type: "string", required: true }, + { key: "count", type: "number", required: false }, + ], + execute: async () => ({ result: "never called" }), + }, + ]; + + const schemas = toToolSchemas(tools); + + expect(schemas).toHaveLength(1); + expect(schemas[0]).toEqual({ + name: "test_tool", + description: "A test tool", + parameters: [ + { key: "input", type: "string", required: true }, + { key: "count", type: "number", required: false }, + ], + }); + + // Confirm no execute property leaked + expect("execute" in schemas[0]).toBe(false); + }); + + test("returns empty array for empty input", () => { + expect(toToolSchemas([])).toEqual([]); + }); + + test("preserves all parameter metadata including required flag", () => { + const tools: ToolDefinition[] = [ + { + name: "tool_with_required", + description: "Has required params", + parameters: [ + { key: "a", type: "string", required: true }, + { key: "b", type: "number", required: true }, + { key: "c", type: "boolean", required: false }, + ], + execute: async () => ({}), + }, + ]; + + const schemas = toToolSchemas(tools); + expect(schemas[0].parameters).toEqual([ + { key: "a", type: "string", required: true }, + { key: "b", type: "number", required: true }, + { key: "c", type: "boolean", required: false }, + ]); + }); +}); diff --git a/apps/web/src/agent/tools/__tests__/transcribe-video.test.ts b/apps/web/src/agent/tools/__tests__/transcribe-video.test.ts index e3013947..af52caea 100644 --- a/apps/web/src/agent/tools/__tests__/transcribe-video.test.ts +++ b/apps/web/src/agent/tools/__tests__/transcribe-video.test.ts @@ -31,7 +31,9 @@ mock.module("@/agent/context", () => ({ resolveAssetFile: mockResolveAssetFile, getAssetHasAudio: mockGetAssetHasAudio, }, - buildSystemPrompt: () => "", + // Do NOT override buildSystemPrompt — it is not needed by this test + // and the mock persists globally across test files in Bun, breaking + // route tests that depend on the real implementation. })); mock.module("@/lib/media/audio", () => ({ diff --git a/apps/web/src/agent/tools/registry.ts b/apps/web/src/agent/tools/registry.ts index ae024e77..7e4a7393 100644 --- a/apps/web/src/agent/tools/registry.ts +++ b/apps/web/src/agent/tools/registry.ts @@ -1,6 +1,20 @@ import { DefinitionRegistry } from "@/lib/registry"; -import type { ToolDefinition } from "@/agent/types"; +import type { ToolDefinition, ToolSchema } from "@/agent/types"; export const toolRegistry = new DefinitionRegistry( "tool", ); + +/** + * Exports registered tools as provider-agnostic schemas (pure data DTOs). + * Adapters convert these to their wire format; `execute` is never exposed. + */ +export function toToolSchemas( + tools: ToolDefinition[] = toolRegistry.getAll(), +): ToolSchema[] { + return tools.map(({ name, description, parameters }) => ({ + name, + description, + parameters, + })); +} diff --git a/apps/web/src/agent/types.ts b/apps/web/src/agent/types.ts index 5510a97b..9aa14835 100644 --- a/apps/web/src/agent/types.ts +++ b/apps/web/src/agent/types.ts @@ -7,9 +7,11 @@ export type ExecutionState = export interface ChatMessage { id: string; - role: "user" | "assistant" | "tool_result"; + role: "system" | "user" | "assistant" | "tool_result"; content: string; toolCalls?: ToolCall[]; + /** Associates a tool_result message with the ToolCall it answers. */ + toolCallId?: string; timestamp: number; } @@ -47,3 +49,14 @@ export interface ToolDefinition { context: AgentContext, ) => Promise; } + +/** + * Provider-agnostic tool schema — a pure data DTO without `execute`. + * Adapters convert this to their wire format; the route never passes + * executable definitions across the provider boundary. + */ +export interface ToolSchema { + name: string; + description: string; + parameters: Array<{ key: string; type: string; required: boolean }>; +} diff --git a/apps/web/src/app/api/agent/chat/__tests__/route.test.ts b/apps/web/src/app/api/agent/chat/__tests__/route.test.ts index cda65b1f..8a182d31 100644 --- a/apps/web/src/app/api/agent/chat/__tests__/route.test.ts +++ b/apps/web/src/app/api/agent/chat/__tests__/route.test.ts @@ -1,7 +1,23 @@ -import { describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, jest, test } from "bun:test"; import { POST } from "@/app/api/agent/chat/route"; import { NextRequest } from "next/server"; +// --------------------------------------------------------------------------- +// Mock the provider module — route never touches the real SDK +// --------------------------------------------------------------------------- + +const mockChat = jest.fn(); + +jest.mock("@/agent/providers", () => ({ + createProvider: () => ({ + chat: mockChat, + }), +})); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + function makeRequest(body: unknown): NextRequest { return new NextRequest("http://localhost/api/agent/chat", { method: "POST", @@ -10,26 +26,104 @@ function makeRequest(body: unknown): NextRequest { }); } +const VALID_CONTEXT = { + projectId: null, + activeSceneId: null, + mediaAssets: [] as Array<{ id: string; name: string; type: string; duration: number }>, + playbackTimeMs: 0, +}; + +const VALID_BODY = { + messages: [{ id: "msg_1", role: "user" as const, content: "Hello", timestamp: Date.now() }], + context: VALID_CONTEXT, +}; + +const savedEnv: Record = {}; + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + describe("POST /api/agent/chat", () => { - test("returns mock response for valid input", async () => { - const req = makeRequest({ - messages: [{ role: "user", content: "Hello" }], + beforeEach(() => { + mockChat.mockReset(); + + // Save and set LLM_* env vars + for (const key of ["LLM_PROVIDER", "LLM_API_KEY", "LLM_MODEL", "LLM_BASE_URL"]) { + savedEnv[key] = process.env[key]; + } + process.env.LLM_PROVIDER = "openai-compatible"; + process.env.LLM_API_KEY = "sk-test"; + process.env.LLM_MODEL = "gpt-4o-mini"; + delete process.env.LLM_BASE_URL; + }); + + afterEach(() => { + // Restore env vars + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + }); + + // ----------------------------------------------------------------------- + // 200 paths + // ----------------------------------------------------------------------- + + test("returns 200 with content for valid request", async () => { + mockChat.mockResolvedValueOnce({ + content: "Hello! How can I help?", + toolCalls: undefined, }); - const res = await POST(req); + const res = await POST(makeRequest(VALID_BODY)); expect(res.status).toBe(200); const data = await res.json(); - expect(data.content).toBe("Let me check your editor context."); - expect(data.toolCalls).toHaveLength(1); - expect(data.toolCalls[0].name).toBe("echo_context"); - expect(data.toolCalls[0].id).toBe("mock_tc_1"); + expect(data.content).toBe("Hello! How can I help?"); + expect(data.toolCalls).toBeUndefined(); }); - test("returns 400 for invalid input — missing messages", async () => { - const req = makeRequest({}); + test("returns 200 with toolCalls when provider returns them", async () => { + mockChat.mockResolvedValueOnce({ + content: "", + toolCalls: [{ id: "tc_1", name: "transcribe_video", args: {} }], + }); - const res = await POST(req); + const res = await POST(makeRequest(VALID_BODY)); + expect(res.status).toBe(200); + + const data = await res.json(); + expect(data.content).toBe(""); + expect(data.toolCalls).toHaveLength(1); + expect(data.toolCalls[0].name).toBe("transcribe_video"); + }); + + test("passes messages, systemPrompt and tools to adapter", async () => { + mockChat.mockResolvedValueOnce({ content: "ok" }); + + await POST(makeRequest(VALID_BODY)); + + const callArgs = mockChat.mock.calls[0][0] as { + messages: unknown[]; + systemPrompt: string; + tools: unknown[]; + }; + + expect(callArgs.messages).toHaveLength(1); + expect(callArgs.systemPrompt).toContain("NeuralCut"); + expect(callArgs.tools).toHaveLength(1); + }); + + // ----------------------------------------------------------------------- + // 400 paths + // ----------------------------------------------------------------------- + + test("returns 400 for missing messages", async () => { + const res = await POST(makeRequest({ context: VALID_CONTEXT })); expect(res.status).toBe(400); const data = await res.json(); @@ -37,82 +131,69 @@ describe("POST /api/agent/chat", () => { expect(data.details).toBeDefined(); }); - test("returns 400 for invalid input — bad role", async () => { - const req = makeRequest({ - messages: [{ role: "invalid_role", content: "test" }], - }); - - const res = await POST(req); + test("returns 400 for invalid role", async () => { + const res = await POST( + makeRequest({ + messages: [{ role: "invalid_role", content: "test" }], + context: VALID_CONTEXT, + }), + ); expect(res.status).toBe(400); }); - test("accepts valid input with optional context", async () => { - const req = makeRequest({ - messages: [{ role: "user", content: "Test" }], - context: { projectId: "p1" }, - }); - - const res = await POST(req); - expect(res.status).toBe(200); - }); - test("returns 400 for non-array messages", async () => { - const req = makeRequest({ - messages: "not an array", - }); - - const res = await POST(req); + const res = await POST( + makeRequest({ messages: "not an array", context: VALID_CONTEXT }), + ); expect(res.status).toBe(400); }); - test("returns transcribe_video tool call when message contains 'transcribe'", async () => { - const req = makeRequest({ - messages: [{ role: "user", content: "Please transcribe this video" }], - }); - - const res = await POST(req); - expect(res.status).toBe(200); - - const data = await res.json(); - expect(data.content).toBe("I'll transcribe your video now."); - expect(data.toolCalls).toHaveLength(1); - expect(data.toolCalls[0].name).toBe("transcribe_video"); - expect(data.toolCalls[0].id).toBe("tc_transcribe_1"); + test("returns 400 for missing context", async () => { + const res = await POST( + makeRequest({ messages: [{ role: "user", content: "Hi" }] }), + ); + expect(res.status).toBe(400); }); - test("returns transcribe_video tool call for case-insensitive 'TRANSCRIBE'", async () => { - const req = makeRequest({ - messages: [{ role: "user", content: "CAN YOU TRANSCRIBE THE AUDIO?" }], - }); + // ----------------------------------------------------------------------- + // 502 — missing env vars / provider config failures + // ----------------------------------------------------------------------- - const res = await POST(req); - expect(res.status).toBe(200); + test("returns 502 when LLM_API_KEY is missing", async () => { + delete process.env.LLM_API_KEY; + + const res = await POST(makeRequest(VALID_BODY)); + expect(res.status).toBe(502); const data = await res.json(); - expect(data.toolCalls[0].name).toBe("transcribe_video"); + expect(data.error).toContain("missing"); }); - test("returns transcribe_video when 'transcription' keyword is present", async () => { - const req = makeRequest({ - messages: [{ role: "user", content: "I need a transcription of this clip" }], - }); + test("returns 502 when LLM_PROVIDER is missing", async () => { + delete process.env.LLM_PROVIDER; - const res = await POST(req); - expect(res.status).toBe(200); - - const data = await res.json(); - expect(data.toolCalls[0].name).toBe("transcribe_video"); + const res = await POST(makeRequest(VALID_BODY)); + expect(res.status).toBe(502); }); - test("falls back to echo_context when message has no transcription intent", async () => { - const req = makeRequest({ - messages: [{ role: "user", content: "What's the weather like?" }], - }); + test("returns 502 when LLM_MODEL is missing", async () => { + delete process.env.LLM_MODEL; - const res = await POST(req); - expect(res.status).toBe(200); + const res = await POST(makeRequest(VALID_BODY)); + expect(res.status).toBe(502); + }); + + // ----------------------------------------------------------------------- + // 502 — provider error + // ----------------------------------------------------------------------- + + test("returns 502 when adapter throws", async () => { + mockChat.mockRejectedValueOnce(new Error("Network timeout")); + + const res = await POST(makeRequest(VALID_BODY)); + expect(res.status).toBe(502); const data = await res.json(); - expect(data.toolCalls[0].name).toBe("echo_context"); + expect(data.error).toBe("LLM provider error"); }); }); diff --git a/apps/web/src/app/api/agent/chat/route.ts b/apps/web/src/app/api/agent/chat/route.ts index 30ea25d3..a791eebb 100644 --- a/apps/web/src/app/api/agent/chat/route.ts +++ b/apps/web/src/app/api/agent/chat/route.ts @@ -1,16 +1,67 @@ import { type NextRequest, NextResponse } from "next/server"; import { z } from "zod"; +import type { AgentContext, ToolDefinition, ToolSchema } from "@/agent/types"; +import { buildSystemPrompt } from "@/agent/system-prompt"; +import { createProvider } from "@/agent/providers"; +import type { ProviderConfig } from "@/agent/providers"; +import { toToolSchemas } from "@/agent/tools/registry"; + +// --------------------------------------------------------------------------- +// Provider-facing tool schemas (execution is client-side) +// echo_context is intentionally excluded — only transcribe_video is exposed. +// --------------------------------------------------------------------------- +const providerToolDefs: ToolDefinition[] = [ + { + name: "transcribe_video", + description: + "Transcribes the audio of a video or audio asset using Whisper. Returns structured transcript with segments and timestamps.", + parameters: [ + { key: "assetId", type: "string", required: false }, + { key: "language", type: "string", required: false }, + { key: "modelId", type: "string", required: false }, + ], + execute: async () => ({}), // stub — never called server-side + }, +]; + +// --------------------------------------------------------------------------- +// Zod validation schemas +// --------------------------------------------------------------------------- +const toolCallSchema = z.object({ + id: z.string(), + name: z.string(), + args: z.record(z.string(), z.unknown()), +}); const chatRequestSchema = z.object({ messages: z.array( z.object({ + id: z.string(), role: z.enum(["user", "assistant", "tool_result"]), content: z.string(), + toolCalls: z.array(toolCallSchema).optional(), + toolCallId: z.string().optional(), + timestamp: z.number(), }), ), - context: z.record(z.string(), z.unknown()).optional(), + context: z.object({ + projectId: z.string().nullable(), + activeSceneId: z.string().nullable(), + mediaAssets: z.array( + z.object({ + id: z.string(), + name: z.string(), + type: z.string(), + duration: z.number(), + }), + ), + playbackTimeMs: z.number(), + }) satisfies z.ZodType, }); +// --------------------------------------------------------------------------- +// Route handler +// --------------------------------------------------------------------------- export async function POST(request: NextRequest) { const body = await request.json(); const result = chatRequestSchema.safeParse(body); @@ -22,35 +73,48 @@ export async function POST(request: NextRequest) { ); } - // Mock intent detection: keyword match on last user message - const lastUserMessage = [...result.data.messages] - .reverse() - .find((m) => m.role === "user"); - const isTranscriptionRequest = - lastUserMessage?.content.toLowerCase().includes("transcri") ?? false; + const { messages, context } = result.data; - if (isTranscriptionRequest) { - return NextResponse.json({ - content: "I'll transcribe your video now.", - toolCalls: [ - { - id: "tc_transcribe_1", - name: "transcribe_video", - args: {}, - }, - ], - }); + // Resolve provider config from LLM_* env vars + const provider = process.env.LLM_PROVIDER; + const apiKey = process.env.LLM_API_KEY; + const model = process.env.LLM_MODEL; + const baseUrl = process.env.LLM_BASE_URL; + + if (!provider || !apiKey || !model) { + return NextResponse.json( + { + error: + "Server configuration error: missing LLM_PROVIDER, LLM_API_KEY, or LLM_MODEL", + }, + { status: 502 }, + ); } - // Default: echo_context fallback - return NextResponse.json({ - content: "Let me check your editor context.", - toolCalls: [ - { - id: "mock_tc_1", - name: "echo_context", - args: {}, - }, - ], - }); + const config: ProviderConfig = { provider, apiKey, model, baseUrl }; + + // Build system prompt with tool guidance + const toolSchemas: ToolSchema[] = toToolSchemas(providerToolDefs); + const systemPrompt = buildSystemPrompt(context, providerToolDefs); + + // Delegate to provider adapter + try { + const adapter = createProvider(config); + const response = await adapter.chat({ + messages, + systemPrompt, + tools: toolSchemas, + }); + + return NextResponse.json({ + content: response.content, + ...(response.toolCalls && { toolCalls: response.toolCalls }), + }); + } catch (error) { + console.error("[agent/chat] Provider error:", error); + return NextResponse.json( + { error: "LLM provider error" }, + { status: 502 }, + ); + } } diff --git a/apps/web/src/components/editor/panels/chat/message-bubble.tsx b/apps/web/src/components/editor/panels/chat/message-bubble.tsx index 636401de..5e5d965c 100644 --- a/apps/web/src/components/editor/panels/chat/message-bubble.tsx +++ b/apps/web/src/components/editor/panels/chat/message-bubble.tsx @@ -16,6 +16,7 @@ const ROLE_LABELS: Record = { user: "You", assistant: "Assistant", tool_result: "Tool", + system: "System", }; function TranscriptCard({ data }: { data: TranscriptData }) { diff --git a/apps/web/src/types/jest-compat.d.ts b/apps/web/src/types/jest-compat.d.ts new file mode 100644 index 00000000..52c49cee --- /dev/null +++ b/apps/web/src/types/jest-compat.d.ts @@ -0,0 +1,15 @@ +/** + * Ambient augmentation for Bun's jest-compatible mocking. + * + * bun:test exports `jest` as a namespace with fn/restoreAllMocks/etc. + * but omits jest.mock() from the types even though it works at runtime. + * This module augmentation bridges the gap. + */ +declare module "bun:test" { + namespace jest { + function mock( + moduleName: string, + factory: () => Record, + ): void; + } +} diff --git a/bun.lock b/bun.lock index bce5233b..a39f2c2f 100644 --- a/bun.lock +++ b/bun.lock @@ -54,6 +54,7 @@ "nanoid": "^5.1.5", "next": "16.1.3", "next-themes": "^0.4.4", + "openai": "^5.1.0", "opencut-wasm": "^0.2.5", "pg": "^8.16.2", "postgres": "^3.4.5", @@ -1359,6 +1360,8 @@ "onnxruntime-web": ["onnxruntime-web@1.22.0-dev.20250409-89f8206ba4", "", { "dependencies": { "flatbuffers": "^25.1.24", "guid-typescript": "^1.0.9", "long": "^5.2.3", "onnxruntime-common": "1.22.0-dev.20250409-89f8206ba4", "platform": "^1.3.6", "protobufjs": "^7.2.4" } }, "sha512-0uS76OPgH0hWCPrFKlL8kYVV7ckM7t/36HfbgoFw6Nd0CZVVbQC4PkrR8mBX8LtNUFZO25IQBqV2Hx2ho3FlbQ=="], + "openai": ["openai@5.23.2", "", { "peerDependencies": { "ws": "^8.18.0", "zod": "^3.23.8" }, "optionalPeers": ["ws", "zod"], "bin": { "openai": "bin/cli" } }, "sha512-MQBzmTulj+MM5O8SKEk/gL8a7s5mktS9zUtAkU257WjvobGc9nKcBuVwjyEEcb9SI8a8Y2G/mzn3vm9n1Jlleg=="], + "opencut-wasm": ["opencut-wasm@0.2.9", "", {}, "sha512-yZilhRRgNA02XY9Bq2yt6FidbnDQS1OYsvtNczxF6jL+nvl3vRboG0owwpQY0SPIbeJoJjJBuOVy0i1Pp6JS/w=="], "p-limit": ["p-limit@6.2.0", "", { "dependencies": { "yocto-queue": "^1.1.1" } }, "sha512-kuUqqHNUqoIWp/c467RI4X6mmyuojY5jGutNU0wVTmEOOfcuwLqyMVoAi9MKi2Ak+5i9+nhmrK4ufZE8069kHA=="], diff --git a/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/archive-report.md b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/archive-report.md new file mode 100644 index 00000000..620f2630 --- /dev/null +++ b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/archive-report.md @@ -0,0 +1,74 @@ +# Archive Report: llm-agent-core-fase-1 + +**Change**: llm-agent-core-fase-1 +**Archived**: 2026-04-23 +**Verdict from verify**: PASS WITH WARNINGS +**Mode**: hybrid (engram + openspec) + +--- + +## Artifact Traceability (Engram Observation IDs) + +| Artifact | Engram Obs ID | Topic Key | +|----------|---------------|-----------| +| Proposal | #982 | sdd/llm-agent-core-fase-1/proposal | +| Spec (reconciled) | #984 | sdd/llm-agent-core-fase-1/spec | +| Design | #985 | sdd/llm-agent-core-fase-1/design | +| Tasks | #987 | sdd/llm-agent-core-fase-1/tasks | +| Apply Progress | #988 | sdd/llm-agent-core-fase-1/apply-progress | +| Verify Report | #998 | sdd/llm-agent-core-fase-1/verify-report | + +--- + +## Specs Synced to Main + +| Domain | Action | Details | +|--------|--------|---------| +| agent-session-shell | Updated | 3 requirements replaced (API Proxy Route, Orchestrator Shell, Tool Registry Shell); 4 requirements added (Provider Adapter Interface, Provider Configuration, Tool Argument Validation, Max Iteration Guard); 3 requirements preserved (Chat Store, Agent Store, Shared Foundational Types) | +| agent-context-bridge | Updated | 1 requirement replaced (Context Injection into System Prompt); 3 requirements preserved (AgentContext Contract, EditorContext Adapter, No Direct EditorCore Access) | + +--- + +## Archive Contents (OpenSpec filesystem) + +`openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/` +- proposal.md ✅ +- specs/agent-session-shell/spec.md ✅ +- specs/agent-context-bridge/spec.md ✅ +- design.md ✅ +- tasks.md ✅ +- verify-report.md ✅ +- exploration.md ✅ +- 22/22 tasks complete + +--- + +## Implementation Summary + +Replaced the mock chat route with a real LLM loop using a provider-agnostic adapter architecture. One `openai-compatible` adapter covers OpenAI, Groq, local Ollama, and any `baseUrl`-driven provider. The client-side orchestrator drives a multi-turn tool-call loop with MAX_ITERATIONS = 8, per-tool error recovery, and argument validation. `echo_context` is excluded from the real provider path. `transcribe_video` is the sole registered tool. + +**Key files changed**: `apps/web/src/agent/providers/`, `apps/web/src/app/api/agent/chat/route.ts`, `apps/web/src/agent/orchestrator.ts`, `apps/web/src/agent/tools/registry.ts`, `apps/web/src/agent/system-prompt.ts`, `.env.local`, `.env.example` + +**Test results**: 60 focused tests pass / 90 total across full change-related suite / TypeScript compiles with zero errors. + +--- + +## Important Warnings (preserved from verify) + +1. **Provider-agnostic core verified with one openai-compatible adapter path in Phase 1** — additional native adapters (e.g., Gemini) are future work. +2. **`echo_context` excluded from the real provider path** — still exists for client-side debugging but is never sent to the LLM. +3. **MAX_ITERATIONS = 8 by explicit user choice** — not configurable, hardcoded in orchestrator. +4. **Full `apps/web` suite still has one unrelated migration failure** outside this slice: `src/services/storage/migrations/__tests__/v22-to-v23.test.ts` (284 pass / 1 fail). +5. **Evidence for transcribe tool path is split across focused tests** rather than one dedicated end-to-end runtime test — coverage shape warning, not a correctness issue. + +--- + +## Source of Truth Updated + +The following main specs now reflect the new behavior: +- `openspec/specs/agent-session-shell/spec.md` +- `openspec/specs/agent-context-bridge/spec.md` + +## SDD Cycle Complete + +The change has been fully explored, proposed, specified, designed, tasked, implemented, verified, and archived. diff --git a/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/design.md b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/design.md new file mode 100644 index 00000000..310dd29d --- /dev/null +++ b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/design.md @@ -0,0 +1,118 @@ +# Design: Provider-Agnostic LLM Agent Core — Phase 1 + +## Technical Approach + +Introduce a thin provider-adapter seam at the server boundary so the route never depends on a specific LLM SDK directly. Phase 1 ships one `openai-compatible` adapter (covers OpenAI, Groq, local Ollama, any `baseUrl`-driven provider). The adapter owns message-format conversion, tool-schema conversion, and the API call. The route validates input, builds the system prompt, and delegates to the adapter. The client-side orchestrator gains a multi-turn loop with iteration cap and arg validation. `echo_context` stays out of the provider-facing tool list. `@asset` explicitly deferred. + +## Architecture Decisions + +| Decision | Choice | Rejected | Rationale | +|----------|--------|----------|-----------| +| Provider integration | `ProviderAdapter` interface + factory; route calls adapter | `openai` SDK directly in route | Proposal requires provider-agnostic core; direct SDK couples route to one vendor | +| Config naming | `LLM_PROVIDER`, `LLM_API_KEY`, `LLM_BASE_URL`, `LLM_MODEL` | `OPENAI_*` vars | Generic names allow adapter swap without env rename; backward compat aliases unnecessary | +| Adapter boundary | Adapter owns message + tool schema conversion + API call | Route does conversion, adapter only calls | Keeps conversion logic co-located with the wire format it serves | +| Tool schema location | Adapter converts `ToolDefinition[]` internally; `toOpenAIFunctions()` moves into adapter | Keep in registry | Registry should not know about any provider's wire format | +| Tool execution | Client-side (orchestrator) | Server-side | Tools need EditorCore/WASM/File — browser-only | +| Iteration cap | Hardcoded `MAX_ITERATIONS = 8` | Configurable | User-specified; 8 provides sufficient reasoning depth | +| `echo_context` | Registered but excluded from provider tool defs | Remove entirely | Still useful for client-side debugging; just not LLM-visible | +| `@asset` deferral | Out of scope | Include | Requires context parser + autocomplete UI + asset resolution — prerequisites absent | + +## Data Flow + +``` +Client (orchestrator.ts) Server (route.ts) Adapter +───────────────────── ───────────────── ───────── +1. POST {messages, context} ───→ 2. Validate (zod) + 3. buildSystemPrompt(ctx, tools) + 4. createProvider(config) + 5. provider.chat() ────────→ 6. Convert msgs → wire fmt + 7. Convert ToolDef[] → wire fmt + 8. Call API ──→ Provider + 9. Convert resp ←────── back + ←──── 10. Return {content, toolCalls} +11. toolCalls? + ├─ YES: validate args, resolve via registry, append results → go to 1 (loop) + └─ NO: append assistant message → done +MAX_ITERATIONS exceeded → limit message, idle +``` + +## File Changes + +| File | Action | Description | +|------|--------|-------------| +| `apps/web/src/agent/providers/types.ts` | **Create** | `ProviderAdapter` interface, `ProviderConfig`, canonical response types | +| `apps/web/src/agent/providers/openai-compatible.ts` | **Create** | OpenAI-compatible adapter implementing `ProviderAdapter` | +| `apps/web/src/agent/providers/index.ts` | **Create** | `createProvider(config)` factory | +| `apps/web/src/app/api/agent/chat/route.ts` | **Modify** | Replace direct `openai` import with `createProvider(config)`; thin validate → build prompt → delegate | +| `apps/web/src/agent/orchestrator.ts` | **Modify** | Multi-turn `while` loop with arg validation + iteration cap | +| `apps/web/src/agent/tools/registry.ts` | **Modify** | Remove `OpenAIFunctionTool` type and `toOpenAIFunctions()` (moved to adapter) | +| `apps/web/.env.local` | **Modify** | Rename `OPENAI_*` → `LLM_*` vars; add `LLM_PROVIDER` | +| `apps/web/.env.example` | **Modify** | Same rename | + +## Interfaces / Contracts + +### ProviderAdapter + +```ts +// providers/types.ts +interface ProviderConfig { + provider: string; // "openai-compatible" (Phase 1 only value) + apiKey: string; + model: string; + baseUrl?: string; +} + +interface ProviderResponse { + content: string; + toolCalls?: ToolCall[]; +} + +interface ProviderAdapter { + chat(params: { + messages: ChatMessage[]; + systemPrompt: string; + tools: ToolDefinition[]; + }): Promise; +} +``` + +### Factory + +```ts +// providers/index.ts +export function createProvider(config: ProviderConfig): ProviderAdapter { + switch (config.provider) { + case "openai-compatible": + return new OpenAICompatibleAdapter(config); + default: + throw new Error(`Unknown LLM provider: ${config.provider}`); + } +} +``` + +### Environment Variables + +```env +LLM_PROVIDER=openai-compatible # Required +LLM_API_KEY=sk-... # Required +LLM_MODEL=gpt-4o-mini # Required +LLM_BASE_URL= # Optional — for compatible providers +``` + +## Testing Strategy + +| Layer | What | Approach | +|-------|------|----------| +| Unit | `OpenAICompatibleAdapter.chat()` with mocked `openai` SDK | Verify message/tool conversion + response mapping | +| Unit | `createProvider()` factory | Valid config returns adapter; unknown provider throws | +| Unit | Route with mocked `createProvider` | Test 200/400/502 paths | +| Unit | Orchestrator multi-turn loop | Iteration cap, arg validation, tool re-submission, 0/1/2 tool rounds | +| Unit | System prompt tool guidance | Assert tool names appear in output | + +## Migration / Rollout + +No data migration. Env vars rename from `OPENAI_*` to `LLM_*` is the only config change. Existing `toOpenAIFunctions()` callers in tests must import from the adapter instead of registry. Rollback: revert route to direct `openai` call, restore `OPENAI_*` vars. + +## Open Questions + +- None — `echo_context` decision settled: registered but excluded from provider-facing defs via the explicit `providerToolDefs` array passed to the adapter. diff --git a/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/exploration.md b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/exploration.md new file mode 100644 index 00000000..83c63264 --- /dev/null +++ b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/exploration.md @@ -0,0 +1,211 @@ +# Exploration: LLM Agent Core — Phase 1 + +## Current State + +### Agent Infrastructure (shipped in `infra-habilitadora-fase-1` + `transcribe-video-fase-1`) + +The agent pipeline is wired end-to-end but driven entirely by a **mock API route** that does keyword matching: + +``` +ChatPanel → handleSend → orchestrator.run(messages, context) → POST /api/agent/chat → mock route + ↓ +orchestrator receives { content, toolCalls[] } → resolves toolCalls via toolRegistry → appends to chatStore +``` + +**Key files and their current roles:** + +| File | Current role | +|------|-------------| +| `agent/types.ts` | `ChatMessage`, `ToolCall`, `ToolResult`, `AgentContext`, `ToolDefinition` — all typed | +| `agent/orchestrator.ts` | Client-side single-pass: POST to API route → resolve tool calls → append messages | +| `agent/tools/registry.ts` | `DefinitionRegistry` — `register()`, `get()`, `getAll()` | +| `agent/tools/mock.tool.ts` | `echo_context` — returns context summary, for debugging | +| `agent/tools/transcribe-video.tool.ts` | `transcribe_video` — full Whisper pipeline via EditorContextAdapter | +| `agent/system-prompt.ts` | Static builder: editor name, project, scene, assets, playback position | +| `agent/context.ts` | EditorContextAdapter — only file that touches EditorCore | +| `agent/context-mapper.ts` | Pure data mapping (WASM-free testability) | +| `app/api/agent/chat/route.ts` | **MOCK** — keyword match ("transcri") → hardcoded tool calls | +| `stores/chat-store.ts` | Zustand: messages, loading, error, addMessage, sendMessage | +| `stores/agent-store.ts` | Zustand: status, activeTool, context | +| `components/.../chat/index.tsx` | ChatPanel UI — sends via orchestratorRun | + +### What Must Change + +The mock `route.ts` is the **entire bottleneck**. It: +1. Returns hardcoded responses based on keyword matching +2. Has NO understanding of conversation history +3. Has NO real LLM integration +4. Always returns a `toolCalls` array (never free-form chat) + +Everything else (types, orchestrator, registry, context adapter, stores, UI) is well-structured and **reusable with minimal changes**. + +## Affected Areas + +- `apps/web/src/app/api/agent/chat/route.ts` — **must be rewritten** to call a real LLM provider +- `apps/web/src/agent/types.ts` — `ToolDefinition.parameters` uses a flat `{ key, type, required }[]` format; needs to map to provider-native JSON Schema for tool calling +- `apps/web/src/agent/orchestrator.ts` — currently single-pass; needs loop support for multi-turn tool calling (LLM may call tools, get results, then call more tools) +- `apps/web/src/agent/system-prompt.ts` — needs to be sent to the provider as a system message (already built, just needs to be consumed server-side) +- `apps/web/src/agent/tools/registry.ts` — `getAll()` exists and returns all `ToolDefinition`s; needs a method to export them as provider-native tool schemas +- `apps/web/src/agent/tools/mock.tool.ts` — `echo_context` becomes unnecessary once the LLM can answer context questions naturally; remove or keep for debugging +- `apps/web/.env.local` — needs `LLM_API_KEY` and potentially `LLM_BASE_URL` +- `apps/web/package.json` — needs the SDK dependency for the chosen provider + +## Approaches + +### 1. OpenAI SDK Direct (single provider, OpenAI-compatible) + +Use the official `openai` npm package. Works with OpenAI, and any OpenAI-compatible API (Ollama, Together, Groq, etc.) by changing `baseURL`. + +- **Pros**: Industry-standard SDK, native tool calling support (`tools` param with JSON Schema), streaming support, battle-tested, one dependency +- **Cons**: Couples to OpenAI's tool-calling format initially; if you later need a non-OpenAI-compatible provider, you'd need an adapter +- **Effort**: Low — route.ts rewrite + tool schema mapper + env var + +### 2. Vercel AI SDK (`ai` package) + +Use Vercel's AI SDK which provides a unified interface over multiple providers (OpenAI, Anthropic, Google, etc.) with `generateText()` / `streamText()`. + +- **Pros**: Provider-agnostic from day one, streaming built-in, tool calling abstraction, integrates with Next.js route handlers natively +- **Cons**: Extra abstraction layer, another dependency to version-track, may over-abstract what is currently a simple call, learning curve for its conventions +- **Effort**: Low-Medium — new dependency + learn SDK patterns + route rewrite + +### 3. Custom thin wrapper over fetch + +Build a minimal `LLMProvider` interface and implement one provider with raw `fetch`. + +- **Pros**: Zero new dependencies, full control, exactly what you need +- **Cons**: Must implement streaming parsing, retry logic, error handling, tool calling serialization yourself; NIH risk +- **Effort**: Medium — significant boilerplate for production-quality + +## Recommendation + +**Approach 1: OpenAI SDK Direct** — and here's why: + +1. **YAGNI**: The user wants ONE provider working first. Don't build abstraction for multiple providers until you actually need the second one. +2. **Tool calling is native**: OpenAI's `tools` parameter maps directly to JSON Schema, which maps cleanly from the existing `ToolDefinition` type. The SDK handles function calling, parallel tool calls, and the full request/response cycle. +3. **OpenAI-compatible = flexible enough**: By setting `baseURL`, you get Ollama (local), Together, Groq, Mistral — all without code changes. This covers the realistic provider landscape without an abstraction layer. +4. **Route handler is the seam**: The API route (`/api/agent/chat`) is already the boundary between client and "LLM". Changing providers later means changing ONE file. That's the correct abstraction level. + +**If the user explicitly needs Anthropic/Claude later**, that's the moment to consider the Vercel AI SDK or a thin adapter — not before. + +## Architecture: How the LLM Loop Works + +``` +Client (orchestrator.ts) Server (route.ts) +───────────────────────── ───────────────────── +POST { messages[], context } + 1. Build system prompt from context + 2. Map toolRegistry.getAll() → OpenAI tools[] + 3. Call openai.chat.completions.create({ + model, messages, tools + }) + 4. If response has tool_calls: + → Return { content, toolCalls } to client + 5. If no tool_calls: + → Return { content } (free-form chat) + +Receives { content, toolCalls? } + ├─ No tool calls → display content + └─ Has tool calls → resolveToolCalls() on client + → Append tool_result messages + → POST again with updated history (loop) +``` + +The key insight: **the orchestrator already handles tool resolution on the client** (tools like `transcribe_video` need `EditorContextAdapter` which only exists client-side). The server just decides WHAT tools to call; the client EXECUTES them and feeds results back. + +This means we need the orchestrator to **loop**: send → receive → execute tools → send results back → receive final response. + +### System Prompt Strategy + +The current `buildSystemPrompt()` is a good start but needs enhancement for the LLM to be genuinely useful: + +```typescript +// What the system prompt should contain: +1. Identity: "You are an AI assistant embedded in the NeuralCut video editor." +2. Editor context: project, scene, assets (already exists) +3. Available tools: list of tool names + descriptions (from registry) +4. Behavioral guidance: when to use tools vs. answer directly +5. Asset awareness: the model should know asset IDs so it can reference them in tool calls +``` + +### Tool Schema Mapping + +Current `ToolDefinition.parameters`: +```typescript +parameters: Array<{ key: string; type: string; required: boolean }> +``` + +OpenAI expects JSON Schema in `tools[].function.parameters`: +```json +{ + "type": "object", + "properties": { "assetId": { "type": "string", "description": "..." } }, + "required": ["assetId"] +} +``` + +A thin mapper function converts between these formats. This lives server-side in the route handler. + +### `transcribe_video` as First Real Tool + +Already implemented and working. To expose it to the LLM: +1. The server reads `toolRegistry.getAll()` and maps to OpenAI tool format +2. The LLM sees `transcribe_video` with its description and parameters +3. When the LLM decides to call it, the orchestrator resolves it client-side as it already does +4. **No changes needed to the tool itself** — it already works via the registry + +### `@asset` References + +**Defer to Phase 2.** Prerequisites: +1. A working LLM chat loop (this phase) +2. The LLM already knowing about assets via system prompt +3. A UI-side parser that detects `@` triggers and offers autocomplete +4. The parser mapping `@asset_name` → `assetId` before sending to the API + +This is a UX feature layered on top of the LLM core, not part of the core itself. + +## Minimum Viable Scope (Thin Vertical Slice) + +**Goal**: "Real LLM chat works + one tool works end-to-end" + +### In Scope +1. Add `openai` SDK dependency +2. Add `LLM_API_KEY` + `LLM_BASE_URL` + `LLM_MODEL` env vars +3. Rewrite `route.ts` to call OpenAI chat completions with: + - System prompt from `buildSystemPrompt(context)` + - Tool schemas from `toolRegistry.getAll()` mapped to OpenAI format + - Conversation history from `messages[]` +4. Add tool schema mapper utility (server-side) +5. Update `orchestrator.ts` to support **multi-turn tool calling loop**: + - Send messages → receive response + - If tool calls: execute, append results, POST again + - Repeat until no tool calls (LLM gives final answer) +6. Wire `transcribe_video` tool (already works, just needs to be visible to LLM) +7. Remove or deprecate keyword-based mock logic + +### Out of Scope (deferred) +- `@asset` reference syntax (Phase 2) +- Streaming responses (can add later) +- Multiple provider support (YAGNI) +- Conversation persistence (in-memory only for now) +- Tool result rendering in UI (just show JSON for now) +- Token counting / rate limiting + +## Risks + +1. **LLM may hallucinate tool calls**: The model might call tools with wrong arguments. Mitigation: validate args server-side before returning to client. +2. **Tool execution is client-only**: `transcribe_video` needs `EditorContextAdapter` which requires `EditorCore`. This already works but means tools can't run server-side. This is correct for now (WASM is client-side) but limits future server-side tools. +3. **Orchestrator loop could infinite-loop**: If the LLM keeps requesting tool calls. Mitigation: max iterations constant (e.g., 5). +4. **API key exposure**: Must use server-only env vars (no `NEXT_PUBLIC_`). The route handler is server-side, so this is safe by default. +5. **Latency**: LLM calls add network latency. The current mock is instant. Users will notice. Mitigation: streaming (deferred) or a loading state that's already in place. +6. **Cost**: Every chat message hits the LLM API. No local caching yet. Start with a cheap model (`gpt-4o-mini` or local Ollama). + +## Ready for Proposal + +**Yes.** The exploration is complete. The scope is clear, the approach is decided (OpenAI SDK direct), and the thin vertical slice is well-defined. + +The orchestrator should tell the user: +- We recommend OpenAI SDK as the thinnest viable integration +- The API route is the ONLY file that fundamentally changes +- The orchestrator needs a loop for multi-turn tool calling +- `transcribe_video` needs zero changes — it just becomes visible to the LLM +- `@asset` should be deferred to Phase 2 diff --git a/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/proposal.md b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/proposal.md new file mode 100644 index 00000000..6f8a24cc --- /dev/null +++ b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/proposal.md @@ -0,0 +1,62 @@ +# Proposal: Provider-Agnostic LLM Core — Phase 1 + +## Intent + +Replace the mock chat route with a real LLM loop while correcting the architecture: provider choice MUST be config-driven, not hardcoded to OpenAI, so NeuralCut can target local Ollama, OpenAI, Groq, Gemini-class future adapters, and other compatible providers from one core contract. + +## Scope + +### In Scope +- Define a common internal provider interface plus server-only config for `provider`, `model`, `apiKey`, and `baseUrl`. +- Replace the mock route with a real provider-backed conversational loop and iterative client-side tool execution. +- Keep `transcribe_video` as the first real tool and inject context/tool guidance through the system prompt. + +### Out of Scope +- `@asset` references, autocomplete, asset parsing, streaming, persistence, token accounting, and richer tool rendering. +- Multiple native transport adapters in this phase; one initial OpenAI-compatible adapter path is enough if the core contract stays provider-agnostic. + +## Capabilities + +### New Capabilities +- None. + +### Modified Capabilities +- `agent-session-shell`: replace canned route behavior with config-selected provider execution and iterative tool-call orchestration. +- `agent-context-bridge`: strengthen system-prompt requirements so editor context is injected as server-consumed provider guidance. + +## Approach + +Add a thin provider port at the server boundary, selected by config. Phase 1 may ship a single OpenAI-compatible adapter covering OpenAI, Groq, local Ollama, and similar `baseUrl`-driven providers, while leaving room for a separate Gemini adapter later. Keep tool execution client-side because tools depend on editor/WASM context. The orchestrator loops until the model stops requesting tools or a max-iteration guard is hit. + +## Affected Areas + +| Area | Impact | Description | +|------|--------|-------------| +| `apps/web/src/app/api/agent/chat/route.ts` | Modified | Route validates config-backed requests and calls selected provider adapter | +| `apps/web/src/agent/orchestrator.ts` | Modified | Multi-turn tool-call loop with guardrails | +| `apps/web/src/agent/system-prompt.ts` | Modified | Provider-ready context and tool guidance | +| `apps/web/src/agent/tools/registry.ts` | Modified | Provider-agnostic tool schema export | +| `apps/web/src/agent/` | Modified | Internal provider interface/adapter seam | +| `apps/web/.env.local` | Modified | Server-only provider/model/apiKey/baseUrl config | + +## Risks + +| Risk | Likelihood | Mitigation | +|------|------------|------------| +| OpenAI-compatible seam misses provider quirks | Med | Keep adapter boundary explicit for provider-specific follow-up | +| Invalid tool args or loops | Med | Validate args and cap iterations | +| App-layer provider logic grows sticky before rust migration | Med | Keep the contract narrow and portable | + +## Rollback Plan + +Restore the mock route, remove provider-config usage, and keep the provider interface isolated so the rollback does not affect tool or UI wiring. + +## Dependencies + +- Server-side provider credentials/config for at least one supported adapter path. + +## Success Criteria + +- [ ] A natural user message receives a real LLM response using config-selected provider settings. +- [ ] `transcribe_video` can be requested, executed client-side, and folded into a final assistant reply. +- [ ] The shipped architecture exposes a provider-agnostic core even if Phase 1 only implements one adapter path. diff --git a/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/specs/agent-context-bridge/spec.md b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/specs/agent-context-bridge/spec.md new file mode 100644 index 00000000..84f3d25a --- /dev/null +++ b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/specs/agent-context-bridge/spec.md @@ -0,0 +1,27 @@ +# Delta for Agent Context Bridge + +## MODIFIED Requirements + +### Requirement: Context Injection into System Prompt + +The system SHALL provide a `buildSystemPrompt(context: AgentContext)` function that produces a system prompt string including the media summary AND guidance about available tools. When `mediaAssets` is empty, the prompt MUST still be valid but indicate no media loaded. The prompt MUST instruct the LLM that it can call tools from a known set (currently `transcribe_video`) and that it should respond in plain text when no tool is needed. The system prompt template MUST be separate from the orchestrator logic and MUST be consumable by the server route (i.e., a pure function with no client-only dependencies). + +(Previously: System prompt included only editor/project context and media summary, no tool guidance, no server-consumption constraint.) + +#### Scenario: Prompt includes media context and tool guidance + +- GIVEN `AgentContext` with `mediaAssets: [{ id: "v1", name: "clip.mp4", type: "video", duration: 30 }]` +- WHEN `buildSystemPrompt(context)` is called +- THEN the returned string contains "clip.mp4", "video", and references "transcribe_video" as an available tool + +#### Scenario: Prompt without media + +- GIVEN `AgentContext` with `mediaAssets: []` +- WHEN `buildSystemPrompt(context)` is called +- THEN the returned string is valid, contains "No media assets loaded", and still references available tools + +#### Scenario: Server-side consumption + +- GIVEN `buildSystemPrompt` is imported in the API route +- WHEN the route calls `buildSystemPrompt(context)` +- THEN it returns a string without importing any client-only modules (no `EditorCore`, no WASM, no browser APIs) diff --git a/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/specs/agent-session-shell/spec.md b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/specs/agent-session-shell/spec.md new file mode 100644 index 00000000..4921d5a6 --- /dev/null +++ b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/specs/agent-session-shell/spec.md @@ -0,0 +1,129 @@ +# Delta for Agent Session Shell + +## ADDED Requirements + +### Requirement: Provider Adapter Interface + +The system SHALL define a server-side provider adapter interface with a single method `chat(params: ProviderChatParams): Promise`. `ProviderChatParams` MUST include `messages`, `systemPrompt`, and `toolSchemas` in a provider-agnostic internal format. `ProviderChatResponse` MUST include `content: string` and `toolCalls?: ProviderToolCall[]` where `ProviderToolCall { id, name, args }`. Each concrete adapter converts internal tool schemas to its own wire format. Phase 1 MAY ship exactly one adapter (OpenAI-compatible) as long as the interface contract remains provider-agnostic. + +#### Scenario: Adapter translates tool schemas to wire format + +- GIVEN `transcribe_video` has internal parameters `[{ key: "assetId", type: "string", required: false }]` +- WHEN the adapter receives tool schemas before calling the provider +- THEN it converts them to its provider-specific wire format + +#### Scenario: Adapter selected by config + +- GIVEN `LLM_PROVIDER` is set to `"openai-compatible"` +- WHEN the route resolves the adapter +- THEN the OpenAI-compatible adapter is returned + +### Requirement: Provider Configuration + +Server-only environment variables `LLM_PROVIDER` (required), `LLM_MODEL` (required), `LLM_API_KEY` (required), and `LLM_BASE_URL` (optional) MUST determine which adapter and model the route uses. Configuration MUST be read server-side only — never exposed to the client. + +#### Scenario: Missing required configuration + +- GIVEN `LLM_API_KEY` is not set in the environment +- WHEN the route attempts to initialize the provider +- THEN the call returns 502 with a missing-configuration error + +### Requirement: Tool Argument Validation + +Before executing any tool call, the orchestrator MUST validate that each argument key declared `required: true` in the tool's `parameters` is present in the call args and is of the declared type. If validation fails, the tool MUST NOT execute; instead the orchestrator MUST produce a `ToolResult` with `error` describing the missing or malformed argument. + +#### Scenario: Missing required argument + +- GIVEN `transcribe_video` declares no required parameters and the LLM returns `toolCalls: [{ name: "transcribe_video", args: {} }]` +- WHEN the orchestrator validates args +- THEN validation passes and the tool executes + +#### Scenario: Wrong argument type + +- GIVEN a tool declares `{ key: "assetId", type: "string", required: true }` +- AND the LLM returns `args: { assetId: 123 }` +- WHEN the orchestrator validates args +- THEN the tool does NOT execute and a `ToolResult` with `error` containing "assetId" is returned + +### Requirement: Max Iteration Guard + +The orchestrator loop MUST enforce a hard iteration cap of **8**. An iteration is one round of "call provider → resolve tool calls → feed results back." If the cap is reached, the orchestrator MUST stop, append a final assistant message indicating the limit was reached, and set `agentStore.status` to `"idle"`. + +#### Scenario: Cap reached mid-loop + +- GIVEN the provider has returned tool calls on 7 consecutive iterations +- WHEN iteration 8 also returns tool calls +- THEN the orchestrator stops, appends "Reached maximum iteration limit" to `chatStore`, and sets status to `"idle"` + +#### Scenario: Loop completes within cap + +- GIVEN the provider returns a final text response after 2 tool-call iterations +- WHEN the orchestrator processes the response +- THEN no cap message is appended and status is `"idle"` + +## MODIFIED Requirements + +### Requirement: API Proxy Route + +The system SHALL expose `POST /api/agent/chat` that accepts `{ messages: ChatMessage[], context?: AgentContext }` and returns `{ content: string, toolCalls?: ToolCall[] }`. The route MUST resolve the provider adapter from configuration, assemble a system prompt via `buildSystemPrompt(context)`, and pass internal tool schemas from the registry to the adapter. The adapter handles all provider-specific wire-format conversion. The request body MUST be validated via zod; invalid input returns 400. Provider errors return 502. + +(Previously: Route used `openai` SDK directly with `OPENAI_API_KEY`/`OPENAI_BASE_URL`/`OPENAI_MODEL` env vars and `toolRegistryToOpenAITools()`.) + +#### Scenario: Valid request returns provider response + +- GIVEN provider config is set and a POST with valid `{ messages, context }` arrives +- WHEN the handler calls through the provider adapter +- THEN it returns 200 with `{ content: "...", toolCalls?: [...] }` + +#### Scenario: Invalid request returns 400 + +- GIVEN a POST request with `{}` (missing `messages`) +- WHEN the handler processes the request +- THEN it returns 400 with an error description + +#### Scenario: Provider error returns 502 + +- GIVEN the provider adapter call throws a network or auth error +- WHEN the handler catches it +- THEN it returns 502 with `{ error: "LLM provider error" }` + +### Requirement: Orchestrator Shell + +The system SHALL provide a client-side `orchestrator.run(messages, context)` function that iterates: (1) sets `agentStore.status` to `"sending"`, (2) POSTs `{ messages, context }` to `/api/agent/chat`, (3) if the response contains `toolCalls`, validates args, resolves each via the tool registry (if a single tool throws, the error MUST be captured in a `ToolResult` and the loop MUST continue so the LLM can recover), appends tool results to the message history, and loops back to step 2 with the updated history, (4) when the response has no `toolCalls`, appends the final assistant message to `chatStore`, sets status to `"idle"`. The loop MUST NOT exceed the max iteration guard. On API or network errors (not per-tool errors), the orchestrator MUST set `chatStore.error` and `agentStore.status` to `"error"`. + +(Previously: Single-pass orchestrator — one API call, no loop, no iteration cap.) + +#### Scenario: Happy path with no tool calls + +- GIVEN `chatStore` has one user message and the API returns `{ content: "Hello!", toolCalls: [] }` +- WHEN `orchestrator.run()` is invoked +- THEN `chatStore.messages` contains the assistant reply "Hello!" +- AND `agentStore.status` is `"idle"` + +#### Scenario: Tool call then final answer + +- GIVEN iteration 1 returns `{ content: "", toolCalls: [{ name: "transcribe_video", args: {} }] }` +- AND the tool executes successfully +- AND iteration 2 returns `{ content: "Here is the transcription...", toolCalls: [] }` +- WHEN the orchestrator loop completes +- THEN `chatStore.messages` contains the tool result AND the final assistant message +- AND `agentStore.status` is `"idle"` + +#### Scenario: Error during tool execution — per-tool recovery + +- GIVEN iteration 1 returns `{ content: "", toolCalls: [{ name: "transcribe_video", args: {} }] }` and the tool throws +- WHEN the orchestrator catches the error inside `resolveToolCalls` +- THEN a `ToolResult` with the error description is appended to the message history +- AND the loop continues, sending tool-result back to the LLM for a final answer + +### Requirement: Tool Registry Shell + +The system SHALL provide a `ToolRegistry` extending `DefinitionRegistry` where `ToolDefinition` contains `{ name, description, parameters, execute }`. The registry MUST support `register`, `get`, `has`, `getAll`, and a `toToolSchemas()` method that exports every registered tool's parameters in a provider-agnostic internal schema format. The provider adapter is responsible for converting these internal schemas to its specific wire format. In this phase `echo_context` MUST NOT be registered; exactly one tool — `transcribe_video` — MUST be registered. + +(Previously: Registry exported `toOpenAISchemas()` producing OpenAI function-calling format directly; `echo_context` was the only registered tool.) + +#### Scenario: Schema export produces internal format + +- GIVEN `transcribe_video` is registered with parameters `[{ key: "assetId", type: "string", required: false }, ...]` +- WHEN `toToolSchemas()` is called +- THEN it returns an array with one entry containing `{ name: "transcribe_video", description, parameters: [...] }` in the internal format diff --git a/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/tasks.md b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/tasks.md new file mode 100644 index 00000000..1047cbdb --- /dev/null +++ b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/tasks.md @@ -0,0 +1,47 @@ +# Tasks: Provider-Agnostic LLM Agent Core — Phase 1 + +## Phase 1: Provider Adapter Extraction + +- [x] 1.1 Create `apps/web/src/agent/providers/types.ts` — `ProviderConfig { provider, apiKey, model, baseUrl? }`, `ProviderResponse { content, toolCalls? }`, `ProviderAdapter.chat()` interface +- [x] 1.2 Create `apps/web/src/agent/providers/openai-compatible.ts` — move `toOpenAIMessages()` from `route.ts` and `toOpenAIFunctions()` from `registry.ts` into adapter; implement `ProviderAdapter.chat()` using `openai` SDK +- [x] 1.3 Create `apps/web/src/agent/providers/index.ts` — `createProvider(config)` factory: switch on `config.provider`, return `OpenAICompatibleAdapter` for `"openai-compatible"`, throw for unknown +- [x] 1.4 Clean `apps/web/src/agent/tools/registry.ts` — remove `OpenAIFunctionTool` type and `toOpenAIFunctions()` export (now adapter-owned) + +## Phase 2: Route + Config Migration + +- [x] 2.1 Rename `OPENAI_*` → `LLM_PROVIDER`, `LLM_API_KEY`, `LLM_MODEL`, `LLM_BASE_URL` in `apps/web/.env.local` and `.env.example` +- [x] 2.2 Rewrite `apps/web/src/app/api/agent/chat/route.ts` — remove direct `openai` import; read `LLM_*` env vars; call `createProvider(config)`; delegate to `provider.chat({ messages, systemPrompt, tools })`; keep zod validation and 400/502 error paths + +## Phase 3: Client Multi-Turn Orchestrator Loop + +- [x] 3.1 Refactor `run()` in `apps/web/src/agent/orchestrator.ts` into `while (iterations < MAX_ITERATIONS=8)`: POST → if `toolCalls`, validate args + resolve via registry + append tool_result to history → loop; if no toolCalls, append final message + break +- [x] 3.2 Add arg validation in orchestrator: check each `required: true` param exists and matches declared type; on failure return `ToolResult` with error (tool does NOT execute) +- [x] 3.3 Max-iteration guard: stop loop, append limit message to chatStore, set idle + +## Phase 4: Tests + +- [x] 4.1 Unit test `createProvider()` in `apps/web/src/agent/providers/__tests__/index.test.ts` — valid config returns adapter; unknown provider throws +- [x] 4.2 Unit test `OpenAICompatibleAdapter.chat()` in `apps/web/src/agent/providers/__tests__/openai-compatible.test.ts` — mock `openai` SDK; verify message/tool conversion and response mapping +- [x] 4.3 Rewrite `apps/web/src/app/api/agent/chat/__tests__/route.test.ts` — mock `createProvider`; test 200/400/502 paths; remove keyword-matching assertions +- [x] 4.4 Rewrite `apps/web/src/agent/__tests__/orchestrator.test.ts` — no-tool round, one tool round + loop, max-iteration guard, arg validation failure; remove `echo_context` references +- [x] 4.5 Update `apps/web/src/agent/__tests__/system-prompt.test.ts` — verify prompt includes tool guidance section with tool names listed + +## Phase 5: Post-Verify Fix Batch + +- [x] 5.1 Fix route missing-config status: 500 → 502 to match spec requirement +- [x] 5.2 Add `ToolSchema` type and `toToolSchemas()` to registry — provider-agnostic schema export without `execute` +- [x] 5.3 Fix TypeScript failures: zod v4 `z.record()` arity, missing `id`/`timestamp` in zod schema, OpenAI union type narrowing for tool_calls, jest.mock type augmentation +- [x] 5.4 Fix pre-existing `message-bubble.tsx` TS error (missing `system` role label) + +## Phase 7: Spec-Wording Alignment + +- [x] 7.1 Align provider-error payload wording with spec: route 502 catch block returns `{ error: "LLM provider error" }` (was `"Failed to get response from AI provider"`). Updated test assertion accordingly. + +## Completed (Prior Work) + +- [x] P0.1 Add `openai` to `apps/web/package.json` and install +- [x] P0.2 Add `OPENAI_*` env vars to `.env.local` / `.env.example` +- [x] P0.3 Extend `ChatMessage.role` to accept `"system"` + `"tool_result"`, add `toolCallId` field +- [x] P0.4 Extend `buildSystemPrompt()` with tool guidance section +- [x] P0.5 Route rewritten with zod validation + OpenAI SDK + tool schema export +- [x] P0.6 Remove `echo_context` import from orchestrator diff --git a/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/verify-report.md b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/verify-report.md new file mode 100644 index 00000000..0fbf4de5 --- /dev/null +++ b/openspec/changes/archive/2026-04-23-llm-agent-core-fase-1/verify-report.md @@ -0,0 +1,127 @@ +## Verification Report + +**Change**: llm-agent-core-fase-1 +**Version**: N/A +**Mode**: Standard (strict TDD explicitly disabled for this verify run) + +--- + +### Completeness +| Metric | Value | +|--------|-------| +| Tasks total | 22 | +| Tasks complete | 22 | +| Tasks incomplete | 0 | + +All checklist items in `openspec/changes/llm-agent-core-fase-1/tasks.md` are marked complete (`16` phase tasks + `6` prior-completed items). + +--- + +### Build & Tests Execution + +**Build / Type Check**: ✅ Passed (type check) / ➖ Build skipped +```text +Type check command: bunx tsc --noEmit +Result: passed with no output + +Build command: skipped +Reason: repository instruction in AGENTS.md says "Never build after changes." +``` + +**Tests**: ✅ Focused slice passed / ⚠️ unrelated full-suite failure remains +```text +Focused verify command: +bun test "src/agent/__tests__/orchestrator.test.ts" "src/agent/providers/__tests__/index.test.ts" "src/agent/providers/__tests__/openai-compatible.test.ts" "src/agent/__tests__/system-prompt.test.ts" "src/agent/tools/__tests__/registry.test.ts" "src/agent/tools/__tests__/transcribe-video.test.ts" "src/agent/__tests__/import-boundary.test.ts" "src/app/api/agent/chat/__tests__/route.test.ts" + +Focused result: 60 passed / 0 failed / 0 skipped + +Full suite command: +bun test + +Full suite result: 284 passed / 1 failed / 0 skipped +Failing test (treated as out-of-scope unless proven introduced by this change): +- src/services/storage/migrations/__tests__/v22-to-v23.test.ts + V22 to V23 Migration > converts project time values from seconds to ticks and fps to a frame-rate object + Expected: 1860000 + Received: 1550 +``` + +**Coverage**: ➖ Not available + +--- + +### Spec Compliance Matrix + +| Requirement | Scenario | Test | Result | +|-------------|----------|------|--------| +| Provider Adapter Interface | Adapter translates tool schemas to wire format | `src/agent/providers/__tests__/openai-compatible.test.ts > converts tool definitions to OpenAI function format` | ✅ COMPLIANT | +| Provider Adapter Interface | Adapter selected by config | `src/agent/providers/__tests__/index.test.ts > returns OpenAICompatibleAdapter for openai-compatible provider` | ✅ COMPLIANT | +| Provider Configuration | Missing required configuration | `src/app/api/agent/chat/__tests__/route.test.ts > returns 502 when LLM_API_KEY is missing` | ✅ COMPLIANT | +| Tool Argument Validation | Missing required argument / optional-arg transcribe path | `src/agent/tools/__tests__/transcribe-video.test.ts > transcribes a single video asset successfully` | ⚠️ PARTIAL | +| Tool Argument Validation | Wrong argument type | `src/agent/__tests__/orchestrator.test.ts > returns error result when arg has wrong type` | ✅ COMPLIANT | +| Max Iteration Guard | Cap reached → limit message, idle | `src/agent/__tests__/orchestrator.test.ts > stops at MAX_ITERATIONS and appends limit message` | ✅ COMPLIANT | +| Max Iteration Guard | Completes within cap | `src/agent/__tests__/orchestrator.test.ts > resolves tool call and loops for final answer` | ✅ COMPLIANT | +| API Proxy Route | Valid request returns provider response | `src/app/api/agent/chat/__tests__/route.test.ts > returns 200 with content for valid request` | ✅ COMPLIANT | +| API Proxy Route | Invalid request returns 400 | `src/app/api/agent/chat/__tests__/route.test.ts > returns 400 for missing messages` | ✅ COMPLIANT | +| API Proxy Route | Provider error returns 502 | `src/app/api/agent/chat/__tests__/route.test.ts > returns 502 when adapter throws` | ✅ COMPLIANT | +| Orchestrator Shell | Happy path with no tool calls | `src/agent/__tests__/orchestrator.test.ts > appends assistant message when response has no tool calls` | ✅ COMPLIANT | +| Orchestrator Shell | Tool call then final answer | `src/agent/__tests__/orchestrator.test.ts > resolves tool call and loops for final answer` | ⚠️ PARTIAL | +| Orchestrator Shell | Error during tool execution — per-tool recovery | `src/agent/__tests__/orchestrator.test.ts > recovers when tool.execute() throws, continues loop to final answer` | ✅ COMPLIANT | +| Tool Registry Shell | Schema export produces internal format | `src/agent/tools/__tests__/registry.test.ts > strips execute from tool definitions, returning pure data schemas` | ✅ COMPLIANT | +| Context Injection into System Prompt | Prompt includes media context and tool guidance | `src/agent/__tests__/system-prompt.test.ts > includes media assets when present` + `includes tool guidance section when tools are provided` | ⚠️ PARTIAL | +| Context Injection into System Prompt | Prompt without media | `src/agent/__tests__/system-prompt.test.ts > is valid when no media assets are loaded` + `includes tool guidance section when tools are provided` | ⚠️ PARTIAL | +| Context Injection into System Prompt | Server-side consumption | `src/agent/__tests__/import-boundary.test.ts > only context.ts imports from @/core in agent/` + route test import/execution | ✅ COMPLIANT | + +**Compliance summary**: 13/17 scenarios compliant + +--- + +### Correctness (Static — Structural Evidence) +| Requirement | Status | Notes | +|------------|--------|-------| +| Provider-agnostic adapter seam | ✅ Implemented | `ProviderAdapter`, `ProviderConfig`, `ProviderResponse`, and `createProvider(config)` isolate route logic from provider SDK details | +| Generic server config | ✅ Implemented | Route reads `LLM_PROVIDER`, `LLM_API_KEY`, `LLM_MODEL`, `LLM_BASE_URL` server-side only | +| Provider-owned wire conversion | ✅ Implemented | Message and tool-schema conversion live in `openai-compatible.ts`, not in the route or registry | +| Provider-agnostic tool schema export | ✅ Implemented | `toToolSchemas()` strips `execute` and returns DTO-only schemas | +| Real provider path excludes `echo_context` | ✅ Implemented | Route passes only `providerToolDefs` with `transcribe_video`; test-only `mock.tool.ts` is not imported by production code | +| Client orchestrator loop | ✅ Implemented | `orchestrator.ts` enforces `MAX_ITERATIONS = 8`, validates args, appends tool results, and loops until final response or cap | +| Tool execution failures handled per-tool | ✅ Implemented | `resolveToolCalls()` catches per-tool failures and converts them into `ToolResult` errors without aborting the loop | +| System prompt is server-consumable | ✅ Implemented | `buildSystemPrompt()` is a pure function with no client-only imports and is used by the route | +| Provider 502 payload matches spec text | ✅ Implemented | Catch block returns `{ error: "LLM provider error" }` | +| Hybrid artifacts reconciled | ✅ Implemented | OpenSpec design/spec/tasks align with the approved MAX_ITERATIONS=8 and provider-error wording decisions | + +--- + +### Coherence (Design) +| Decision | Followed? | Notes | +|----------|-----------|-------| +| Provider integration via adapter factory | ✅ Yes | Route delegates through `createProvider(config)` | +| Generic `LLM_*` config naming | ✅ Yes | Config and route use generic names, not `OPENAI_*` | +| Adapter owns wire conversion | ✅ Yes | Message and tool conversion live in `openai-compatible.ts` | +| Registry exports provider-agnostic schemas | ✅ Yes | `toToolSchemas()` keeps provider wire details out of the registry | +| Tool execution stays client-side | ✅ Yes | Orchestrator resolves tool calls locally | +| `echo_context` hidden from real LLM path | ✅ Yes | Honored: provider-facing path exposes only `transcribe_video` | +| Iteration cap is 8 | ✅ Yes | Implementation and design match the explicit user decision | + +--- + +### Issues Found + +**CRITICAL** (must fix before archive): +- None in the verified slice. + +**WARNING** (should fix): +- The optional-arg `transcribe_video` path is not proven by one end-to-end orchestrator runtime test; evidence is split between tool-isolation and orchestrator-loop tests. +- The “tool call then final answer” and system-prompt scenarios are covered by combined unit evidence rather than one dedicated assertion per scenario. +- Full `bun test` remains red because of an unrelated migration failure outside this slice. + +**SUGGESTION** (nice to have): +- Add one focused runtime test covering: provider returns `transcribe_video` with `{}` args → tool executes → second round returns final answer. +- Add one `buildSystemPrompt()` assertion that checks media context and tool guidance together in the same returned string. + +--- + +### Verdict +PASS WITH WARNINGS + +The requested Phase 1 slice is verified: focused tests are green, type checking passes, the real LLM path stays provider-agnostic, `echo_context` is not exposed to the provider, and the reconciled hybrid artifacts now match the approved decisions. Remaining concerns are coverage-shape warnings plus one unrelated full-suite migration failure outside this change. diff --git a/openspec/specs/agent-context-bridge/spec.md b/openspec/specs/agent-context-bridge/spec.md index e58f0540..ecb7acf7 100644 --- a/openspec/specs/agent-context-bridge/spec.md +++ b/openspec/specs/agent-context-bridge/spec.md @@ -40,19 +40,25 @@ The system SHALL provide an `EditorContextAdapter` with a single method `getCont ### Requirement: Context Injection into System Prompt -The system SHALL provide a `buildSystemPrompt(context: AgentContext)` function that produces a system prompt string including the media summary. When `mediaAssets` is empty, the prompt MUST still be valid but indicate no media loaded. The system prompt template MUST be separate from the orchestrator logic to allow future customization. +The system SHALL provide a `buildSystemPrompt(context: AgentContext)` function that produces a system prompt string including the media summary AND guidance about available tools. When `mediaAssets` is empty, the prompt MUST still be valid but indicate no media loaded. The prompt MUST instruct the LLM that it can call tools from a known set (currently `transcribe_video`) and that it should respond in plain text when no tool is needed. The system prompt template MUST be separate from the orchestrator logic and MUST be consumable by the server route (i.e., a pure function with no client-only dependencies). -#### Scenario: Prompt includes media context +#### Scenario: Prompt includes media context and tool guidance - GIVEN `AgentContext` with `mediaAssets: [{ id: "v1", name: "clip.mp4", type: "video", duration: 30 }]` - WHEN `buildSystemPrompt(context)` is called -- THEN the returned string contains "clip.mp4" and "video" +- THEN the returned string contains "clip.mp4", "video", and references "transcribe_video" as an available tool #### Scenario: Prompt without media - GIVEN `AgentContext` with `mediaAssets: []` - WHEN `buildSystemPrompt(context)` is called -- THEN the returned string is valid and contains "No media assets loaded" +- THEN the returned string is valid, contains "No media assets loaded", and still references available tools + +#### Scenario: Server-side consumption + +- GIVEN `buildSystemPrompt` is imported in the API route +- WHEN the route calls `buildSystemPrompt(context)` +- THEN it returns a string without importing any client-only modules (no `EditorCore`, no WASM, no browser APIs) ### Requirement: No Direct EditorCore Access from Agent diff --git a/openspec/specs/agent-session-shell/spec.md b/openspec/specs/agent-session-shell/spec.md index 4e412720..1ddfac14 100644 --- a/openspec/specs/agent-session-shell/spec.md +++ b/openspec/specs/agent-session-shell/spec.md @@ -2,7 +2,7 @@ ## Purpose -Defines the client-side orchestration boundary between the UI layer, the API proxy, Zustand stores, and the tool registry. This shell wires the full request/response cycle for one mock end-to-end interaction without real LLM or streaming. +Defines the client-side orchestration boundary between the UI layer, the API proxy, Zustand stores, and the tool registry. The route delegates to a config-selected provider adapter for real LLM execution, and the orchestrator drives a multi-turn tool-call loop with iteration guard and argument validation. ## Requirements @@ -41,13 +41,13 @@ The system SHALL maintain an `agentStore` (Zustand) with state: `status: Executi ### Requirement: API Proxy Route -The system SHALL expose `POST /api/agent/chat` that accepts `{ messages: ChatMessage[], context?: AgentContext }` and returns `{ content: string, toolCalls?: ToolCall[] }`. In this phase, the route MUST return a canned mock response — no real LLM call. The route MUST validate the request body schema and return 400 on invalid input. +The system SHALL expose `POST /api/agent/chat` that accepts `{ messages: ChatMessage[], context?: AgentContext }` and returns `{ content: string, toolCalls?: ToolCall[] }`. The route MUST resolve the provider adapter from configuration, assemble a system prompt via `buildSystemPrompt(context)`, and pass internal tool schemas from the registry to the adapter. The adapter handles all provider-specific wire-format conversion. The request body MUST be validated via zod; invalid input returns 400. Provider errors return 502. -#### Scenario: Valid request returns mock response +#### Scenario: Valid request returns provider response -- GIVEN a POST request with valid `{ messages: [{ role: "user", content: "Hi" }] }` -- WHEN the handler processes the request -- THEN it returns 200 with `{ content: "...", toolCalls: [] }` +- GIVEN provider config is set and a POST with valid `{ messages, context }` arrives +- WHEN the handler calls through the provider adapter +- THEN it returns 200 with `{ content: "...", toolCalls?: [...] }` #### Scenario: Invalid request returns 400 @@ -55,9 +55,15 @@ The system SHALL expose `POST /api/agent/chat` that accepts `{ messages: ChatMes - WHEN the handler processes the request - THEN it returns 400 with an error description +#### Scenario: Provider error returns 502 + +- GIVEN the provider adapter call throws a network or auth error +- WHEN the handler catches it +- THEN it returns 502 with `{ error: "LLM provider error" }` + ### Requirement: Orchestrator Shell -The system SHALL provide a client-side `orchestrator.run(messages, context)` function that: (1) sets `agentStore.status` to `"calling"`, (2) calls the API proxy, (3) if the response contains `toolCalls`, resolves each via the tool registry, (4) appends the assistant response to `chatStore`, (5) sets status to `"idle"`. On error at any step, the orchestrator MUST set `chatStore.error` and `agentStore.status` to `"error"`. +The system SHALL provide a client-side `orchestrator.run(messages, context)` function that iterates: (1) sets `agentStore.status` to `"sending"`, (2) POSTs `{ messages, context }` to `/api/agent/chat`, (3) if the response contains `toolCalls`, validates args, resolves each via the tool registry (if a single tool throws, the error MUST be captured in a `ToolResult` and the loop MUST continue so the LLM can recover), appends tool results to the message history, and loops back to step 2 with the updated history, (4) when the response has no `toolCalls`, appends the final assistant message to `chatStore`, sets status to `"idle"`. The loop MUST NOT exceed the max iteration guard. On API or network errors (not per-tool errors), the orchestrator MUST set `chatStore.error` and `agentStore.status` to `"error"`. #### Scenario: Happy path with no tool calls @@ -66,23 +72,31 @@ The system SHALL provide a client-side `orchestrator.run(messages, context)` fun - THEN `chatStore.messages` contains the assistant reply "Hello!" - AND `agentStore.status` is `"idle"` -#### Scenario: Tool call resolution +#### Scenario: Tool call then final answer -- GIVEN the API returns `{ content: "", toolCalls: [{ name: "echo_context", args: {} }] }` -- AND `echo_context` is registered in the tool registry -- WHEN the orchestrator resolves tool calls -- THEN the tool executes and its result is appended to `chatStore.messages` +- GIVEN iteration 1 returns `{ content: "", toolCalls: [{ name: "transcribe_video", args: {} }] }` +- AND the tool executes successfully +- AND iteration 2 returns `{ content: "Here is the transcription...", toolCalls: [] }` +- WHEN the orchestrator loop completes +- THEN `chatStore.messages` contains the tool result AND the final assistant message - AND `agentStore.status` is `"idle"` +#### Scenario: Error during tool execution — per-tool recovery + +- GIVEN iteration 1 returns `{ content: "", toolCalls: [{ name: "transcribe_video", args: {} }] }` and the tool throws +- WHEN the orchestrator catches the error inside `resolveToolCalls` +- THEN a `ToolResult` with the error description is appended to the message history +- AND the loop continues, sending tool-result back to the LLM for a final answer + ### Requirement: Tool Registry Shell -The system SHALL provide a `ToolRegistry` extending `DefinitionRegistry` where `ToolDefinition` contains `{ name, description, parameters, execute: (args, context) => Promise }`. In this phase exactly ONE tool — `echo_context` — MUST be registered. The registry MUST support `register`, `get`, `has`, and `getAll`. +The system SHALL provide a `ToolRegistry` extending `DefinitionRegistry` where `ToolDefinition` contains `{ name, description, parameters, execute }`. The registry MUST support `register`, `get`, `has`, `getAll`, and a `toToolSchemas()` method that exports every registered tool's parameters in a provider-agnostic internal schema format. The provider adapter is responsible for converting these internal schemas to its specific wire format. In this phase `echo_context` MUST NOT be registered; exactly one tool — `transcribe_video` — MUST be registered. -#### Scenario: Mock echo tool executes +#### Scenario: Schema export produces internal format -- GIVEN `echo_context` is registered with `execute: (args, ctx) => ({ projectId: ctx.projectId, ... })` -- WHEN `registry.get("echo_context").execute({}, context)` is called -- THEN it returns a context summary with projectId and mediaCount +- GIVEN `transcribe_video` is registered with parameters `[{ key: "assetId", type: "string", required: false }, ...]` +- WHEN `toToolSchemas()` is called +- THEN it returns an array with one entry containing `{ name: "transcribe_video", description, parameters: [...] }` in the internal format ### Requirement: Shared Foundational Types @@ -93,3 +107,62 @@ The system SHALL define types in `apps/web/src/agent/types.ts`: `ChatMessage { i - GIVEN `types.ts` exports `ChatMessage`, `ToolCall`, `ToolResult`, `ExecutionState`, `AgentContext`, `ToolDefinition` - WHEN any module in `agent/`, `stores/`, or `app/api/agent/` imports from it - THEN TypeScript compiles without errors + +### Requirement: Provider Adapter Interface + +The system SHALL define a server-side provider adapter interface with a single method `chat(params: ProviderChatParams): Promise`. `ProviderChatParams` MUST include `messages`, `systemPrompt`, and `toolSchemas` in a provider-agnostic internal format. `ProviderChatResponse` MUST include `content: string` and `toolCalls?: ProviderToolCall[]` where `ProviderToolCall { id, name, args }`. Each concrete adapter converts internal tool schemas to its own wire format. Phase 1 MAY ship exactly one adapter (OpenAI-compatible) as long as the interface contract remains provider-agnostic. + +#### Scenario: Adapter translates tool schemas to wire format + +- GIVEN `transcribe_video` has internal parameters `[{ key: "assetId", type: "string", required: false }]` +- WHEN the adapter receives tool schemas before calling the provider +- THEN it converts them to its provider-specific wire format + +#### Scenario: Adapter selected by config + +- GIVEN `LLM_PROVIDER` is set to `"openai-compatible"` +- WHEN the route resolves the adapter +- THEN the OpenAI-compatible adapter is returned + +### Requirement: Provider Configuration + +Server-only environment variables `LLM_PROVIDER` (required), `LLM_MODEL` (required), `LLM_API_KEY` (required), and `LLM_BASE_URL` (optional) MUST determine which adapter and model the route uses. Configuration MUST be read server-side only — never exposed to the client. + +#### Scenario: Missing required configuration + +- GIVEN `LLM_API_KEY` is not set in the environment +- WHEN the route attempts to initialize the provider +- THEN the call returns 502 with a missing-configuration error + +### Requirement: Tool Argument Validation + +Before executing any tool call, the orchestrator MUST validate that each argument key declared `required: true` in the tool's `parameters` is present in the call args and is of the declared type. If validation fails, the tool MUST NOT execute; instead the orchestrator MUST produce a `ToolResult` with `error` describing the missing or malformed argument. + +#### Scenario: Missing required argument + +- GIVEN `transcribe_video` declares no required parameters and the LLM returns `toolCalls: [{ name: "transcribe_video", args: {} }]` +- WHEN the orchestrator validates args +- THEN validation passes and the tool executes + +#### Scenario: Wrong argument type + +- GIVEN a tool declares `{ key: "assetId", type: "string", required: true }` +- AND the LLM returns `args: { assetId: 123 }` +- WHEN the orchestrator validates args +- THEN the tool does NOT execute and a `ToolResult` with `error` containing "assetId" is returned + +### Requirement: Max Iteration Guard + +The orchestrator loop MUST enforce a hard iteration cap of **8**. An iteration is one round of "call provider → resolve tool calls → feed results back." If the cap is reached, the orchestrator MUST stop, append a final assistant message indicating the limit was reached, and set `agentStore.status` to `"idle"`. + +#### Scenario: Cap reached mid-loop + +- GIVEN the provider has returned tool calls on 7 consecutive iterations +- WHEN iteration 8 also returns tool calls +- THEN the orchestrator stops, appends "Reached maximum iteration limit" to `chatStore`, and sets status to `"idle"` + +#### Scenario: Loop completes within cap + +- GIVEN the provider returns a final text response after 2 tool-call iterations +- WHEN the orchestrator processes the response +- THEN no cap message is appended and status is `"idle"`