Skip to content

feat(agent-manager): auto-generate conversation titles from the first message - #122

Merged
Asaf-prog merged 6 commits into
mainfrom
feat/conversation-titling
Aug 28, 2026
Merged

feat(agent-manager): auto-generate conversation titles from the first message#122
Asaf-prog merged 6 commits into
mainfrom
feat/conversation-titling

Conversation

@AmitAvital1

@AmitAvital1 AmitAvital1 commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes #117. When the first user message is accepted in a new conversation, the manager writes a trimmed fallback immediately and generates a short title with the system's configured default LLM in independent background work.

Layering

  • agent_engine/engine/text_completion_engine.py defines TextCompletionEngine, an optional capability Protocol for one stateless model call outside the compiled graph (no nodes, tools, or prompt rendering), following the existing optional engine-capability pattern.
  • LangGraphEngine implements it. complete() accepts an optional BaseModelConfig, defaults to defaults.model, and uses the same shared model-construction path as graph nodes, including provider, region, temperature, top_p, and per-call max_tokens.
  • agent_manager/domain/titles.py defines the TitleGenerator port.
  • agent_manager/infrastructure/titles.py owns the prompt, timeout, normalization, and fallback. agent_manager does not import LangChain or construct models directly.
  • The composition root detects the optional capability once at startup. A system without defaults.model keeps the trimmed opening message as its title.

Delivery and lifecycle

  • The fallback is persisted before execution; model-based naming starts alongside the turn and is owned by ConversationService, not by the SSE request.
  • The request observes the title task through asyncio.shield, so disconnecting after the answer cannot cancel title generation or persistence.
  • The conversation stream emits a manager-level title event only after the title has settled and been persisted. If the generated-title write fails, no contradictory event is sent and the UI remains on the persisted fallback.
  • final and pending_approval remain terminal for the main UI execution: the widget releases the composer immediately, while continuing to consume the same response for a later title event. A slow title therefore does not block the next interaction or approval decision.
  • Request controllers remain tracked until their responses actually close, so component teardown still aborts every outstanding stream even after the main execution has been released.

Robustness

  • Title generation is triggered from the authoritative pre-append conversation head, so it runs exactly once; editing the first message does not generate another title.
  • Provider failure, timeout, unusable output, generator defects, and persistence failures remain secondary and cannot change a completed turn into a failed one.
  • Input and output are bounded. Output normalization handles matching quote pairs across scripts, punctuation outside quotes, empty/punctuation-only/emoji-only answers, and multi-line responses.
  • Titling does not add messages to conversation history or consume the conversation's token budget.

Deliberate: short opening messages skip the LLM

An opening message already short enough to be a title (≤48 characters after whitespace collapsing) is used as-is. This is intentional: a message such as Reset my password is already short and meaningful, and a model paraphrase would add cost without reliably improving it. Longer openings use the LLM.

Test plan

  • make format-check, make lint, make typecheck, and make generate-check
  • Full pytest suite: 934 passed; one unrelated Click stderr-capture test fails identically on a clean origin/main snapshot (test_agentctl_run_without_session_prints_reusable_generated_session)
  • npm run typecheck:widget, npm run typecheck:e2e, npm run build:widget, and npm run test:widget
  • Full Playwright suite: 39 passed
  • Regression coverage for slow title after final, a second turn before title delivery, UI thread update, request cancellation, persistence failure, secondary delivery failure, exact-once behavior after first-message edits, full model config, timeouts, normalization, budget/history isolation, and no-model fallback

Acceptance criteria

  • Title generated automatically for a new conversation from the first user message
  • Generated only once
  • Short and meaningful; LLM instructed to return only the title
  • Persisted as conversation metadata and exposed by the history API
  • UI shows the fallback while generation is pending
  • UI receives and displays the settled title without a page refresh
  • Slow titling does not keep the main execution active or block the next interaction
  • Disconnecting the SSE request does not cancel title persistence
  • The UI never receives a generated title that failed to persist
  • Failure is logged and degrades without failing the chat flow
  • Provider/model agnostic and isolated behind generic engine/domain ports
  • A future caller can supply a dedicated BaseModelConfig without changing the title feature
  • Backend and browser-level tests cover the feature and its lifecycle races

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks for the implementation — the overall direction looks good, but I found a few blockers before I can approve.

First, this branch needs to be rebased onto the current main. PR #111 has already been merged and reorganized the application layer, while this PR is still based on the previous structure. GitHub currently marks the PR as non-mergeable, so I think the rebase should happen before we review the final implementation details.

