feat:Auto AI generated titles in extension - #978
Conversation
Groundwork for replacing raw-prompt session names with generated titles. Five pure functions, no filesystem and no LLM client, so the logic most likely to need tuning is testable without any mocks. isWeakTitle uses a length threshold rather than a stopword list. A word list would be English-only and would silently never fire for other languages, leaving those sessions with the raw prompt forever. A false positive here just costs one small call and yields an equal-or-better title, so the rule fails cheaply in the direction it fails. sanitizeTitle returns null rather than truncating when a model ignores the instruction and writes a paragraph. Truncating would put a mangled sentence in the sidebar and hide the failure; null keeps the existing title instead. buildTitleRequest applies every truncation cap before anything reaches a provider, so the input size is bounded no matter how large the session grows.
Records that the background titler has already named a session, so the heuristic title stops overwriting it and generation never runs twice for the same session. The field has to be added to rebuildIndex as well as saveSession. Both hand-list every metadata field, so a field present in one and missing from the other is dropped with no error and no warning: a corrupt-index recovery would silently clear the flag and make every recovered session eligible for re-titling. The added test corrupts sessions.json to force that path, and a matching test now pins the same property for titleManuallySet, where the consequence would be a user's manual rename quietly losing its protection.
onRenameSession was wired straight to the setSessionName React setter, so a CLI rename lived only in memory and was gone on restart. Route it through a handler that also calls sessionManager.renameSession. The invisible half matters more than the visible one. renameSession is what sets titleManuallySet, the flag both the autosave heuristic and the upcoming background titler check before overwriting a title. Because the CLI could never set it, a CLI user had no way to protect a session name at all. The VS Code extension has always set it, so this closes a real gap between the two surfaces rather than adding a new behaviour. Also drops the now-inaccurate comment in useSessionAutosave that documented the CLI rename as never reaching disk.
Decides which client generates a session title. The default is the session's own already-constructed client, so nothing extra is built, no new auth is needed, and it works with Ollama or any OpenAI-compatible endpoint. No model name appears in this file. There is no fallback list and no per-provider table of known-cheap models, because either would break on custom endpoints and local setups where the model list is arbitrary. titleModel and titleProvider are unset by default and read purely from user config. When a configured override cannot be built, which realistically means a user named a model they do not have, it warns once per process and falls back to the session client rather than returning nothing. Going quiet would leave them with a feature that looks broken and says nothing. The spec pins its own config directory because getAppConfig reads from disk lazily, and without that the tests would load the developer's real config and fail for anyone who has titleModel set.
Single LLM round-trip, no tools passed, so there is no tool-schema overhead on the request and no way for it to turn into a tool loop. Every failure path returns null, which the caller reads as "keep the existing title". A client that throws, a request that aborts, an empty response, a response with no choices, and a model that ignores the instruction and writes a paragraph all degrade the same way. Titling is cosmetic and must never surface an error to the user or fail a turn. Tests cover each of those paths plus abort-signal forwarding, using a plain object satisfying LLMClient rather than a mocking library.
Owns every precondition, so neither call site holds policy: smartTitles enabled, no generation already in flight for this session, a first user message exists, at least one assistant message exists, the heuristic title is weak, and the stored session is neither manually renamed nor already generated. Three safety properties, each with a test. The session is re-read immediately before the write, because the user can rename while the model call is in flight and without that re-read the generator races the rename and wins. An in-flight set stops two turns finishing close together from both launching a call. The 20s timeout uses its own AbortController rather than the session's, since AcpSession.cancel() swaps that one out and borrowing it would attach the call to a stale controller. It writes with saveSession and never renameSession. renameSession sets titleManuallySet, which would mark an AI title as user-chosen and make the user's own rename structurally indistinguishable from a generated one. Also fixes the config loader. loadSessionConfig hand-builds its result from a whitelist, so smartTitles, titleModel and titleProvider were declared in AppConfig but silently discarded at load time: the off switch and the model override would both have done nothing, with no error. The keys are now read, and the specs write nanocoder-preferences.json, which is where session config actually lives rather than agents.config.json.
…test The title was taken from the most recent user message and reassigned on every autosave, so a session's name tracked whatever had just been typed rather than naming the session. Long sessions ended up labelled by their least representative message. Derive it from the first user message instead, first line only, matching what the ACP path has always done. That also removes a real divergence: both paths write the same sessions.json, so the two implementations were overwriting each other's titles with different values. Extends the overwrite guard to cover titleGenerated as well as titleManuallySet, then calls the background titler after each save. Without the guard change a generated title would be clobbered on the very next autosave, which is why the two land together. The call is fire and forget and reuses the existing saveChainRef serialisation, so it cannot race a save in flight and cannot delay or fail one.
Hooks the background titler into the existing turn-end finally block, fire and forget so the turn returns to idle immediately. On success it sends a _nanocoder/sessionTitleChanged ext notification so an editor client can refresh its session list without polling. Turn success is tracked in a local flag rather than inferred from reaching the finally. The cancel path added in Nano-Collective#789 returns from inside the catch, and the error path rethrows, and both still run the finally, so "we got here" says nothing about whether the turn completed. The session object carries no usable signal either: cancel() aborts its controller and immediately swaps in a fresh one. Three tests, and the two negative ones were verified as real pins by temporarily forcing the guard true and confirming they fail. Without that check they would have passed vacuously, since they also passed before any of this was wired up. Also carries titleGenerated forward on every save, alongside titleManuallySet, so a title survives the next turn's persist.
The agent now tells the extension when it renames a session in the background, and the extension refreshes its session list in response. The extension is a thin ACP client: it has no model client and no session store of its own, and gets its list from listSessions() over the wire. So the title has to be produced agent-side and pushed to it. The agent sends _nanocoder/sessionTitleChanged via notify(); the client receives it as extNotification(method, params) and re-fetches the whole list rather than patching one row, which avoids a second code path that can drift. Unknown ext methods are ignored so a newer CLI can send things this extension does not understand yet. acp-ext-notification.spec.ts pins the wire contract with a real round-trip over ndJsonStream, since that link is invisible to both sides' unit tests. It lives under source/acp/ because the AVA glob does not cover plugins/vscode/src/*.spec.ts. Also drops the CLI-side pieces this feature does not need: the TUI no longer generates titles of its own, and /rename keeps its previous in-memory behaviour. What stays in useSessionAutosave is the guard on titleGenerated - without it, opening the same project in the TUI would silently overwrite the title the agent just generated, since both write the same sessions.json.
Titling deliberately waits for a second user turn or a tool call, because a
first message short enough to trigger it ("fix this", "hi") does not say what
the session is about. buildTitleRequest then dropped everything except that
first message, so the turn we waited for never reached the model and the
title was generated from exactly the text we had already judged too thin.
TitleContext now carries userMessages: string[] instead of a single
firstUserMessage, filled by a new extractUserMessages() - the first three
user turns in order, blanks dropped, each truncated to the same 500 chars
the single message used to get. The precondition in maybe-generate-title
reads its count from that same helper rather than a separate filter, so one
function decides what counts as a turn.
The system prompt asks for a summary of the exchange and states that the
first request is what the session is about, so a passing remark in a later
turn adds detail without renaming the session after itself.
assets/nanocoder-vscode.vsix is a build artifact, and two rebuilds of a zip never merge - any PR that commits it conflicts with every other PR that does. Release CI already rebuilds it from source (release.yml), and the PR check only asserts the file exists, so carrying a rebuilt copy here buys nothing and costs a conflict on every sync. Restored to the blob main had at the branch point, so this branch's net change to it is zero.
The VS Code UI prepends "[Active file: <path>]" to the user's text, and both title paths read the raw message. The heuristic title ate its whole 50-character budget on the path before reaching the user's words, and the generated-title prompt opened with a file path that biased the model toward path-shaped names. Strip it in extractUserMessages, and move the heuristic into a shared deriveTitleFromFirstMessage() so the ACP save path and the CLI autosave cannot drift - they write to the same store. It returns null rather than '' so a message opening with a newline no longer persists a nameless session. Also guard the assistant-reply lookup with a typeof check, matching the first-user lookup above it, and catch the title notification's rejection: it fires after the turn goes idle, so a closed connection would surface as an unhandled rejection.
|
Hey @will-lamerton , @akramcodez , @Avtrkrb have a look here :-) |
|
@arpan7sarkar could you please resolve the conflicts so we can do the final review? Btw, great work! 🔥 |
Conflicts were in three files, resolved to keep both sides whole: - source/acp/acp-agent.ts: import conflict only. Kept upstream's TimelineManager alongside this branch's maybeGenerateTitle. - source/acp/acp-agent.spec.ts: both sides appended a new test block at the same point. Kept both - upstream's timeline/list and timeline/revert tests and this branch's background-titling tests. - source/hooks/useSessionAutosave.ts: the real conflict. Upstream taught deriveSessionTitle to skip approved-plan injections and internal walkthrough messages; this branch had removed the helper and derived the title from the first user message instead. Rebuilt the helper so it keeps upstream's two filters and its fallback, but scans forward and delegates to deriveTitleFromFirstMessage, so the CLI autosave and the ACP save path agree on the title they write to the shared store. The titleGenerated guard is carried alongside upstream's titleManuallySet. deriveTitleFromFirstMessage now re-appends the trailing ellipsis on a truncated title, preserving upstream's user-visible truncation marker now that both save paths route through it.
will-lamerton
left a comment
There was a problem hiding this comment.
Nice work - the layering is right (one impure function, everything else pure and directly tested), the re-read-before-write guard against a mid-flight rename is a real race most implementations miss, and acp-ext-notification.spec.ts verifying notify() -> extNotification over a real ndJSON stream is the one thing a stub could never catch.
I verified the branch locally: tsc --noEmit, knip, and biome check are all clean, and the touched specs pass.
A few things I'd like addressed before merge.
Blocking
1. A model that never returns a usable title re-calls on every turn, forever. titleGenerated is only set on success, so if generateSessionTitle returns null (paragraph response, connection refused, timeout) nothing is recorded and the next successful turn runs the whole thing again, indefinitely. Small local models are both the target audience and the population most likely to ignore "reply with only the title", so this is the expected path rather than the edge case. The result is a silent extra LLM call every turn for the life of the session. A per-process failure counter (give up after 2-3) or a persisted attempt count would cap it.
2. The inFlight guard has a gap it claims to close. inFlight.has() is checked at maybe-generate-title.ts:53 but add() is at :68, with await manager.readSession() in between, so two callers can both pass the check before either adds. Small window in practice, but the fix is free: move inFlight.add(sessionId) up to right after the synchronous guards (line 62) and extend the existing try/finally to cover it.
3. A chat() that ignores the abort signal wedges the session permanently. TITLE_TIMEOUT_MS relies entirely on the provider honouring signal. If one doesn't, the promise never settles, inFlight.delete never runs, and that session can never be titled again for the process lifetime. Promise.race([generateSessionTitle(...), timeout]) would make the 20s bound actually hold.
Worth answering
4. Titling tokens are invisible in usage tracking. buildResponseUsage is called from conversation-loop.tsx and acp-conversation.ts, not from inside client.chat, so these tokens are billed by the provider but never show up in /usage. The feature is on by default, so this deserves at least a line in the changeset.
5. smartTitles is a global config key but only the ACP path generates. maybeGenerateTitle has exactly one call site. CLI users get the documented config keys but no generated titles. Either wire it into the CLI turn completion or scope the naming and docs to the extension. (Minor: the comment at maybe-generate-title.spec.ts:902 says "Both call sites" - there is one.)
6. Test-coverage gap on the headline autosave fix. useSessionAutosave.spec.ts wasn't updated, and every existing deriveSessionTitle case has at most one real user message, so they pass identically under both the old backward scan and the new forward scan. Nothing pins the behaviour the PR fixed. A case with two substantive user turns asserting the first wins would close it.
Also worth calling out in the description: on next autosave, existing sessions get retitled from their first message. Expected, but it's a visible one-time churn in everyone's history list.
Smaller notes
PATH_ARG_KEYSincludescommand, so bash command strings reach the title model. Fine by default (same model as the session), but withtitleProviderset, paths, commands and up to 3 user turns go to a third party. One sentence in the config doc comment would cover it.ACTIVE_FILE_PREFIXdoesn't match\r\n, and the same regex is duplicated inline atacp-agent.ts:906. Worth having that call site use the exported constant so the two can't drift.WEAK_TITLE_THRESHOLD = 40characters is very high for CJK, where 40 chars is a long prompt. The comment acknowledges the tradeoff, but in practice nearly every CJK session will trigger a call.acp-ext-notification.spec.tsnever closes its streams or connections. It passes, but at.teardownclosing the writables would stop a future change leaving the AVA worker hanging.- The new
acp-agent.spec.tsnegative assertions use fixed 200ms sleeps against fire-and-forget work. Deterministic today, but the kind of thing that goes flaky on a loaded CI box. cachedClientintitle-client.tsis never invalidated on config change.resetTitleClientCachealready exists, so calling it fromclearAppConfigwould be cheap.- The
message-helpers.spec.tsrewrite is unrelated to this feature (it fixes a stale 4th argument left over from 6c1e5f3 and swapst.pass()for real prop assertions). Strict improvement, but it belongs in its own commit or a line in the description.
|
Thanks @will-lamerton for the review 🫡 , I will look into them all and finalize the pr let's see what new things I learn from here |
Description
Implements #808
Sessions in the History list are named after the opening prompt verbatim, so they end up called "hi" or "fix this". The agent now generates a descriptive title once per session and pushes it to the extension over ACP as a
_nanocoder/sessionTitleChangednotification, so the History list refreshes in place. It only fires when the opening prompt is too thin to be useful and there's real context to name, uses the session's own client by default (sessions.titleModel/titleProviderto override,sessions.smartTitles: falseto disable), and never overwrites a manual rename.While wiring this up, found the CLI's autosave derived its title from the latest user message and rewrote it on every save, into the same
sessions.jsonthe extension reads, so it would have stomped any generated title within 30 seconds. Fixed that, plusloadSessionConfigsilently dropping the new config keys.Screencast_20260827_131640.mp4
Type of Change
Changeset
pnpm changeset) describing this change for the changelogDocs-only or internal chores need no changeset (or run
pnpm changeset --emptyto note that intentionally).Testing
Automated Tests
Manual Testing
Checklist