From e2be7629c2aa245bc52e11906180343c4ccf429c Mon Sep 17 00:00:00 2001 From: siddseethepalli Date: Thu, 16 Apr 2026 21:27:00 +0000 Subject: [PATCH] runtime/analyze-conversation: route through callSite: 'analyzeConversation' --- .../__tests__/analyze-conversation.test.ts | 68 +++++++------------ .../runtime/services/analyze-conversation.ts | 32 ++++----- 2 files changed, 41 insertions(+), 59 deletions(-) diff --git a/assistant/src/runtime/services/__tests__/analyze-conversation.test.ts b/assistant/src/runtime/services/__tests__/analyze-conversation.test.ts index 93d367eaeb1..47f1ab9a9da 100644 --- a/assistant/src/runtime/services/__tests__/analyze-conversation.test.ts +++ b/assistant/src/runtime/services/__tests__/analyze-conversation.test.ts @@ -50,23 +50,6 @@ mock.module("../../../export/transcript-formatter.js", () => ({ buildAnalysisTranscript: () => "user: hi", })); -// Default config stub — individual tests can override via mockGetConfig. -interface AnalysisConfigStub { - analysis: { - modelIntent?: string; - modelOverride?: string; - }; -} -const mockGetConfig = mock( - (): AnalysisConfigStub => ({ - analysis: {}, - }), -); - -mock.module("../../../config/loader.js", () => ({ - getConfig: mockGetConfig, -})); - import { AssistantEventHub } from "../../assistant-event-hub.js"; import type { SendMessageDeps } from "../../http-types.js"; import { analyzeConversation } from "../analyze-conversation.js"; @@ -90,8 +73,6 @@ beforeEach(() => { mockFindAnalysisConversationFor.mockImplementation(() => null); mockGetConversationSource.mockReset(); mockGetConversationSource.mockImplementation(() => null); - mockGetConfig.mockReset(); - mockGetConfig.mockImplementation(() => ({ analysis: {} })); }); function makeConversation() { @@ -221,12 +202,17 @@ describe("analyzeConversation", () => { expect(allowedTools).toBeInstanceOf(Set); expect(allowedTools?.size).toBe(0); - // Fires the agent loop. + // Fires the agent loop with the analyzeConversation call-site so the + // per-call provider config flows through `resolveCallSiteConfig`. expect(conversation.runAgentLoop).toHaveBeenCalledWith( expect.any(String), "msg-1", expect.any(Function), - expect.objectContaining({ isInteractive: false, isUserMessage: true }), + expect.objectContaining({ + isInteractive: false, + isUserMessage: true, + callSite: "analyzeConversation", + }), ); }); @@ -405,40 +391,36 @@ describe("analyzeConversation", () => { expect(mockAddMessage).not.toHaveBeenCalled(); }); - test("auto: passes modelOverride through to getOrCreateConversation when set in config", async () => { - mockGetConfig.mockImplementation(() => ({ - analysis: { - modelIntent: "quality-optimized", - modelOverride: "claude-opus-4-6", - }, - })); + test("auto: routes the agent loop through callSite: 'analyzeConversation'", async () => { const conversation = makeConversation(); const deps = makeDeps(conversation); await analyzeConversation("conv-1", deps, { trigger: "auto" }); - expect(deps.getOrCreateConversation).toHaveBeenCalledWith( - "analysis-new", - expect.objectContaining({ - modelIntent: "quality-optimized", - modelOverride: "claude-opus-4-6", - }), + expect(conversation.runAgentLoop).toHaveBeenCalledWith( + expect.any(String), + "msg-1", + expect.any(Function), + expect.objectContaining({ callSite: "analyzeConversation" }), ); }); - test("auto: does not pass modelOverride/modelIntent keys when config leaves them unset", async () => { + test("does not thread modelIntent/modelOverride into getOrCreateConversation", async () => { + // Per-call model selection now happens via the call-site resolver against + // `llm.callSites.analyzeConversation`, not via legacy modelIntent/ + // modelOverride keys on the conversation create options. const conversation = makeConversation(); const deps = makeDeps(conversation); await analyzeConversation("conv-1", deps, { trigger: "auto" }); - const [, passedOpts] = ( - deps.getOrCreateConversation.mock.calls as unknown as Array< - [string, Record] - > - )[0] ?? ["", {}]; - expect(passedOpts).toBeDefined(); - expect("modelIntent" in (passedOpts ?? {})).toBe(false); - expect("modelOverride" in (passedOpts ?? {})).toBe(false); + const calls = deps.getOrCreateConversation.mock + .calls as unknown as Array<[string, Record | undefined]>; + expect(calls.length).toBe(1); + const passedOpts = calls[0]?.[1]; + if (passedOpts !== undefined) { + expect("modelIntent" in passedOpts).toBe(false); + expect("modelOverride" in passedOpts).toBe(false); + } }); }); diff --git a/assistant/src/runtime/services/analyze-conversation.ts b/assistant/src/runtime/services/analyze-conversation.ts index 3efffebf73c..90d584e6ce4 100644 --- a/assistant/src/runtime/services/analyze-conversation.ts +++ b/assistant/src/runtime/services/analyze-conversation.ts @@ -8,15 +8,18 @@ * Two triggers are supported: * - **manual**: user-initiated analysis. Creates a fresh conversation each * invocation, runs with `trustClass: "unknown"`, and strips the tool - * surface. Byte-for-byte unchanged from the original route logic. + * surface. * - **auto**: called by the auto-analyze job when a source conversation * reaches a natural pause. Reuses a rolling analysis conversation per * parent (creating one if none exists), runs with `trustClass: * "guardian"`, and keeps the full tool surface so the analysis agent can - * write memory and skills directly. Reads optional model overrides from - * `analysis.modelIntent` / `analysis.modelOverride` config. + * write memory and skills directly. + * + * Both triggers route the agent loop through `callSite: 'analyzeConversation'` + * so per-call provider/model selection flows through `resolveCallSiteConfig` + * against `llm.callSites.analyzeConversation` (populated by the unify-llm + * workspace migration from the legacy `analysis.modelIntent`/`modelOverride`). */ -import { getConfig } from "../../config/loader.js"; import type { ServerMessage } from "../../daemon/message-protocol.js"; import { AUTO_ANALYSIS_GROUP_ID, @@ -31,7 +34,6 @@ import { getMessages, } from "../../memory/conversation-crud.js"; import { resolveConversationId } from "../../memory/conversation-key-store.js"; -import type { ModelIntent } from "../../providers/types.js"; import { getLogger } from "../../util/logger.js"; import { buildAssistantEvent } from "../assistant-event.js"; import { DAEMON_INTERNAL_ASSISTANT_ID } from "../assistant-scope.js"; @@ -173,8 +175,6 @@ export async function analyzeConversation( let prompt: string; let trustClass: "unknown" | "guardian"; let stripTools: boolean; - let modelIntent: ModelIntent | undefined; - let modelOverride: string | undefined; if (opts.trigger === "manual") { const newConv = createConversation({ @@ -205,10 +205,6 @@ export async function analyzeConversation( prompt = buildAutoAnalysisPrompt(transcript); trustClass = "guardian"; stripTools = false; - - const analysisConfig = getConfig().analysis; - modelIntent = analysisConfig.modelIntent; - modelOverride = analysisConfig.modelOverride; } // h. Load the conversation into memory with the appropriate trust @@ -219,13 +215,14 @@ export async function analyzeConversation( // Hoisted ahead of message persistence so the auto branch can detect a // still-running prior agent loop on the rolling conversation and bail out // before mutating any state. See concurrency guard below. + // + // Per-call model selection is no longer threaded through `getOrCreateConversation`; + // the runAgentLoop call below passes `callSite: 'analyzeConversation'` so the + // unified call-site resolver picks up provider/model from + // `llm.callSites.analyzeConversation`. const analysisConversation = await deps.sendMessageDeps.getOrCreateConversation( analysisConversationId, - { - ...(modelIntent !== undefined ? { modelIntent } : {}), - ...(modelOverride !== undefined ? { modelOverride } : {}), - }, ); // h.1. Concurrency guard (auto trigger only). The rolling analysis @@ -298,11 +295,14 @@ export async function analyzeConversation( analysisConversation.abortController = new AbortController(); analysisConversation.currentRequestId = crypto.randomUUID(); - // l. Fire-and-forget the agent loop + // l. Fire-and-forget the agent loop. `callSite: 'analyzeConversation'` + // routes the per-call provider config through `resolveCallSiteConfig` + // against `llm.callSites.analyzeConversation`. analysisConversation .runAgentLoop(prompt, messageId, onEvent, { isInteractive: false, isUserMessage: true, + callSite: "analyzeConversation", }) .catch((err) => { log.error(