A few additional issues to address after the rebase:

  1. The UI does not currently refresh when the generated title becomes available.
    The backend updates the persisted title asynchronously, but the Threads UI only loads titles when the history panel is opened. If the panel is already open while title generation finishes, the user will keep seeing the fallback title until the thread list is manually reloaded.

    The issue explicitly requires the UI to reflect the generated title without a full-page refresh, so we need a synchronization/update mechanism here.

  2. The title model does not preserve the full model configuration.
    build_titler() currently forwards only provider, name, and the overridden max_tokens.

    We should also preserve relevant configuration such as region, temperature, and top_p.

    This is particularly important for Bedrock: a system may have a valid model.region configured in YAML, but the title model currently drops it and may fail during startup unless the region also happens to exist in the environment.

  3. Short first messages currently bypass the LLM entirely.
    If the opening message is already shorter than the title limit, it is used directly as the title and no title-generation call is made.

    That is a reasonable optimization, but it does not match the current acceptance criteria in Feature: Automatically Generate a Conversation Title from the First User Message #117, which describe title generation as a dedicated LLM task based on the first user message.

    We should either remove that shortcut and always generate the initial title through the configured title generator, or explicitly agree that this is the desired behavior and update the requirement accordingly.

After the branch is rebased and these points are addressed, I’d be happy to review it again.

AmitAvital1 added a commit that referenced this pull request Aug 22, 2026
Address review on #122:

- infrastructure/titles.py: build_titler() was forwarding only
  provider/name/max_tokens, dropping region/temperature/top_p. A Bedrock
  deployment with model.region set in YAML (not via env fallback) would
  crash the title model at startup while its agents worked fine. Forward
  the deployment's full model config; keep max_tokens as our own override,
  since a title is a handful of words regardless of what the agents are
  configured to answer with.
- widget: refresh the open Threads panel after every completed turn.
  Titling runs concurrently with the turn and is bounded to 24 tokens, so
  it's almost always done before the turn's own stream finishes — this
  reflects a generated title without a manual reopen, at the cost of one
  cheap GET, and without a dedicated push channel.
@AmitAvital1
AmitAvital1 force-pushed the feat/conversation-titling branch from 36d4a55 to 3283a90 Compare August 22, 2026 06:42
@AmitAvital1

Copy link
Copy Markdown
Collaborator Author

Thanks for the review — addressed 3 of the 4, and want to push back on the 4th with reasoning rather than just complying.

