diff --git a/.changeset/ai-session-titles.md b/.changeset/ai-session-titles.md new file mode 100644 index 000000000..b0638829a --- /dev/null +++ b/.changeset/ai-session-titles.md @@ -0,0 +1,18 @@ +--- +"@nanocollective/nanocoder": minor +--- + +Added automatic session titles. +A session keeps its opening prompt as the title, and when that prompt is too thin to be useful the agent generates a descriptive name once, after the first turn that ran a tool or the first follow-up message. +Manual renames are never overwritten. + +Titling runs in the ACP agent, so it applies to the VS Code extension and other ACP clients; the CLI keeps its heuristic title. +It uses the session's own model by default - set `sessions.titleModel` / `sessions.titleProvider` to point it at a cheaper or local one, or `sessions.smartTitles: false` to turn it off. +Pointing `titleProvider` at a different provider sends it the opening user turns and a summary of the tools that ran, which includes file paths and bash command strings. + +Two things worth knowing: +the tokens a title costs are billed by the provider but are not counted in `/usage`, since the call is made outside the conversation loop that builds usage records; +and existing sessions are retitled from their first message on their next autosave, which is a one-time visible reshuffle of the history list. + +Also fixed the CLI's autosave deriving the session title from the latest user message and rewriting it on every save, which overwrote titles in the store the VS Code extension reads from. +Closes #808. diff --git a/plugins/vscode/src/acp-client.spec.ts b/plugins/vscode/src/acp-client.spec.ts index e2e9a30dc..c63cf2e45 100644 --- a/plugins/vscode/src/acp-client.spec.ts +++ b/plugins/vscode/src/acp-client.spec.ts @@ -30,6 +30,21 @@ test('NanocoderAcpClient - permission flow', async (t) => { t.false(client.hasPendingPermissions(), 'Pending permissions should be cleared'); }); +test('NanocoderAcpClient - forwards background title notifications', async (t) => { + const client = makeClient({}); + let notified = false; + client.onSessionTitleChanged = () => { + notified = true; + }; + + await client.handleExtNotification('_nanocoder/sessionTitleChanged', { + sessionId: 'session-1', + title: 'Updated title', + }); + + t.true(notified); +}); + function makeClient(connection: unknown) { const outputChannel = { appendLine: () => {} } as any; const client = new NanocoderAcpClient(outputChannel, new AcpStateManager()); diff --git a/plugins/vscode/src/acp-client.ts b/plugins/vscode/src/acp-client.ts index 499584b95..b58a96087 100644 --- a/plugins/vscode/src/acp-client.ts +++ b/plugins/vscode/src/acp-client.ts @@ -28,6 +28,8 @@ export class NanocoderAcpClient { public onStateSync?: (state: StateSyncPayload) => void; public onSessionArtifacts?: (meta: unknown) => void; public onConnectionReady?: () => void; + /** Fires when a background title update is received. */ + public onSessionTitleChanged?: () => void; public currentMode?: string; public availableModes: string[] = []; @@ -90,6 +92,13 @@ export class NanocoderAcpClient { this._clearPendingPermissions(); } + /** Handle custom notifications from the agent. */ + async handleExtNotification(method: string, _params: unknown): Promise { + if (method === '_nanocoder/sessionTitleChanged') { + this.onSessionTitleChanged?.(); + } + } + async handlePermissionRequest(params: any): Promise { const toolCall = params.toolCall; const toolCallId = toolCall.toolCallId; diff --git a/plugins/vscode/src/acp-process-manager.ts b/plugins/vscode/src/acp-process-manager.ts index 004ef4eef..43c195881 100644 --- a/plugins/vscode/src/acp-process-manager.ts +++ b/plugins/vscode/src/acp-process-manager.ts @@ -161,6 +161,9 @@ export class AcpProcessManager { }, requestPermission: async (params: any) => { return this.acpClient.handlePermissionRequest(params); + }, + extNotification: async (method: string, params: any) => { + return this.acpClient.handleExtNotification(method, params); } } as any), stream); this.acpClient.setConnection(connection); diff --git a/plugins/vscode/src/chat-webview-provider.ts b/plugins/vscode/src/chat-webview-provider.ts index 19a908d30..4ef6d4f07 100644 --- a/plugins/vscode/src/chat-webview-provider.ts +++ b/plugins/vscode/src/chat-webview-provider.ts @@ -114,6 +114,11 @@ export class ChatWebviewProvider }); }; + // Refresh the history list after a background title update. + this._acpClient.onSessionTitleChanged = () => { + void this._broadcastSessions(); + }; + this._acpClient.onConnectionReady = () => { this._initializeSessionIfReady(); }; diff --git a/source/acp/acp-agent.spec.ts b/source/acp/acp-agent.spec.ts index 25afb86bd..6ab6a6ae7 100644 --- a/source/acp/acp-agent.spec.ts +++ b/source/acp/acp-agent.spec.ts @@ -964,6 +964,103 @@ test.serial( }, ); +// ============================================================================ +// background session titling +// ============================================================================ + +test('AcpAgent.prompt - a weak title waits for meaningful context', async t => { + const {agent} = createAgent(); + + let chatCalls = 0; + agent['initContext'].client.chat = async () => { + chatCalls++; + // Calls 1 and 2 are conversation turns; call 3 is the titler. + return chatCalls < 3 + ? {choices: [{message: {content: 'Done.'}}]} + : {choices: [{message: {content: 'Fix Login Redirect'}}]}; + }; + + const session = await agent.newSession({cwd: '/tmp'}); + await agent.prompt({ + sessionId: session.sessionId, + prompt: [{type: 'text', text: 'fix this'}], + }); + + await agent['pendingTitleGeneration']; + const beforeContext = await sessionManager.readSession(session.sessionId); + t.not(beforeContext?.titleGenerated, true); + t.is(chatCalls, 1); + + await agent.prompt({ + sessionId: session.sessionId, + prompt: [{type: 'text', text: 'summarize the README'}], + }); + + await agent['pendingTitleGeneration']; + const titled = await sessionManager.readSession(session.sessionId); + t.true(titled?.titleGenerated, 'expected a generated title to be persisted'); + t.is(titled?.title, 'Fix Login Redirect'); + // A generated title must never masquerade as a user rename. + t.not(titled?.titleManuallySet, true); + + // A third turn must not re-title: titleGenerated short-circuits it. + await agent.prompt({ + sessionId: session.sessionId, + prompt: [{type: 'text', text: 'and now this'}], + }); + await agent['pendingTitleGeneration']; + + const after = await sessionManager.readSession(session.sessionId); + t.is(after!.title, 'Fix Login Redirect'); + // Exactly one more chat call, the conversation turn, and no second titler. + t.is(chatCalls, 4); +}); + +test('AcpAgent.prompt - a cancelled turn does not generate a title', async t => { + const {agent} = createAgent(); + + let chatCalls = 0; + agent['initContext'].client.chat = async () => { + chatCalls++; + throw new Error('Operation was cancelled'); + }; + + const session = await agent.newSession({cwd: '/tmp'}); + await agent.prompt({ + sessionId: session.sessionId, + prompt: [{type: 'text', text: 'fix this'}], + }); + await agent['pendingTitleGeneration']; + + // The cancel path early-returns from inside catch, which still runs the + // finally. Reaching the finally must not be mistaken for a clean turn. + t.is(chatCalls, 1); + const stored = await sessionManager.readSession(session.sessionId); + t.not(stored?.titleGenerated, true); +}); + +test('AcpAgent.prompt - an errored turn does not generate a title', async t => { + const {agent} = createAgent(); + + let chatCalls = 0; + agent['initContext'].client.chat = async () => { + chatCalls++; + throw new Error('RequestError: Internal error (500)'); + }; + + const session = await agent.newSession({cwd: '/tmp'}); + await t.throwsAsync( + agent.prompt({ + sessionId: session.sessionId, + prompt: [{type: 'text', text: 'fix this'}], + }), + ); + await agent['pendingTitleGeneration']; + + t.is(chatCalls, 1); + const stored = await sessionManager.readSession(session.sessionId); + t.not(stored?.titleGenerated, true); +}); /** Throwaway workspace for the timeline tests, removed by the caller. */ const createTimelineWorkspace = (label: string): string => { const cwd = join( diff --git a/source/acp/acp-agent.ts b/source/acp/acp-agent.ts index 579f6dc79..db560add8 100644 --- a/source/acp/acp-agent.ts +++ b/source/acp/acp-agent.ts @@ -54,7 +54,12 @@ import { import {resolveTune} from '@/config/tune'; import {appendRelevantProjectContextWithCount} from '@/memory/project-context'; import {TimelineManager} from '@/services/timeline-manager'; +import {maybeGenerateTitle} from '@/session/maybe-generate-title'; import {sessionManager} from '@/session/session-manager'; +import { + ACTIVE_FILE_PREFIX, + deriveTitleFromFirstMessage, +} from '@/session/title-generator'; import {getTuneToolMode} from '@/types/config'; import {getLogger} from '@/utils/logging'; import {buildSystemPrompt, setLastBuiltPrompt} from '@/utils/prompt-builder'; @@ -79,6 +84,13 @@ async function listSessionArtifacts(sessionId: string) { } export class AcpAgent implements Agent { + /** + * The in-flight background titling run. Exposed only so tests can await + * work that production deliberately fires and forgets - asserting on it + * with a fixed sleep goes flaky the moment CI is loaded. + */ + private pendingTitleGeneration: Promise = Promise.resolve(); + private sessions = new Map(); private initContext: AcpInitContext; private conn: AgentSideConnection; @@ -192,6 +204,11 @@ export class AcpAgent implements Agent { session.beginTurn(); + // Both the cancel early-return below and the rethrow after it still run + // the finally, so a clean turn has to be tracked explicitly rather than + // inferred from getting there. + let turnSucceeded = false; + try { const {text: userText, images} = await acpContentToUserMessage( params.prompt, @@ -371,6 +388,7 @@ export class AcpAgent implements Agent { nonInteractiveAlwaysAllow, }); this.attachResponseUsage(session, response, previousAssistant); + turnSucceeded = true; return response; } catch (error) { const errorMsg = error instanceof Error ? error.message : String(error); @@ -421,6 +439,30 @@ export class AcpAgent implements Agent { await this.saveAcpSessionToDisk(session).catch(err => { logger.error(`Failed to save ACP session ${session.sessionId}: ${err}`); }); + + // Fire and forget: the turn must return to idle immediately, and a + // cosmetic title landing a moment later is fine. The promise is kept + // only so tests can await it instead of sleeping; nothing in + // production reads it. + if (turnSucceeded) { + this.pendingTitleGeneration = maybeGenerateTitle({ + sessionId: session.sessionId, + messages: session.messages, + client: this.initContext.client, + onTitle: title => { + // notify(), not the deprecated extNotification() alias. + // The client receives it as extNotification(method, params). + // Lands after the turn went idle, so the client may already be + // gone; an unhandled rejection here would kill the agent. + void this.conn + .notify('_nanocoder/sessionTitleChanged', { + sessionId: session.sessionId, + title, + }) + .catch(() => {}); + }, + }).catch(() => {}); + } } } @@ -933,17 +975,17 @@ export class AcpAgent implements Agent { let title = existingSession?.title; if (!title || title === 'New Session') { const firstUserMessage = saveableMessages.find(m => m.role === 'user'); - if (firstUserMessage && typeof firstUserMessage.content === 'string') { - title = firstUserMessage.content.split('\n')[0].substring(0, 50); - } else { - title = 'New Session'; - } + title = + (typeof firstUserMessage?.content === 'string' + ? deriveTitleFromFirstMessage(firstUserMessage.content) + : null) ?? 'New Session'; } await sessionManager.saveSession({ id: session.sessionId, title, titleManuallySet: existingSession?.titleManuallySet, + titleGenerated: existingSession?.titleGenerated, createdAt: existingSession?.createdAt || timestamp, lastAccessedAt: timestamp, messageCount: saveableMessages.length, @@ -954,7 +996,7 @@ export class AcpAgent implements Agent { if (m.role === 'user' && typeof m.content === 'string') { return { ...m, - content: m.content.replace(/^\[Active file: [^\]]+\]\n\n/, ''), + content: m.content.replace(ACTIVE_FILE_PREFIX, ''), }; } return m; diff --git a/source/acp/acp-ext-notification.spec.ts b/source/acp/acp-ext-notification.spec.ts new file mode 100644 index 000000000..e48071879 --- /dev/null +++ b/source/acp/acp-ext-notification.spec.ts @@ -0,0 +1,81 @@ +import test from 'ava'; +import { + AgentSideConnection, + ClientSideConnection, + ndJsonStream, +} from '@agentclientprotocol/sdk'; + +console.log('\nacp-ext-notification.spec.ts'); + +/** + * The agent announces a generated session title with conn.notify(); the VS Code + * client is expected to receive it as extNotification(method, params). + * + * Every other test in this repo stubs the connection, so that dispatch has + * never actually crossed a wire. If the SDK routed it differently, the sidebar + * would silently never refresh while all the unit tests stayed green. + */ +test('a title notification sent with notify() arrives as extNotification', async t => { + const a2b = new TransformStream(); + const b2a = new TransformStream(); + + const received: Array<{method: string; params: unknown}> = []; + let resolveReceived: () => void; + const gotOne = new Promise(r => { + resolveReceived = r; + }); + + // Client side, mirroring how acp-process-manager builds its handler object. + new ClientSideConnection( + () => + ({ + sessionUpdate: async () => {}, + requestPermission: async () => ({outcome: {outcome: 'cancelled'}}), + extNotification: async (method: string, params: unknown) => { + received.push({method, params}); + resolveReceived(); + }, + }) as never, + ndJsonStream(b2a.writable, a2b.readable), + ); + + // Agent side. + const agentConn = new AgentSideConnection( + () => ({}) as never, + ndJsonStream(a2b.writable, b2a.readable), + ); + + await agentConn.notify('_nanocoder/sessionTitleChanged', { + sessionId: 'session-1', + title: 'Fix Login Redirect', + }); + + // Close both directions and drop the deadline timer however this ends. The + // test passes without it today, but a change that stopped the notification + // from arriving would leave the AVA worker holding open streams. + let deadline: ReturnType | undefined; + t.teardown(async () => { + if (deadline) clearTimeout(deadline); + await Promise.allSettled([ + a2b.writable.close(), + b2a.writable.close(), + ]); + }); + + await Promise.race([ + gotOne, + new Promise((_r, reject) => { + deadline = setTimeout( + () => reject(new Error('notification never arrived')), + 3000, + ); + }), + ]); + + t.is(received.length, 1); + t.is(received[0].method, '_nanocoder/sessionTitleChanged'); + t.deepEqual(received[0].params, { + sessionId: 'session-1', + title: 'Fix Login Redirect', + }); +}); diff --git a/source/app/utils/app-util.ts b/source/app/utils/app-util.ts index dd727a959..4af0ac297 100644 --- a/source/app/utils/app-util.ts +++ b/source/app/utils/app-util.ts @@ -10,7 +10,11 @@ import {CopilotLogin} from '@/commands/copilot-login'; import {createStatsDisplayElement} from '@/commands/stats'; import BashProgress from '@/components/bash-progress'; import CommandProgress from '@/components/command-progress'; -import {DELAY_COMMAND_COMPLETE_MS, MAX_SESSION_NAME_LENGTH} from '@/constants'; +import { + BASH_OUTPUT_PREFIX, + DELAY_COMMAND_COMPLETE_MS, + MAX_SESSION_NAME_LENGTH, +} from '@/constants'; import {sharedProposalStore} from '@/memory/proposal-store'; import {CheckpointManager} from '@/services/checkpoint-manager'; import {clearPendingHookContext} from '@/services/lifecycle-hooks'; @@ -184,7 +188,7 @@ async function handleBashCommand( if (llmContext) { const userMessage: Message = { role: 'user', - content: `Bash command output:\n\`\`\`\n$ ${bashCommand}\n${llmContext}\n\`\`\``, + content: `${BASH_OUTPUT_PREFIX}\n\`\`\`\n$ ${bashCommand}\n${llmContext}\n\`\`\``, }; setMessages([...messages, userMessage]); } diff --git a/source/config/index.ts b/source/config/index.ts index 813070481..ac0ed7817 100644 --- a/source/config/index.ts +++ b/source/config/index.ts @@ -264,6 +264,7 @@ export const DEFAULT_SESSION_CONFIG: NonNullable = { maxMessages: 1000, retentionDays: 30, directory: '', + smartTitles: true, }; // Load session configuration and Returns default config if not specified @@ -311,6 +312,19 @@ function loadSessionConfig(): AppConfig['sessions'] { defaults.retentionDays ?? 30, ), directory: sessions.directory || defaults.directory, + smartTitles: + sessions.smartTitles !== undefined + ? Boolean(sessions.smartTitles) + : defaults.smartTitles, + // No default model: unset means "use the session's own". + titleModel: + typeof sessions.titleModel === 'string' + ? sessions.titleModel + : undefined, + titleProvider: + typeof sessions.titleProvider === 'string' + ? sessions.titleProvider + : undefined, }; } return null; @@ -766,6 +780,30 @@ function loadAppConfig(): AppConfig { let _appConfig: AppConfig | null = null; +/** + * Bumped whenever the cached config is dropped or reloaded. + * + * Modules that derive something expensive from config (a constructed client, + * say) cache it against this number instead of re-deriving on every read. They + * cannot simply be reset from here: the interesting ones sit above config in + * the import graph, and reaching down to them would give this module - which + * everything imports - a cycle back through client-factory. + */ +let _configGeneration = 0; + +/** + * How many times the config has been dropped or reloaded this process. + * + * Fold it into a cache key to have that cache follow config edits. A key built + * only from the config values a module reads misses changes underneath them: + * `titleProvider: "ollama"` is the same string before and after its baseURL is + * edited, but it no longer names the same endpoint. + * @public + */ +export function getConfigGeneration(): number { + return _configGeneration; +} + /** * Lazy-loaded app config to avoid circular dependencies during module initialization * @public @@ -800,11 +838,13 @@ export function getRetryLimits(): RetryLimitsConfig { // Function to reload the app configuration (useful after config file changes) export function reloadAppConfig(): void { _appConfig = loadAppConfig(); + _configGeneration++; } // Function to clear the cached app configuration (useful for testing) export function clearAppConfig(): void { _appConfig = null; + _configGeneration++; } let cachedColors: Colors | null = null; diff --git a/source/constants.ts b/source/constants.ts index c59d2f22a..e1be70911 100644 --- a/source/constants.ts +++ b/source/constants.ts @@ -38,6 +38,13 @@ export const MAX_FILE_READ_RETRIES = 3; // === SESSION NAMES === export const MAX_SESSION_NAME_LENGTH = 100; +/** + * Opening of the synthetic `role: 'user'` turn that carries `!bash` output to + * the model. It is protocol, not a request, so title derivation skips it - + * shared with the builder so the two cannot drift. + */ +export const BASH_OUTPUT_PREFIX = 'Bash command output:'; + // === LIMITS === export const MAX_CHECKPOINT_FILES = 50; export const MAX_TIMELINE_ENTRIES = 50; diff --git a/source/hooks/chat-handler/utils/message-helpers.spec.ts b/source/hooks/chat-handler/utils/message-helpers.spec.ts index d1528e24e..1d5f17397 100644 --- a/source/hooks/chat-handler/utils/message-helpers.spec.ts +++ b/source/hooks/chat-handler/utils/message-helpers.spec.ts @@ -1,42 +1,42 @@ import test from 'ava'; -import {displayError} from './message-helpers.js'; import type React from 'react'; -test('displayError - handles cancellation errors specially', t => { +import {ErrorMessage} from '@/components/message-box'; +import {displayError} from './message-helpers.js'; + +function captureErrorMessage(error: unknown): React.ReactElement { let capturedComponent: React.ReactNode = null; - const addToChatQueue = (component: React.ReactNode) => { + displayError(error, 'test', component => { capturedComponent = component; - }; + }); - const error = new Error('Operation was cancelled'); - displayError(error, 'test', addToChatQueue, () => 1); + if (!capturedComponent || typeof capturedComponent !== 'object' || !('props' in capturedComponent)) { + throw new Error('displayError did not enqueue a React element'); + } - t.truthy(capturedComponent); - // Check that component was created (we can't easily inspect JSX in tests) - t.pass(); -}); + return capturedComponent as React.ReactElement; +} -test('displayError - handles generic errors', t => { - let capturedComponent: React.ReactNode = null; - const addToChatQueue = (component: React.ReactNode) => { - capturedComponent = component; - }; +test('displayError - handles cancellation errors specially', t => { + const component = captureErrorMessage(new Error('Operation was cancelled')); + + t.is(component.type, ErrorMessage); + t.is(component.props.message, 'Interrupted by user.'); + t.true(component.props.hideBox); +}); - const error = new Error('Test error'); - displayError(error, 'test', addToChatQueue, () => 1); +test('displayError - formats generic errors', t => { + const component = captureErrorMessage(new Error('Test error')); - t.truthy(capturedComponent); - t.pass(); + t.is(component.type, ErrorMessage); + t.is(component.props.message, 'Test error'); + t.true(component.props.hideBox); }); test('displayError - handles non-Error objects', t => { - let capturedComponent: React.ReactNode = null; - const addToChatQueue = (component: React.ReactNode) => { - capturedComponent = component; - }; - - displayError('string error', 'test', addToChatQueue, () => 1); + const component = captureErrorMessage({reason: 'string error'}); - t.truthy(capturedComponent); - t.pass(); + t.is(component.type, ErrorMessage); + t.is(component.props.message, '{"reason":"string error"}'); + t.true(component.props.hideBox); }); diff --git a/source/hooks/useSessionAutosave.spec.ts b/source/hooks/useSessionAutosave.spec.ts index 6152fbd10..28eb4e992 100644 --- a/source/hooks/useSessionAutosave.spec.ts +++ b/source/hooks/useSessionAutosave.spec.ts @@ -88,6 +88,59 @@ test('approved plan injection does not replace the user-derived title', t => { t.is(title, 'Implement the greeting helper'); }); +test('the first substantive user turn names the session, not the latest', t => { + // The bug this fixes: deriving from the latest user message made the title + // a rolling mirror of whatever was typed most recently, and autosave + // rewrote it every 30 seconds. Every other case here has a single real + // user turn, so it passes under both a forward and a backward scan - this + // one only passes forward. + const title = deriveSessionTitle([ + {role: 'user', content: 'Add rate limiting to the auth endpoints'}, + {role: 'assistant', content: 'Done.'}, + {role: 'user', content: 'Now update the README'}, + {role: 'assistant', content: 'Done.'}, + ]); + + t.is(title, 'Add rate limiting to the auth endpoints'); +}); + +test('bash output never names the session', t => { + // !bash output is pushed as a plain role:'user' turn with no displayOnly + // flag. The forward scan would latch onto it and, unlike the old backward + // scan, never recover once the real request arrived. + const title = deriveSessionTitle([ + { + role: 'user', + content: 'Bash command output:\n```\n$ git status\nOn branch main\n```', + }, + {role: 'assistant', content: 'You are on main.'}, + {role: 'user', content: 'add retry logic to the OpenRouter client'}, + ]); + + t.is(title, 'add retry logic to the OpenRouter client'); +}); + +test('the active-file prefix never becomes the title', t => { + // The VS Code UI prepends this; it is plumbing, not the request. + const title = deriveSessionTitle([ + { + role: 'user', + content: '[Active file: source/app/App.tsx]\n\nfix the crash on resume', + }, + ]); + + t.is(title, 'fix the crash on resume'); +}); + +test('a first turn that is only plumbing falls through to the next real one', t => { + const title = deriveSessionTitle([ + {role: 'user', content: '[Active file: a.ts]\n\n'}, + {role: 'user', content: 'Add rate limiting'}, + ]); + + t.is(title, 'Add rate limiting'); +}); + test('session titles keep the existing 50-character truncation', t => { const content = 'a'.repeat(51); diff --git a/source/hooks/useSessionAutosave.ts b/source/hooks/useSessionAutosave.ts index 417741a93..bdc2e2d27 100644 --- a/source/hooks/useSessionAutosave.ts +++ b/source/hooks/useSessionAutosave.ts @@ -2,7 +2,9 @@ import {useCallback, useEffect, useRef, useState} from 'react'; import {isApprovedPlanMessage} from '@/artifacts/approved-plan'; import {isInternalWalkthroughMessage} from '@/artifacts/walkthrough-lifecycle'; import {getAppConfig} from '@/config/index'; +import {BASH_OUTPUT_PREFIX} from '@/constants'; import {sessionManager} from '@/session/session-manager'; +import {deriveTitleFromFirstMessage} from '@/session/title-generator'; import type {Message} from '@/types/core'; import {formatError} from '@/utils/error-formatter'; import {logWarning} from '@/utils/message-queue'; @@ -25,19 +27,40 @@ export function shouldResetSessionId( return previousMessageCount > 0 && currentMessageCount === 0; } +/** + * The plain title for a CLI-autosaved session. Scans forward: the FIRST real + * user turn names the session, because deriving from the latest one made the + * title a rolling mirror of whatever was typed most recently. Approved-plan + * injections and internal walkthrough protocol messages are plumbing, not + * requests, so they never name a session. Truncation and the active-file + * prefix strip are delegated to the same helper the ACP save path uses - + * both write to this store, so both must agree on the title. + */ +/** + * `!bash` output arrives as a plain `role: 'user'` turn with no displayOnly + * flag, so a forward scan would name the whole session after it and, unlike + * the old backward scan, never recover once a real request arrives. + */ +function isBashOutputMessage(message: Message): boolean { + return ( + typeof message.content === 'string' && + message.content.startsWith(BASH_OUTPUT_PREFIX) + ); +} + export function deriveSessionTitle(messages: Message[]): string { - for (let index = messages.length - 1; index >= 0; index--) { - const message = messages[index]; + for (const message of messages) { if ( - message?.role === 'user' && - !isApprovedPlanMessage(message) && - !isInternalWalkthroughMessage(message) + message?.role !== 'user' || + isApprovedPlanMessage(message) || + isInternalWalkthroughMessage(message) || + isBashOutputMessage(message) ) { - return ( - message.content.substring(0, 50) + - (message.content.length > 50 ? '...' : '') - ); + continue; } + + const title = deriveTitleFromFirstMessage(message.content); + if (title) return title; } return `Session ${new Date().toLocaleDateString()}`; @@ -191,7 +214,9 @@ export function useSessionAutosave({ // the update path instead of calling createSession() again. const liveSessionId = currentSessionIdRef.current; - // Derive a human-readable title from the most recent user message. + // Derive from the FIRST user message, through the same helper the + // ACP path uses. Deriving it from the latest one made the title a + // rolling mirror of whatever was typed most recently. // The full message array is always written - maxMessages bounds only // what is sent to the model (sliced in the conversation loop). const title = deriveSessionTitle(persistedMessages); @@ -202,11 +227,10 @@ export function useSessionAutosave({ // Write the full history — no truncation. session.messages = persistedMessages; session.messageCount = persistedMessages.length; - // A manually-renamed title sticks — don't let the auto-derived - // title clobber it. Currently only the VS Code extension's - // rename sets this flag; the CLI's /rename command only - // updates in-memory display state and never reaches disk. - if (!session.titleManuallySet) { + // A manually-renamed title, or one the ACP agent generated, + // sticks - don't let the auto-derived title clobber either. + // Both flags are written to the same store this hook saves to. + if (!session.titleManuallySet && !session.titleGenerated) { session.title = title; } session.provider = capturedProvider; diff --git a/source/session/maybe-generate-title.spec.ts b/source/session/maybe-generate-title.spec.ts new file mode 100644 index 000000000..2134b0357 --- /dev/null +++ b/source/session/maybe-generate-title.spec.ts @@ -0,0 +1,551 @@ +import {mkdirSync, writeFileSync} from 'node:fs'; +import {mkdtemp, rm} from 'node:fs/promises'; +import {tmpdir} from 'node:os'; +import {join} from 'node:path'; +import test from 'ava'; +import {clearAppConfig} from '@/config/index'; +import type {LLMClient, Message} from '@/types/core'; +import { + maybeGenerateTitle, + resetTitleGenerationState, +} from './maybe-generate-title.js'; +import {SessionManager} from './session-manager.js'; + +console.log('\nmaybe-generate-title.spec.ts'); + +// getAppConfig() reads from disk, so pin it or these tests inherit whatever +// the developer has configured locally. +const testConfigDir = join(tmpdir(), `nanocoder-title-orch-cfg-${Date.now()}`); +mkdirSync(testConfigDir, {recursive: true}); +process.env.NANOCODER_CONFIG_DIR = testConfigDir; +process.chdir(testConfigDir); + +// Session config is read from nanocoder-preferences.json under a `nanocoder` +// key, not from agents.config.json. +function writeSessionConfig(sessions: Record): void { + writeFileSync( + join(testConfigDir, 'nanocoder-preferences.json'), + JSON.stringify({nanocoder: {sessions}}), + ); + clearAppConfig(); +} + +let testDir: string; +let manager: SessionManager; + +test.beforeEach(async () => { + writeSessionConfig({}); + // inFlight and the attempt counter are module state, not per-manager. + resetTitleGenerationState(); + testDir = await mkdtemp(join(tmpdir(), 'title-orch-test-')); + manager = new SessionManager(join(testDir, 'sessions')); + await manager.initialize(); +}); + +test.afterEach(async () => { + if (testDir) await rm(testDir, {recursive: true, force: true}); +}); + +function client(content: string, onChat?: () => void): LLMClient { + return { + getCurrentModel: () => 'fake', + setModel: () => {}, + getContextSize: () => 8192, + getAvailableModels: async () => ['fake'], + getProviderConfig: () => ({name: 'fake'}), + chat: async () => { + onChat?.(); + return {choices: [{message: {role: 'assistant', content}}]}; + }, + clearContext: async () => {}, + getTimeout: () => undefined, + } as unknown as LLMClient; +} + +const turn: Message[] = [ + {role: 'user', content: 'fix this'}, + { + role: 'assistant', + content: 'Done.', + tool_calls: [ + {id: '1', function: {name: 'read_file', arguments: {path: 'a.ts'}}}, + ], + }, +]; + +const greetingTurn: Message[] = [ + {role: 'user', content: 'hi'}, + {role: 'assistant', content: 'Hello! How can I help?'}, +]; + +async function seed(title: string, extra: Record = {}) { + const session = await manager.createSession({ + title, + messageCount: 2, + provider: 'fake', + model: 'fake', + workingDirectory: '/tmp', + messages: turn, + }); + if (Object.keys(extra).length > 0) { + await manager.saveSession({...session, ...extra}); + } + return session; +} + +test('generates and persists a title for a weak session', async t => { + const session = await seed('fix this'); + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: client('Fix Login Redirect'), + manager, + }); + + const reloaded = await manager.readSession(session.id); + t.is(reloaded?.title, 'Fix Login Redirect'); + t.true(reloaded?.titleGenerated); + // Must not masquerade as a user rename, or the user's own rename becomes + // indistinguishable from an AI one. + t.not(reloaded?.titleManuallySet, true); +}); + +test('does not title a greeting until a second user turn adds context', async t => { + const session = await manager.createSession({ + title: 'hi', + messageCount: 2, + provider: 'fake', + model: 'fake', + workingDirectory: '/tmp', + messages: greetingTurn, + }); + let called = false; + + await maybeGenerateTitle({ + sessionId: session.id, + messages: greetingTurn, + client: client('Should Not Be Used', () => { + called = true; + }), + manager, + }); + + const stillGreeting = await manager.readSession(session.id); + t.is(stillGreeting?.title, 'hi'); + t.not(stillGreeting?.titleGenerated, true); + t.false(called); + + await manager.saveSession({ + ...stillGreeting!, + messages: [ + ...greetingTurn, + {role: 'user', content: 'summarize the README'}, + {role: 'assistant', content: 'The README describes the project.'}, + ], + messageCount: 4, + }); + + await maybeGenerateTitle({ + sessionId: session.id, + messages: [ + ...greetingTurn, + {role: 'user', content: 'summarize the README'}, + {role: 'assistant', content: 'The README describes the project.'}, + ], + client: client('README Overview'), + manager, + }); + + const titled = await manager.readSession(session.id); + t.is(titled?.title, 'README Overview'); + t.true(titled?.titleGenerated); +}); + +test('never overwrites a manually renamed title', async t => { + const session = await seed('fix this'); + await manager.renameSession(session.id, 'My Own Name'); + + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: client('Fix Login Redirect'), + manager, + }); + + const reloaded = await manager.readSession(session.id); + t.is(reloaded?.title, 'My Own Name'); +}); + +test('does not call the model at all when the title is already strong', async t => { + const strong = 'refactor session-manager to use atomic writes everywhere'; + const session = await seed(strong); + let called = false; + + await maybeGenerateTitle({ + sessionId: session.id, + messages: [{role: 'user', content: strong}, turn[1]], + client: client('Something Else', () => { + called = true; + }), + manager, + }); + + t.false(called); + t.is((await manager.readSession(session.id))?.title, strong); +}); + +test('does not re-generate once titleGenerated is set', async t => { + const session = await seed('fix this', { + title: 'Already Named', + titleGenerated: true, + }); + let called = false; + + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: client('Something Else', () => { + called = true; + }), + manager, + }); + + t.false(called); + t.is((await manager.readSession(session.id))?.title, 'Already Named'); +}); + +test('does not fire before an assistant message exists', async t => { + const session = await seed('fix this'); + let called = false; + + await maybeGenerateTitle({ + sessionId: session.id, + messages: [{role: 'user', content: 'fix this'}], + client: client('Too Early', () => { + called = true; + }), + manager, + }); + + t.false(called); +}); + +test('leaves the title alone when the model returns nothing usable', async t => { + const session = await seed('fix this'); + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: client(' '), + manager, + }); + + const reloaded = await manager.readSession(session.id); + t.is(reloaded?.title, 'fix this'); + t.not(reloaded?.titleGenerated, true); +}); + +test('reports the title through onTitle', async t => { + const session = await seed('fix this'); + let reported: string | null = null; + + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: client('Fix Login Redirect'), + manager, + onTitle: title => { + reported = title; + }, + }); + + t.is(reported, 'Fix Login Redirect'); +}); + +test('does not report through onTitle when nothing was persisted', async t => { + const session = await seed('fix this'); + let reported: string | null = null; + + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: client(' '), + manager, + onTitle: title => { + reported = title; + }, + }); + + t.is(reported, null); +}); + +test('smartTitles false disables generation entirely', async t => { + writeSessionConfig({smartTitles: false}); + const session = await seed('fix this'); + let called = false; + + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: client('Fix Login Redirect', () => { + called = true; + }), + manager, + }); + + t.false(called); + t.is((await manager.readSession(session.id))?.title, 'fix this'); +}); + +test('a missing session is a no-op, not a throw', async t => { + await t.notThrowsAsync( + maybeGenerateTitle({ + sessionId: '00000000-0000-4000-8000-000000000000', + messages: turn, + client: client('Fix Login Redirect'), + manager, + }), + ); +}); + +test('a rename that lands mid-flight still wins', async t => { + const session = await seed('fix this'); + + // Rename while the model call is in flight. Without the re-read before + // write, the generator would clobber the user's choice. + const racingClient = { + ...client('Fix Login Redirect'), + chat: async () => { + await manager.renameSession(session.id, 'Renamed Mid Flight'); + return { + choices: [ + {message: {role: 'assistant', content: 'Fix Login Redirect'}}, + ], + }; + }, + } as unknown as LLMClient; + + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: racingClient, + manager, + }); + + const reloaded = await manager.readSession(session.id); + t.is(reloaded?.title, 'Renamed Mid Flight'); + t.not(reloaded?.titleGenerated, true); +}); + +test('never rejects, even when the session store throws', async t => { + // The call site invokes this as a bare `void` with no .catch(), so a + // rejection here becomes an unhandled rejection and takes the process down. + // An uninitialised SessionManager does exactly this: readSession builds a + // path from an undefined directory and throws TypeError. + const brokenManager = { + readSession: async () => { + throw new TypeError('paths[0] must be a string'); + }, + saveSession: async () => {}, + } as unknown as SessionManager; + + await t.notThrowsAsync( + maybeGenerateTitle({ + sessionId: '00000000-0000-4000-8000-000000000000', + messages: turn, + client: client('Fix Login Redirect'), + manager: brokenManager, + }), + ); +}); + +test('the follow-up user turn reaches the model, not just the first', async t => { + const session = await manager.createSession({ + title: 'hi', + messageCount: 4, + provider: 'fake', + model: 'fake', + workingDirectory: '/tmp', + messages: greetingTurn, + }); + + let prompt = ''; + const capturingClient = { + ...client('README Overview'), + chat: async (messages: Message[]) => { + prompt = messages.map(m => m.content).join('\n'); + return {choices: [{message: {role: 'assistant', content: 'README Overview'}}]}; + }, + } as unknown as LLMClient; + + await maybeGenerateTitle({ + sessionId: session.id, + messages: [ + ...greetingTurn, + {role: 'user', content: 'summarize the README'}, + {role: 'assistant', content: 'The README describes the project.'}, + ], + client: capturingClient, + manager, + }); + + // Titling waits for this turn, so dropping it would throw away the only + // message that says what the session is actually about. + t.true(prompt.includes('summarize the README')); + t.true(prompt.includes('hi')); +}); + +test('an assistant message without string content does not abort titling', async t => { + const session = await manager.createSession({ + title: 'hi', + messageCount: 4, + provider: 'fake', + model: 'fake', + workingDirectory: '/tmp', + messages: greetingTurn, + }); + + // A resumed session can carry an assistant entry with no string content. + // Reading .trim() off it threw, and the catch turned that into a debug line. + await maybeGenerateTitle({ + sessionId: session.id, + messages: [ + {role: 'assistant', content: undefined as unknown as string}, + {role: 'user', content: 'hi'}, + {role: 'user', content: 'summarize the README'}, + ], + client: client('README Overview'), + manager, + }); + + const saved = await manager.readSession(session.id); + t.is(saved?.title, 'README Overview'); + t.true(saved?.titleGenerated); +}); + +// --------------------------------------------------------------------------- +// Concurrency + +test('two turns finishing together make only one model call', async t => { + const session = await seed('fix this'); + let calls = 0; + let release: (() => void) | undefined; + const gate = new Promise(resolve => { + release = resolve; + }); + + const slow = { + ...client('Fix Login Redirect'), + chat: async () => { + calls++; + await gate; + return { + choices: [{message: {role: 'assistant', content: 'Fix Login Redirect'}}], + }; + }, + } as unknown as LLMClient; + + const both = Promise.all([ + maybeGenerateTitle({sessionId: session.id, messages: turn, client: slow, manager}), + maybeGenerateTitle({sessionId: session.id, messages: turn, client: slow, manager}), + ]); + release?.(); + await both; + + t.is(calls, 1, 'the inFlight guard must close before the first await'); +}); + +// --------------------------------------------------------------------------- +// The timeout has to hold even against a provider that ignores the signal + +test('a chat that never settles does not wedge the session', async t => { + const session = await seed('fix this'); + // Ignores the abort signal entirely, exactly like a provider that does not + // wire it through. Without the race this promise - and the inFlight entry + // with it - would never resolve. + const wedged = { + ...client('unused'), + chat: () => new Promise(() => {}), + } as unknown as LLMClient; + + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: wedged, + manager, + timeoutMs: 30, + }); + + // The real proof: the session is still titleable afterwards. A leaked + // inFlight entry would make every later attempt a silent no-op. + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: client('Fix Login Redirect'), + manager, + }); + + t.is((await manager.readSession(session.id))?.title, 'Fix Login Redirect'); +}); + +// --------------------------------------------------------------------------- +// Giving up: a model that never returns a usable title + +test('stops calling the model after three unusable responses', async t => { + const session = await seed('fix this'); + let calls = 0; + // A paragraph never survives sanitizeTitle, so every attempt "succeeds" at + // the transport level and still yields no title - the common local-model + // failure, and the one that used to re-run on every turn forever. + const paragraph = client( + 'Sure! Here is a title for your session: '.repeat(5), + () => { + calls++; + }, + ); + + for (let i = 0; i < 6; i++) { + await maybeGenerateTitle({ + sessionId: session.id, + messages: turn, + client: paragraph, + manager, + }); + } + + t.is(calls, 3, 'the attempt cap must hold across turns, not per turn'); + const reloaded = await manager.readSession(session.id); + t.is(reloaded?.title, 'fix this'); + t.not(reloaded?.titleGenerated, true); +}); + +test('a spent attempt budget does not leak to another session', async t => { + const exhausted = await seed('fix this'); + const fresh = await seed('fix this'); + let calls = 0; + const paragraph = client( + 'not a title, an entire sentence of prose here', + () => { + calls++; + }, + ); + + for (let i = 0; i < 4; i++) { + await maybeGenerateTitle({ + sessionId: exhausted.id, + messages: turn, + client: paragraph, + manager, + }); + } + const spent = calls; + + await maybeGenerateTitle({ + sessionId: fresh.id, + messages: turn, + client: client('Fix Login Redirect'), + manager, + }); + + t.is(calls, spent, 'the second session must not spend the first budget'); + t.is((await manager.readSession(fresh.id))?.title, 'Fix Login Redirect'); +}); diff --git a/source/session/maybe-generate-title.ts b/source/session/maybe-generate-title.ts new file mode 100644 index 000000000..e4d7199b1 --- /dev/null +++ b/source/session/maybe-generate-title.ts @@ -0,0 +1,159 @@ +import {getAppConfig} from '@/config/index'; +import type {LLMClient, Message} from '@/types/core'; +import {getLogger} from '@/utils/logging'; +import {type SessionManager, sessionManager} from './session-manager'; +import {resolveTitleClient} from './title-client'; +import { + extractToolSummaries, + extractUserMessages, + generateSessionTitle, + isWeakTitle, +} from './title-generator'; + +/** Local models can be slow, but a cosmetic title is never worth hanging on. */ +const TITLE_TIMEOUT_MS = 20_000; + +/** + * A model that never returns a usable title would otherwise re-run the whole + * path on every turn for the life of the session. Small local models are both + * the target audience here and the population most likely to ignore "reply + * with only the title", so that is the expected path, not an edge case. + */ +const MAX_TITLE_ATTEMPTS = 3; + +/** Stops two turns finishing close together from both launching a call. */ +const inFlight = new Set(); + +/** Attempts spent per session, so a session that never titles stops trying. */ +const attemptsBySession = new Map(); + +/** Test seam. Production code never calls this. */ +export function resetTitleGenerationState(): void { + inFlight.clear(); + attemptsBySession.clear(); +} + +export interface MaybeGenerateTitleOptions { + sessionId: string; + /** The conversation so far. Only the first turn is ever read. */ + messages: Message[]; + /** The session's own client. Used unless config names an override. */ + client: LLMClient; + /** Injected so tests can point at a temp directory. */ + manager?: SessionManager; + /** Called only when a title was actually persisted, for live UI updates. */ + onTitle?: (title: string) => void; + /** Test seam, so the wedged-provider case need not wait the real 20s. */ + timeoutMs?: number; +} + +/** + * Give a session a real name, at most once, and only when the cheap heuristic + * title is too thin to be useful. Every failure path is silent: the heuristic + * title stands and the turn is unaffected. + */ +export async function maybeGenerateTitle( + options: MaybeGenerateTitleOptions, +): Promise { + try { + await runTitleGeneration(options); + } catch (error) { + getLogger().debug(`Session title generation failed: ${error}`); + } +} + +async function runTitleGeneration( + options: MaybeGenerateTitleOptions, +): Promise { + const {sessionId, messages, client, onTitle} = options; + const timeoutMs = options.timeoutMs ?? TITLE_TIMEOUT_MS; + const manager = options.manager ?? sessionManager; + + // Every guard from here to inFlight.add() is synchronous. Nothing may await + // in between, or two turns finishing together both pass the check and both + // make a call - the exact case this set exists to prevent. + if (getAppConfig().sessions?.smartTitles === false) return; + if (inFlight.has(sessionId)) return; + if ((attemptsBySession.get(sessionId) ?? 0) >= MAX_TITLE_ATTEMPTS) return; + + const firstUser = messages.find(m => m.role === 'user'); + if (!firstUser || typeof firstUser.content !== 'string') return; + if (!messages.some(m => m.role === 'assistant')) return; + if (!isWeakTitle(firstUser.content)) return; + + const toolSummaries = extractToolSummaries(messages); + const userMessages = extractUserMessages(messages); + if (userMessages.length < 2 && toolSummaries.length === 0) return; + + inFlight.add(sessionId); + // Not the session's own controller: AcpSession.cancel() swaps that one out, + // so borrowing it would leave this call attached to a stale controller. + const timeout = new AbortController(); + let timer: ReturnType | undefined; + + try { + const session = await manager.readSession(sessionId); + if (!session) return; + if (session.titleManuallySet || session.titleGenerated) return; + + const assistantReply = messages.find( + m => + m.role === 'assistant' && + typeof m.content === 'string' && + m.content.trim().length > 0, + )?.content; + + // Spend the attempt before making it, so a null response, a throw, a + // timeout and a provider that never settles all cost the same. Cleared + // only on success, which is also when titleGenerated stops us anyway. + // + // Transient failures are capped alongside unusable responses on purpose: + // the two are indistinguishable from here without asking the provider why + // it failed, and the cost of getting it wrong is asymmetric. Over-capping + // leaves one session on its heuristic title; under-capping bills a model + // call every turn for the life of the session. + attemptsBySession.set( + sessionId, + (attemptsBySession.get(sessionId) ?? 0) + 1, + ); + + // The abort signal asks the provider to stop; the race is what makes the + // bound hold. A provider that ignores the signal would otherwise leave + // this promise unsettled, and with it the inFlight entry, so the session + // could never be titled again for the process lifetime. + const deadline = new Promise(resolve => { + timer = setTimeout(() => { + timeout.abort(); + resolve(null); + }, timeoutMs); + }); + + const title = await Promise.race([ + (async () => { + const titleClient = await resolveTitleClient(client); + return generateSessionTitle( + titleClient, + {userMessages, toolSummaries, assistantReply}, + timeout.signal, + ); + })(), + deadline, + ]); + if (!title) return; + + // Re-read: the user may have renamed the session while we were waiting. + // Without this the generator races a manual rename and wins. + const fresh = await manager.readSession(sessionId); + if (!fresh || fresh.titleManuallySet || fresh.titleGenerated) return; + + // saveSession, never renameSession - the latter sets titleManuallySet, + // which would make an AI title indistinguishable from the user's own. + await manager.saveSession({...fresh, title, titleGenerated: true}); + + attemptsBySession.delete(sessionId); + onTitle?.(title); + } finally { + if (timer) clearTimeout(timer); + inFlight.delete(sessionId); + } +} diff --git a/source/session/session-manager.spec.ts b/source/session/session-manager.spec.ts index 9a735bb17..a16a47339 100644 --- a/source/session/session-manager.spec.ts +++ b/source/session/session-manager.spec.ts @@ -1086,3 +1086,69 @@ test.serial( t.is(loaded!.title, 'Kept title'); }, ); + +test('titleGenerated round-trips through save and read', async t => { + const session = await manager.createSession({ + title: 'hi', + messageCount: 1, + provider: 'ollama', + model: 'qwen3', + workingDirectory: '/tmp', + messages: [{role: 'user', content: 'hi'}], + }); + + await manager.saveSession({ + ...session, + title: 'Fix Login Bug', + titleGenerated: true, + }); + + const reloaded = await manager.readSession(session.id); + t.is(reloaded?.title, 'Fix Login Bug'); + t.true(reloaded?.titleGenerated); +}); + +test('titleGenerated survives an index rebuild', async t => { + const session = await manager.createSession({ + title: 'hi', + messageCount: 1, + provider: 'ollama', + model: 'qwen3', + workingDirectory: '/tmp', + messages: [{role: 'user', content: 'hi'}], + }); + await manager.saveSession({ + ...session, + title: 'Fix Login Bug', + titleGenerated: true, + }); + + // Corrupt the index so listSessions is forced to rebuild it from the + // session files on disk. rebuildIndex hand-lists every metadata field, + // so a field missing there is dropped silently. + await writeFile(join(sessionsDir, 'sessions.json'), 'not json at all'); + + const listed = await manager.listSessions(); + const found = listed.find(s => s.id === session.id); + t.truthy(found); + t.true(found?.titleGenerated); +}); + +test('titleManuallySet also survives an index rebuild', async t => { + const session = await manager.createSession({ + title: 'hi', + messageCount: 1, + provider: 'ollama', + model: 'qwen3', + workingDirectory: '/tmp', + messages: [{role: 'user', content: 'hi'}], + }); + await manager.renameSession(session.id, 'My Own Name'); + + await writeFile(join(sessionsDir, 'sessions.json'), 'not json at all'); + + const listed = await manager.listSessions(); + const found = listed.find(s => s.id === session.id); + t.is(found?.title, 'My Own Name'); + t.true(found?.titleManuallySet); +}); diff --git a/source/session/session-manager.ts b/source/session/session-manager.ts index a5ca4c4a0..208d81a88 100644 --- a/source/session/session-manager.ts +++ b/source/session/session-manager.ts @@ -24,6 +24,9 @@ export interface Session { /** True once a user has explicitly renamed this session, so autosave's * auto-derived title (from the latest message) stops overwriting it. */ titleManuallySet?: boolean; + /** True once the background titler has named this session, so the + * heuristic title stops overwriting it and we never re-generate. */ + titleGenerated?: boolean; } export interface SessionMetadata { @@ -36,6 +39,7 @@ export interface SessionMetadata { model: string; workingDirectory: string; titleManuallySet?: boolean; + titleGenerated?: boolean; } function isRecord(obj: unknown): obj is Record { @@ -225,6 +229,7 @@ export class SessionManager { model: session.model, workingDirectory: session.workingDirectory, titleManuallySet: session.titleManuallySet, + titleGenerated: session.titleGenerated, }; if (existingSessionIndex >= 0) { @@ -287,6 +292,7 @@ export class SessionManager { model: parsed.model, workingDirectory: parsed.workingDirectory, titleManuallySet: parsed.titleManuallySet, + titleGenerated: parsed.titleGenerated, }); } } catch (_fileError) { diff --git a/source/session/title-client.spec.ts b/source/session/title-client.spec.ts new file mode 100644 index 000000000..b1a146bc6 --- /dev/null +++ b/source/session/title-client.spec.ts @@ -0,0 +1,118 @@ +import {mkdirSync, writeFileSync} from 'node:fs'; +import {tmpdir} from 'node:os'; +import {join} from 'node:path'; +import test from 'ava'; +import {clearAppConfig} from '@/config/index'; +import type {LLMClient} from '@/types/core'; +import {resetTitleClientCache, resolveTitleClient} from './title-client.js'; + +console.log('\ntitle-client.spec.ts'); + +// getAppConfig() lazily loads from disk, so without this the test would read +// the developer's real config and fail for anyone who has titleModel set. +const testConfigDir = join(tmpdir(), `nanocoder-title-client-${Date.now()}`); +mkdirSync(testConfigDir, {recursive: true}); +process.env.NANOCODER_CONFIG_DIR = testConfigDir; +process.chdir(testConfigDir); + +// Session config is read from nanocoder-preferences.json under a `nanocoder` +// key, not from agents.config.json. +function writeSessionConfig(sessions: Record): void { + writeFileSync( + join(testConfigDir, 'nanocoder-preferences.json'), + JSON.stringify({nanocoder: {sessions}}), + ); + clearAppConfig(); +} + +function fakeClient(label: string): LLMClient { + return { + getCurrentModel: () => label, + setModel: () => {}, + getContextSize: () => 8192, + getAvailableModels: async () => [label], + getProviderConfig: () => ({name: 'fake'}), + chat: async () => ({ + choices: [{message: {role: 'assistant', content: ''}}], + }), + clearContext: async () => {}, + getTimeout: () => undefined, + } as unknown as LLMClient; +} + +test.beforeEach(() => { + resetTitleClientCache(); + writeSessionConfig({}); +}); + +test('returns the session client when no override is configured', async t => { + const session = fakeClient('session-model'); + const resolved = await resolveTitleClient(session); + t.is(resolved, session); +}); + +test('returns the session client when sessions block exists but names no model', async t => { + writeSessionConfig({autoSave: true}); + const session = fakeClient('session-model'); + t.is(await resolveTitleClient(session), session); +}); + +test('falls back to the session client when the override cannot be built', async t => { + // The realistic failure: a user names a provider or model they do not have. + // The feature must degrade to the session model, not go silently dead. + writeSessionConfig({ + titleProvider: 'no-such-provider-exists', + titleModel: 'nope', + }); + const session = fakeClient('session-model'); + t.is(await resolveTitleClient(session), session); +}); + +// The tests below need a provider that actually constructs, so they write a +// real agents.config.json into the pinned config dir (also the cwd). +function writeProviders(providers: unknown[]): void { + writeFileSync( + join(testConfigDir, 'agents.config.json'), + JSON.stringify({nanocoder: {providers}}), + ); + clearAppConfig(); +} + +const alpha = { + name: 'alpha', + type: 'openai-compatible', + baseUrl: 'http://127.0.0.1:9/v1', + apiKey: 'x', + models: ['alpha-model'], +}; + +test('the cached client is rebuilt when the configured title model changes', async t => { + writeProviders([alpha]); + writeSessionConfig({titleProvider: 'alpha', titleModel: 'alpha-model'}); + + const session = fakeClient('session-model'); + const first = await resolveTitleClient(session); + t.not(first, session, 'a configured, constructible provider should be used'); + t.is(await resolveTitleClient(session), first, 'same config reuses the client'); + + writeSessionConfig({titleProvider: 'alpha', titleModel: 'a-different-model'}); + const rebuilt = await resolveTitleClient(session); + t.not(rebuilt, first, 'a config change must not serve the stale client'); +}); + +test('the cached client is rebuilt when the provider it names is edited', async t => { + // The case a provider/model cache key cannot see: both names are untouched, + // so the key built from them is byte-identical, but "alpha" now points at a + // different endpoint. Without the config generation in the key this serves a + // client aimed at the old baseURL for the rest of the process. + writeProviders([alpha]); + writeSessionConfig({titleProvider: 'alpha', titleModel: 'alpha-model'}); + + const session = fakeClient('session-model'); + const first = await resolveTitleClient(session); + t.not(first, session, 'a configured, constructible provider should be used'); + + writeProviders([{...alpha, baseUrl: 'http://127.0.0.1:10/v1'}]); + const rebuilt = await resolveTitleClient(session); + t.not(rebuilt, first, 'an edited provider must not serve the stale client'); +}); diff --git a/source/session/title-client.ts b/source/session/title-client.ts new file mode 100644 index 000000000..d315f53e2 --- /dev/null +++ b/source/session/title-client.ts @@ -0,0 +1,59 @@ +import {createLLMClient} from '@/client-factory'; +import {getAppConfig, getConfigGeneration} from '@/config/index'; +import type {LLMClient} from '@/types/core'; +import {getLogger} from '@/utils/logging'; + +/** Built at most once per config, and only when an override is configured. */ +let cachedClient: LLMClient | null = null; +/** + * The config generation plus the provider/model the cached client was built + * from. The names alone are not enough: editing a provider's baseURL or apiKey + * leaves `titleProvider` reading the same string while it no longer points at + * the same endpoint, so the generation is what makes any config edit rebuild. + */ +let cachedKey: string | null = null; +let warnedAboutFailure = false; + +/** Test seam. Production code never calls this. */ +export function resetTitleClientCache(): void { + cachedClient = null; + cachedKey = null; + warnedAboutFailure = false; +} + +/** + * Which client generates the title. Default is the session's own, so nothing + * extra is constructed and no new auth is needed. A user who wants to spend + * less can name a model in config; we never pick one for them. + */ +export async function resolveTitleClient( + sessionClient: LLMClient, +): Promise { + const sessions = getAppConfig().sessions; + const model = sessions?.titleModel; + const provider = sessions?.titleProvider; + + if (!model && !provider) return sessionClient; + + const key = `${getConfigGeneration()}\u0000${provider ?? ''}\u0000${model ?? ''}`; + if (cachedClient && cachedKey === key) return cachedClient; + + try { + const {client} = await createLLMClient(provider, model); + cachedClient = client; + cachedKey = key; + return client; + } catch (error) { + // Fall back rather than going quiet, so a typo in config does not look + // like a broken feature. Warn once per process, not once per session. + if (!warnedAboutFailure) { + warnedAboutFailure = true; + const named = [provider, model].filter(Boolean).join('/'); + getLogger().warn( + `Session title model "${named}" could not be used (${error}). ` + + 'Falling back to the session model.', + ); + } + return sessionClient; + } +} diff --git a/source/session/title-generator.spec.ts b/source/session/title-generator.spec.ts new file mode 100644 index 000000000..85db3e4f4 --- /dev/null +++ b/source/session/title-generator.spec.ts @@ -0,0 +1,393 @@ +import test from 'ava'; +import type {LLMClient} from '@/types/core'; +import { + buildTitleRequest, + deriveTitleFromFirstMessage, + extractUserMessages, + extractToolSummaries, + generateSessionTitle, + isWeakTitle, + normalizeFirstMessage, + sanitizeTitle, +} from './title-generator.js'; + +test('normalizeFirstMessage strips the active-file prefix', t => { + const input = '[Active file: source/app/App.tsx]\n\nfix the crash'; + t.is(normalizeFirstMessage(input), 'fix the crash'); +}); + +test('normalizeFirstMessage keeps only the first line', t => { + t.is(normalizeFirstMessage('first line\nsecond line'), 'first line'); +}); + +test('normalizeFirstMessage trims and collapses inner whitespace', t => { + t.is(normalizeFirstMessage(' fix the bug '), 'fix the bug'); +}); + +test('isWeakTitle table', t => { + t.true(isWeakTitle('hi')); + t.true(isWeakTitle('fix this')); + t.true(isWeakTitle('hello nanocoder')); + t.true(isWeakTitle('fix the auth bug')); + t.true(isWeakTitle('why is the login test failing')); + t.true(isWeakTitle('ログイン処理のバグを直して')); + t.false( + isWeakTitle('refactor session-manager to use atomic writes everywhere'), + ); +}); + +test('a substantive CJK prompt is not treated as weak', t => { + // 34 characters, but a flat 40-character threshold would still call it + // weak and spend a model call on it. CJK carries far more meaning per + // character, so those count double. + t.false( + isWeakTitle('セッションマネージャーを原子的書き込みに全面的にリファクタリングして'), + ); + // Still weak, and should be: this really is a one-line request. + t.true(isWeakTitle('ログイン処理のバグを直して')); +}); + +test('the active-file prefix is stripped with CRLF line endings too', t => { + t.is( + normalizeFirstMessage('[Active file: a.ts]\r\n\r\nfix the crash'), + 'fix the crash', + ); + t.is( + deriveTitleFromFirstMessage('[Active file: a.ts]\r\n\r\nfix the crash'), + 'fix the crash', + ); +}); + +test('isWeakTitle measures the normalized string, not the raw one', t => { + // Real content behind a prefix that would otherwise inflate the length. + const withPrefix = + '[Active file: a.ts]\n\nrefactor session-manager to use atomic writes everywhere'; + t.false(isWeakTitle(withPrefix)); + // A long prefix must not rescue a short message. + t.true(isWeakTitle('[Active file: some/very/long/path/to/a/file.ts]\n\nhi')); +}); + +test('extractToolSummaries pairs tool names with their path argument', t => { + const messages = [ + {role: 'user' as const, content: 'fix this'}, + { + role: 'assistant' as const, + content: '', + tool_calls: [ + { + id: '1', + function: { + name: 'read_file', + arguments: {path: 'source/auth/login.ts'}, + }, + }, + { + id: '2', + function: { + name: 'string_replace', + arguments: {file_path: 'source/auth/login.ts'}, + }, + }, + ], + }, + ]; + t.deepEqual(extractToolSummaries(messages), [ + 'read_file: source/auth/login.ts', + 'string_replace: source/auth/login.ts', + ]); +}); + +test('extractToolSummaries falls back to the bare name with no path arg', t => { + const messages = [ + { + role: 'assistant' as const, + content: '', + tool_calls: [ + {id: '1', function: {name: 'list_directory', arguments: {}}}, + ], + }, + ]; + t.deepEqual(extractToolSummaries(messages), ['list_directory']); +}); + +test('extractToolSummaries returns empty when no tools ran', t => { + const messages = [ + {role: 'user' as const, content: 'hi'}, + {role: 'assistant' as const, content: 'Hello.'}, + ]; + t.deepEqual(extractToolSummaries(messages), []); +}); + +test('extractToolSummaries caps at 10 entries', t => { + const messages = [ + { + role: 'assistant' as const, + content: '', + tool_calls: Array.from({length: 25}, (_v, i) => ({ + id: String(i), + function: {name: `tool_${i}`, arguments: {}}, + })), + }, + ]; + t.is(extractToolSummaries(messages).length, 10); +}); + +test('sanitizeTitle strips wrapping quotes', t => { + t.is(sanitizeTitle('"Fix Login Redirect"'), 'Fix Login Redirect'); + t.is(sanitizeTitle("'Fix Login Redirect'"), 'Fix Login Redirect'); +}); + +test('sanitizeTitle strips a leading Title: prefix', t => { + t.is(sanitizeTitle('Title: Fix Login Redirect'), 'Fix Login Redirect'); +}); + +test('sanitizeTitle strips markdown emphasis', t => { + t.is(sanitizeTitle('**Fix Login Redirect**'), 'Fix Login Redirect'); +}); + +test('sanitizeTitle strips trailing punctuation', t => { + t.is(sanitizeTitle('Fix Login Redirect.'), 'Fix Login Redirect'); +}); + +test('sanitizeTitle takes only the first non-empty line', t => { + t.is( + sanitizeTitle('\n\nFix Login Redirect\nsome rambling'), + 'Fix Login Redirect', + ); +}); + +test('sanitizeTitle rejects a paragraph rather than truncating it', t => { + const paragraph = + 'Sure! Here is a title that describes the session in detail, ' + + 'covering the authentication work and the various files that were ' + + 'touched during the course of this particular conversation session.'; + t.is(sanitizeTitle(paragraph), null); +}); + +test('sanitizeTitle returns null for empty or whitespace input', t => { + t.is(sanitizeTitle(''), null); + t.is(sanitizeTitle(' \n '), null); + t.is(sanitizeTitle('""'), null); +}); + +test('buildTitleRequest truncates each tool summary to 100 chars', t => { + const messages = buildTitleRequest({ + userMessages: ['fix this'], + toolSummaries: [`read_file: ${'y'.repeat(300)}`], + }); + const userContent = messages[messages.length - 1].content; + t.false(userContent.includes('y'.repeat(101))); +}); + +test('buildTitleRequest uses the assistant reply only when no tools ran', t => { + const withTools = buildTitleRequest({ + userMessages: ['fix this'], + toolSummaries: ['read_file: a.ts'], + assistantReply: 'I looked at the parser.', + }); + t.false( + withTools[withTools.length - 1].content.includes( + 'I looked at the parser.', + ), + ); + + const withoutTools = buildTitleRequest({ + userMessages: ['fix this'], + toolSummaries: [], + assistantReply: 'I looked at the parser.', + }); + t.true( + withoutTools[withoutTools.length - 1].content.includes( + 'I looked at the parser.', + ), + ); +}); + +test('buildTitleRequest truncates the assistant reply to 300 chars', t => { + const messages = buildTitleRequest({ + userMessages: ['fix this'], + toolSummaries: [], + assistantReply: 'z'.repeat(800), + }); + const userContent = messages[messages.length - 1].content; + t.false(userContent.includes('z'.repeat(301))); +}); + +test('buildTitleRequest opens with a system message', t => { + const messages = buildTitleRequest({ + userMessages: ['fix this'], + toolSummaries: [], + }); + t.is(messages[0].role, 'system'); + t.is(messages.length, 2); +}); + +function fakeClientReturning(content: string): LLMClient { + return { + getCurrentModel: () => 'fake', + setModel: () => {}, + getContextSize: () => 8192, + getAvailableModels: async () => ['fake'], + getProviderConfig: () => ({name: 'fake'}), + chat: async () => ({ + choices: [{message: {role: 'assistant', content}}], + }), + clearContext: async () => {}, + getTimeout: () => undefined, + } as unknown as LLMClient; +} + +const ctx = {userMessages: ['fix this'], toolSummaries: ['read_file: a.ts']}; + +test('generateSessionTitle returns a sanitized title', async t => { + const client = fakeClientReturning('"Fix Login Redirect Bug."'); + t.is(await generateSessionTitle(client, ctx), 'Fix Login Redirect Bug'); +}); + +test('generateSessionTitle returns null when the client throws', async t => { + const client = { + ...fakeClientReturning(''), + chat: async () => { + throw new Error('connection refused'); + }, + } as unknown as LLMClient; + t.is(await generateSessionTitle(client, ctx), null); +}); + +test('generateSessionTitle returns null on an empty response', async t => { + t.is(await generateSessionTitle(fakeClientReturning(''), ctx), null); + t.is(await generateSessionTitle(fakeClientReturning(' \n '), ctx), null); +}); + +test('generateSessionTitle returns null when the model writes a paragraph', async t => { + const paragraph = 'Certainly! '.repeat(30); + t.is(await generateSessionTitle(fakeClientReturning(paragraph), ctx), null); +}); + +test('generateSessionTitle returns null when there are no choices', async t => { + const client = { + ...fakeClientReturning(''), + chat: async () => ({choices: []}), + } as unknown as LLMClient; + t.is(await generateSessionTitle(client, ctx), null); +}); + +test('generateSessionTitle passes no tools to the client', async t => { + let seenTools: unknown; + const client = { + ...fakeClientReturning('Fix Login Bug'), + chat: async (_messages: unknown, tools: unknown) => { + seenTools = tools; + return { + choices: [{message: {role: 'assistant', content: 'Fix Login Bug'}}], + }; + }, + } as unknown as LLMClient; + await generateSessionTitle(client, ctx); + t.deepEqual(seenTools, {}); +}); + +test('generateSessionTitle forwards the abort signal', async t => { + let seenSignal: AbortSignal | undefined; + const client = { + ...fakeClientReturning('Fix Login Bug'), + chat: async ( + _messages: unknown, + _tools: unknown, + _callbacks: unknown, + signal?: AbortSignal, + ) => { + seenSignal = signal; + return { + choices: [{message: {role: 'assistant', content: 'Fix Login Bug'}}], + }; + }, + } as unknown as LLMClient; + const controller = new AbortController(); + await generateSessionTitle(client, ctx, controller.signal); + t.is(seenSignal, controller.signal); +}); + +test('buildTitleRequest includes the follow-up user turns, not just the first', t => { + const messages = buildTitleRequest({ + userMessages: ['fix this', 'now also update the tests'], + toolSummaries: [], + }); + const userContent = messages[messages.length - 1].content; + t.true(userContent.includes('fix this')); + t.true(userContent.includes('now also update the tests')); +}); + +test('buildTitleRequest caps the conversation at the first 3 user turns', t => { + const messages = buildTitleRequest({ + userMessages: ['one', 'two', 'three', 'four'], + toolSummaries: [], + }); + const userContent = messages[messages.length - 1].content; + t.true(userContent.includes('three')); + t.false(userContent.includes('four')); +}); + +test('buildTitleRequest truncates every user turn to 500 chars', t => { + const messages = buildTitleRequest({ + userMessages: ['a'.repeat(900), 'b'.repeat(900)], + toolSummaries: [], + }); + const userContent = messages[messages.length - 1].content; + t.true(userContent.includes('a'.repeat(500))); + t.false(userContent.includes('a'.repeat(501))); + t.true(userContent.includes('b'.repeat(500))); + t.false(userContent.includes('b'.repeat(501))); +}); + +test('extractUserMessages takes the first turns in order, skipping empties', t => { + t.deepEqual( + extractUserMessages([ + {role: 'user', content: 'fix this'}, + {role: 'assistant', content: 'ok'}, + {role: 'user', content: ' '}, + {role: 'user', content: 'now the tests'}, + {role: 'user', content: 'and the docs'}, + {role: 'user', content: 'and the changelog'}, + ]), + ['fix this', 'now the tests', 'and the docs'], + ); +}); + +test('extractUserMessages strips the active-file prefix VS Code injects', t => { + t.deepEqual( + extractUserMessages([ + {role: 'user', content: '[Active file: source/app/App.tsx]\n\nfix the crash'}, + ]), + ['fix the crash'], + ); +}); + +test('extractUserMessages keeps the whole body, not just the first line', t => { + t.deepEqual( + extractUserMessages([ + {role: 'user', content: 'add retries\n\nthe provider times out'}, + ]), + ['add retries\n\nthe provider times out'], + ); +}); + +test('deriveTitleFromFirstMessage strips the prefix and keeps the first line', t => { + t.is( + deriveTitleFromFirstMessage( + '[Active file: source/app/App.tsx]\n\nfix the crash\nmore detail', + ), + 'fix the crash', + ); +}); + +test('deriveTitleFromFirstMessage returns null when nothing usable is left', t => { + t.is(deriveTitleFromFirstMessage('\n\nreal content on line three'), null); + t.is(deriveTitleFromFirstMessage(' '), null); + t.is(deriveTitleFromFirstMessage('[Active file: a.ts]\n\n'), null); +}); + +test('deriveTitleFromFirstMessage caps the title length', t => { + const title = deriveTitleFromFirstMessage('x'.repeat(200)); + t.is(title, `${'x'.repeat(50)}...`); +}); diff --git a/source/session/title-generator.ts b/source/session/title-generator.ts new file mode 100644 index 000000000..f0407c424 --- /dev/null +++ b/source/session/title-generator.ts @@ -0,0 +1,220 @@ +import {MAX_SESSION_NAME_LENGTH} from '@/constants'; +import type {LLMClient, Message} from '@/types/core'; + +/** Below this many characters, a first message is too thin to be a title. */ +const WEAK_TITLE_THRESHOLD = 40; + +const MAX_USER_MESSAGE_CHARS = 500; + +/** How many opening user turns shape the title. */ +const MAX_USER_MESSAGES = 3; +const MAX_TOOL_SUMMARIES = 10; +const MAX_TOOL_SUMMARY_CHARS = 100; +const MAX_ASSISTANT_REPLY_CHARS = 300; + +const MAX_HEURISTIC_TITLE_CHARS = 50; + +/** + * Prepended by the VS Code UI, so it is plumbing rather than the request. + * Exported so the ACP save path strips it with the same pattern - two copies + * of this regex drifted apart once already. + */ +export const ACTIVE_FILE_PREFIX = /^\[Active file: [^\]]+\]\r?\n\r?\n/; + +/** + * Argument names that usually carry the thing a tool acted on, best first. + * `command` means bash command strings reach the title model. That is the + * session's own model by default; with `sessions.titleProvider` set it is + * whichever provider the user named. See the note on that config key. + */ +const PATH_ARG_KEYS = ['path', 'file_path', 'filePath', 'pattern', 'command']; + +export interface TitleContext { + /** The opening user turns, in order. The first one anchors the title. */ + userMessages: string[]; + /** e.g. ["read_file: source/auth/login.ts"] */ + toolSummaries: string[]; + /** Used only when toolSummaries is empty. */ + assistantReply?: string; +} + +/** + * Strip the active-file prefix the VS Code UI injects, keep the first line, + * and collapse whitespace. Everything downstream measures this, not the raw + * message, so a long file path can neither inflate nor rescue a short prompt. + */ +export function normalizeFirstMessage(content: string): string { + return content + .replace(ACTIVE_FILE_PREFIX, '') + .split('\n')[0] + .replace(/\s+/g, ' ') + .trim(); +} + +/** + * The plain title used until a generated one lands. Shared by the ACP save + * path and the CLI autosave, which write to the same store. Null, never '', + * so a message opening with a newline cannot persist a nameless session. + * An over-long first line keeps the trailing ellipsis, so a clipped title + * still reads as clipped in the history list. + */ +export function deriveTitleFromFirstMessage(content: string): string | null { + const firstLine = content + .replace(ACTIVE_FILE_PREFIX, '') + .split('\n')[0] + .trim(); + if (!firstLine) return null; + return firstLine.length > MAX_HEURISTIC_TITLE_CHARS + ? `${firstLine.slice(0, MAX_HEURISTIC_TITLE_CHARS)}...` + : firstLine; +} + +/** + * CJK scripts pack far more meaning into a character than Latin ones, so a + * flat 40-character threshold would mark almost every Chinese, Japanese or + * Korean prompt as weak and spend a call on it. Counting those characters + * double puts the two scripts on roughly the same footing. + */ +const CJK_CHARACTER = + /[\u3040-\u30ff\u3400-\u4dbf\u4e00-\u9fff\uf900-\ufaff\uac00-\ud7af]/g; + +function informationLength(text: string): number { + return text.length + (text.match(CJK_CHARACTER)?.length ?? 0); +} + +/** + * Length only, deliberately. An English stopword list would silently never + * fire for non-English users, and a false positive here costs one small call + * and yields an equal-or-better title. + */ +export function isWeakTitle(firstUserMessage: string): boolean { + return ( + informationLength(normalizeFirstMessage(firstUserMessage)) < + WEAK_TITLE_THRESHOLD + ); +} + +/** + * The opening user turns, in order, blanks dropped. Titling waits for a second + * turn or a tool call, so those later turns are usually where the actual task + * is stated - taking only the first would discard the context we waited for. + */ +export function extractUserMessages(messages: Message[]): string[] { + const turns: string[] = []; + + for (const message of messages) { + if (turns.length >= MAX_USER_MESSAGES) break; + if (message.role !== 'user') continue; + if (typeof message.content !== 'string') continue; + + const trimmed = message.content.replace(ACTIVE_FILE_PREFIX, '').trim(); + if (trimmed) turns.push(trimmed); + } + + return turns; +} + +/** Tool names plus what they acted on. This is what turns "fix this" into a title. */ +export function extractToolSummaries(messages: Message[]): string[] { + const summaries: string[] = []; + + for (const message of messages) { + for (const call of message.tool_calls ?? []) { + if (summaries.length >= MAX_TOOL_SUMMARIES) return summaries; + + const args = call.function.arguments ?? {}; + const key = PATH_ARG_KEYS.find(k => typeof args[k] === 'string'); + const detail = key ? String(args[key]) : ''; + + summaries.push( + detail ? `${call.function.name}: ${detail}` : call.function.name, + ); + } + } + + return summaries; +} + +/** + * Normalize whatever the model returned. Small local models routinely ignore + * "reply with only the title", so this is required, not defensive. Returns + * null when nothing usable is left, and null means we keep the existing title. + */ +export function sanitizeTitle(raw: string): string | null { + const firstLine = raw.split('\n').find(line => line.trim().length > 0); + if (!firstLine) return null; + + const cleaned = firstLine + .trim() + .replace(/^title\s*:\s*/i, '') + .replace(/^[*_`]+|[*_`]+$/g, '') + .replace(/^["'“”‘’]+|["'“”‘’]+$/g, '') + .replace(/[.!?,;:]+$/, '') + .replace(/\s+/g, ' ') + .trim(); + + if (!cleaned) return null; + // A model that wrote a paragraph must degrade to "no change", never to a + // paragraph in the sidebar. Truncating would hide the failure. + if (cleaned.length > MAX_SESSION_NAME_LENGTH) return null; + + return cleaned; +} + +/** All truncation happens here, before anything reaches a provider. */ +export function buildTitleRequest(ctx: TitleContext): Message[] { + const turns = ctx.userMessages + .slice(0, MAX_USER_MESSAGES) + .map(turn => turn.slice(0, MAX_USER_MESSAGE_CHARS)); + + const parts = [ + turns.length > 1 + ? `Conversation so far:\n${turns.map((t, i) => `${i + 1}. ${t}`).join('\n')}` + : `User request: ${turns[0] ?? ''}`, + ]; + + if (ctx.toolSummaries.length > 0) { + const tools = ctx.toolSummaries + .slice(0, MAX_TOOL_SUMMARIES) + .map(s => s.slice(0, MAX_TOOL_SUMMARY_CHARS)); + parts.push(`Actions taken:\n${tools.join('\n')}`); + } else if (ctx.assistantReply) { + parts.push( + `Assistant reply: ${ctx.assistantReply.slice(0, MAX_ASSISTANT_REPLY_CHARS)}`, + ); + } + + return [ + { + role: 'system', + content: + 'You name coding sessions. Summarise the whole exchange into ONLY a ' + + 'title of 3 to 6 words describing the task worked on. The first ' + + 'request is what the session is about; later ones add detail. ' + + 'No quotes, no trailing punctuation, no explanation, no preamble.', + }, + {role: 'user', content: parts.join('\n\n')}, + ]; +} + +/** + * The one impure export. Returns null on every failure path so a titling + * problem can never surface an error to the user or fail a turn. + * + * Tools are passed as {} so there is no tool-schema overhead on the request + * and no way for the call to turn into a tool loop. + */ +export async function generateSessionTitle( + client: LLMClient, + ctx: TitleContext, + signal?: AbortSignal, +): Promise { + try { + const response = await client.chat(buildTitleRequest(ctx), {}, {}, signal); + const content = response.choices[0]?.message?.content; + if (typeof content !== 'string') return null; + return sanitizeTitle(content); + } catch (_error) { + return null; + } +} diff --git a/source/types/config.ts b/source/types/config.ts index 18eca9614..f878aa028 100644 --- a/source/types/config.ts +++ b/source/types/config.ts @@ -382,6 +382,16 @@ export interface AppConfig { maxMessages?: number; retentionDays?: number; directory?: string; + /** Generate a title once per session. ACP clients only. Default true. */ + smartTitles?: boolean; + /** Title generation model. Defaults to the session's. */ + titleModel?: string; + /** + * Title generation provider. Defaults to the session's; a different one is + * sent the opening user turns and tool summaries, which include file paths + * and bash command strings. + */ + titleProvider?: string; }; // Headless / non-interactive conversation limits (--plain and ACP loops) @@ -604,6 +614,9 @@ export interface UserPreferences { maxMessages?: number; retentionDays?: number; directory?: string; + smartTitles?: boolean; + titleModel?: string; + titleProvider?: string; }; paste?: PasteConfig; };