1. Rebase — done. Rebased onto current main (past #111's application-layer split into conversation_service.py / errors.py / prepared_conversation_turn.py, and the Engine capability protocols moving to their own modules). One conflict, in the import block; resolved, force-pushed.

2. Dropped region/temperature/top_p — fixed, and you were right about the Bedrock case specifically. build_titler() now forwards the deployment's full model config to build_chat_model(), not just provider/name. Found that #111 had already introduced the canonical pattern for this exact problem (_model_factory_kwargs in graph_builder.py) — didn't reuse it directly since it's module-private and built for a pluggable/introspected factory, which we don't need here (we always call the real build_chat_model), but mirrored its shape. max_tokens stays our own override — a title is a handful of words regardless of what the deployment's agents are configured to answer with, and that wasn't part of your ask. Added test_the_systems_full_model_config_reaches_the_factory and test_titlings_own_output_cap_overrides_the_deployments to cover both halves.

3. UI not refreshing while the panel stays open — fixed, without a push channel. Added refreshThreadsIfOpen() to the widget: after every completed turn, if the Threads panel is currently open, it silently re-pulls the list. Since titling runs concurrently with the turn and is capped at 24 output tokens, it's almost always done before the turn's own stream finishes, so this covers the common case you flagged without a dedicated SSE channel or backend change — we'd considered a push mechanism earlier and concluded the cost wasn't justified for what's normally a sub-second window. typecheck:widget / build:widget / test:widget all pass; widget.js regenerated and committed.

4. Short messages skipping the LLM — keeping this one, pushing back. None of the acceptance criteria actually say "must always call an LLM" — they say the title is automatically generated, based on the first message, short, and meaningful. A message like "אפס לי את הסיסמה" (16 chars) is already all three: it's a better title than anything a model would paraphrase it into, and it costs nothing. Removing the shortcut means burning a real LLM call on an estimated 30-40% of conversations for a strictly worse result in the common case, and better only in the (also common) case where the opening message legitimately needs summarizing — which the 48-char threshold already routes to the LLM. Would rather keep the threshold as documented, intentional behavior than "fix" it into paying for and sometimes downgrading titles that already work. Happy to tune the threshold if 48 chars feels wrong, but don't think the LLM should run unconditionally.

Ready for another pass.

@AmitAvital1

Copy link
Copy Markdown
Collaborator Author

Pushed a follow-up architectural change on top of the previous fixes, worth flagging explicitly since it changes the shape of the diff:

agent_manager was talking to a model directly for titling (build_chat_model + ainvoke) — the only place it did, everywhere else it depends on Engine for all message → response work. Moved that into agent_engine instead:

  • New optional capability TextCompletionEngine (Protocol), matching how RunStatusEngine/ApprovalEngine already expose optional runtime capabilities on top of Engine.
  • LangGraphEngine implements it via defaults.model — same region/temperature/top_p/provider construction path every agent's own model already goes through (reused model_factory_kwargs, made public for this). This actually supersedes the manual region/temperature/top_p forwarding fix from the last round — it's now handled by the same code path as agent models, not a hand-maintained list in agent_manager.
  • ConversationTitler now depends on TextCompletionEngine, not BaseChatModel. agent_manager no longer imports langchain anywhere.
  • build_titler/parse_model_ref/EXTRA_TITLE_MODEL are gone. The composition root asks the already-built engine whether it can complete text (defaults.model configured) and uses it if so — checked once at startup, no env var, no YAML change.

Net effect: agent_manager stays fully agnostic to how (or whether) a model gets called — it only knows it can ask the engine for a title and the engine may say no.

All gates green (make test: 907 passed, same one pre-existing unrelated CLI failure). Ready for another look whenever convenient.

@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — this is much closer now. The rebase/refactor alignment and model-config handling look good, and the CI is green.

I still see one blocking issue in the UI update flow:

The generated title is still produced in a background task, while the UI refreshes the thread list only once when the main turn finishes.

That creates a race like:

turn starts → title generation starts in background → main turn finishes → UI refreshes threads → fallback title is still stored → title generation finishes later → DB is updated → UI never refreshes again

So the current solution works only if title generation happens to finish before the main turn. The comment even says this is “usually” the case, but I think this needs to be deterministic.

We need a mechanism that guarantees the UI eventually receives the generated title after generation actually completes — for example an explicit update/event, polling while the title is pending, or another approach consistent with the existing architecture.

Two smaller points to align as well:

  • Short first messages still bypass the LLM entirely. If that optimization is intentional, I think the requirement/PR description should explicitly say so; otherwise it should follow the same title-generation flow.
  • The PR description still documents EXTRA_TITLE_MODEL, but the current implementation always uses the engine’s default model. Either wire the override back in or update the PR description to match the implemented behavior.

Once the UI race is fixed and these contract/details are aligned, I’m good with another pass.

@AmitAvital1

Copy link
Copy Markdown
Collaborator Author

All three addressed.

1. The UI race — you were right, fixed properly. My previous answer defended it with "titling almost always finishes first," which is a probability where you correctly asked for a guarantee. It also wasn't even reliably true: a fast turn against a slow provider loses the race, up to the generator's 5s timeout.

The turn's own SSE stream now carries the result, which turned out to have exact precedent — turn_started is already a manager-level event emitted by the route rather than the engine, so title follows the same shape:

  • PreparedConversationTurn carries the background naming task, so the handle dies with the turn rather than living on in service-level state.
  • ConversationService.wait_for_generated_title() resolves once naming actually completes, however late.
  • The stream route emits event: title after final; the widget applies it directly. refreshThreadsIfOpen is gone — no more guessing which finished first.

Cost is nil in practice: naming starts with the turn, so by the time the stream ends it has normally already resolved and the await returns immediately. Worst case is bounded by the timeout that already existed, and only on a conversation's first turn.

The event carries the settled title — generated, or the trim standing in for a failed generation — so a client relaying it never has to distinguish the two. Covered by test_the_generated_title_is_delivered_even_when_it_outlives_the_turn (releases the completer only after the turn has fully drained, i.e. exactly your race) plus SSE end-to-end tests. docs/WIDGET_ARCHITECTURE.md's event list is updated.

2. Short messages skipping the LLM — documented as intentional. Added a dedicated section to the PR description explaining the reasoning rather than leaving it implicit.

3. Stale EXTRA_TITLE_MODEL in the description — my miss, rewritten. The description was written before the engine refactor and still described build_titler/parse_model_ref/EXTRA_TITLE_MODEL, none of which exist. Fully rewritten to match what's implemented.

On the configurability criterion specifically: complete() takes an optional model: BaseModelConfig defaulting to defaults.model, so pointing titling at a different model is an engine-side wiring change that touches no titling code — the architectural requirement in #117 is satisfied without a config knob that would have to live somewhere awkward. Happy to add an explicit source for it (a YAML field feels more right than an env var, since model config is otherwise declarative) if you'd like that in this PR rather than later.

make test: 915 passed, same one pre-existing unrelated CLI failure. Widget typecheck/build/test clean.

… message

Generates a short title from a conversation's opening message using the
system's configured LLM (or an override via EXTRA_TITLE_MODEL), once per
conversation. The trimmed opening message is written synchronously as an
immediate fallback title; the LLM-generated title overwrites it from a
background task that never blocks or fails the turn it names.

- domain/titles.py: TitleGenerator port (ABC), matching the Repository/
  CallbackProvider convention.
- infrastructure/titles.py: ConversationTitler adapter built on
  build_chat_model, provider/model agnostic. Total over any model output —
  bounded, unquoted across scripts, and never empty.
- application/service.py: wires titling into the existing once-per-
  conversation hook; the call is isolated from the conversation's own
  history and token budget.
- agent_engine/logging_config.py: log() now threads exc_info through,
  fixing a pre-existing bug where warning tracebacks were silently dropped.

Closes #117
build_titler() forwarded only provider/name/max_tokens, dropping
region/temperature/top_p. A Bedrock deployment with model.region set in
YAML (not via env fallback) would crash the title model at startup while
its agents worked fine.

Forward the deployment's full model config; keep max_tokens as our own
override, since a title is a handful of words regardless of what the
agents are configured to answer with.
…gine

agent_manager talked to a model directly for titling — the only place it
ever did, everywhere else it depends on Engine for message -> response. Move
that capability into agent_engine instead, matching how RunStatusEngine and
ApprovalEngine already expose optional runtime capabilities:

- agent_engine/engine/text_completion_engine.py: TextCompletionEngine, an
  optional Protocol for one stateless model call outside the compiled
  graph — no nodes, no tools, no prompt rendering.
- LangGraphEngine implements it via defaults.model (region/temperature/
  top_p included, via the now-public model_factory_kwargs helper shared
  with node model construction) and the engine's own callbacks. Built
  fresh per call rather than cached, so a per-call max_tokens reaches the
  provider through its own constructor kwarg instead of relying on every
  provider integration honoring an invoke-time override.
- ConversationTitler now depends on TextCompletionEngine, not
  BaseChatModel — agent_manager no longer imports langchain anywhere.
  build_titler/parse_model_ref and EXTRA_TITLE_MODEL are gone: the
  composition root asks the already-built engine whether it can complete
  text and uses it if so, checked once at startup rather than per turn.

This also drops the region/temperature/top_p forwarding fixed in the
previous commit — the deployment's own model-construction path handles it
now, the same way it already does for every agent's model.
…ing field, make complete() take a model

Three fixes on top of the last commit's TextCompletionEngine:

- build_model(): GraphBuilder._build_model and Engine.complete() both did
  factory(provider, name, temperature, **kwargs) by hand. One function in
  graph_builder.py now, used by both; the kwargs-filtering helper it wraps
  goes back to being module-private since nothing outside build_model calls
  it directly anymore.
- Dropped RunnableConfig["tags"] from complete() — it had no precedent
  anywhere in the codebase, I added it without a reason. run_name stays
  (matches _thread_config's existing use for conversation turns) and is now
  commented as the external LangChain field it is.
- complete() takes an optional `model: BaseModelConfig | None`, defaulting
  to defaults.model rather than being hardwired to it. The capability was
  otherwise unusable for a future caller that needs a specific model rather
  than the system's default; the data structure to express that already
  exists, so the parameter was the missing piece, not new plumbing.
…acing it

Titling runs alongside the turn, but the widget refreshed the thread list
exactly once when the turn's stream closed. If the title landed after that —
a fast turn, a slow provider, anything up to the generator's 5s timeout — the
UI kept the fallback until the panel was reopened. The previous fix reasoned
that titling "almost always" finishes first, which is a probability, not the
guarantee this needs.

The turn's own SSE stream now carries the result, following the existing
precedent that manager-level events (`turn_started`) are emitted by the route
rather than the engine:

- PreparedConversationTurn carries the background naming task, so the handle
  dies with the turn instead of outliving it in service-level state, and
  whoever delivers the turn can also deliver its title.
- ConversationService.wait_for_generated_title() resolves once naming
  actually completes. Free for a live turn — naming starts with the turn, so
  it has usually finished by the time the stream ends — and bounded by the
  generator's own timeout otherwise.
- The stream route emits `event: title` after `final`; the widget applies it
  directly and no longer guesses by refreshing on turn completion.

The event carries the settled title, generated or the trim standing in for a
failed generation, so a client relaying it never has to tell the two apart.
@AmitAvital1
AmitAvital1 force-pushed the feat/conversation-titling branch from 0130b6b to 71f332e Compare August 23, 2026 19:10
@Asaf-prog

Copy link
Copy Markdown
Collaborator

Thanks — this is much closer now. The previous UI race is addressed properly with the explicit title SSE event, the model abstraction looks good, and CI is green.

I still see three issues I’d like to address before approving:

  1. Title generation can still delay completion of the main chat flow.

After the engine emits final, the route does:

title = await service.wait_for_generated_title(turn)

and only then allows the SSE stream to finish.

Since title generation can wait up to 5 seconds, the frontend can still consider the execution active even though the actual agent response has already completed. In the widget, finishExecution() only happens after the stream iterator finishes.

I think final should remain the terminal point for the main execution from the UI’s perspective. A slow title should not prevent the user from starting the next interaction.

  1. The SSE request lifecycle can cancel the background title task.

wait_for_generated_title() directly awaits turn.title_task.

If the client disconnects while the route is waiting for the title, cancellation of the request task can propagate to the title task as well. That means closing the browser/widget or losing the connection after the agent already finished could prevent the title from ever being persisted.

The title generation should be independent background application work. At minimum the await should be cancellation-safe (asyncio.shield or equivalent), though I think keeping its lifecycle independent from the SSE request would be cleaner.

  1. The emitted title can disagree with persisted state.

Currently _generate_title() does roughly:

await self._rename(conversation_id, title)
return title

but _rename() catches and logs persistence errors without propagating them.

So if rename_session() fails, the SSE can still send the generated title to the UI even though the DB still contains the fallback. The UI would then show one title until the next reload, when it changes back.

The title event should represent the title that was actually persisted/current, not merely the model output.

The deliberate short-message optimization is clear now in the PR description, so I don’t consider that a technical blocker anymore as long as that behavior is intentional.

Once these lifecycle/consistency issues are addressed, I think this will be in very good shape for approval.

@AmitAvital1

Copy link
Copy Markdown
Collaborator Author

Thanks — I verified all three points against the full request/application/UI lifecycle, and they were valid. Fixed in 57da8f8.

  1. A slow title no longer keeps the main execution active. The widget now treats both final and pending_approval as terminal for the main execution, clears the active-execution state immediately, and keeps consuming the response only for the later secondary title event. This also avoids delaying an approval decision. Request controllers remain tracked separately until their streams actually close, so unmount still aborts every outstanding response.

  2. Disconnect cancellation no longer propagates into titling. ConversationService owns every title task in its background-task set, and wait_for_generated_title() observes it through asyncio.shield(). A cancelled SSE request stops waiting but the application-owned task continues and persists the title. There is a regression test that cancels the waiter before releasing a deliberately slow completer, then verifies the task was not cancelled and the generated title was stored.

  3. The event now agrees with persistence. _rename() reports success/failure; _generate_title() returns a deliverable title only when that value is already the persisted fallback or its write succeeds. A failed generated-title write leaves the fallback in the repository and emits no title event. Secondary title-delivery defects are also isolated after the engine stream is terminal, so they cannot retroactively mark a completed turn failed.

The deeper pass found and fixed two adjacent correctness issues as well:

  • “first turn” was inferred from bounded prior context, which could retitle when editing the first message; it now uses the authoritative pre-append conversation head, so generation is truly once per conversation.
  • The widget contract docs contradicted actual fallback-event behavior; they now describe a settled, persisted title after the main terminal event.

Validation:

  • format, Ruff, mypy, and generated-stub check: clean
  • pytest: all relevant tests pass; the one remaining Click stderr-capture failure reproduces identically on a clean origin/main snapshot
  • widget typecheck/build/unit self-check: clean
  • Playwright: 39 passed, including a new race test where turn 1 reaches final, turn 2 completes while turn 1 is still waiting for its title, and the open Threads drawer then updates from the delayed title event

I also rewrote the PR description so the documented lifecycle and test plan match the implementation now.

@AmitAvital1
AmitAvital1 requested a review from Asaf-prog August 28, 2026 10:47
@Asaf-prog
Asaf-prog merged commit 5f7257b into main Aug 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Automatically Generate a Conversation Title from the First User Message

2 participants