From e6e63e6a13f1fcb8a9cd5d6f2afa36a3a2182c6f Mon Sep 17 00:00:00 2001 From: LinYushen Date: Thu, 9 Jul 2026 13:49:07 +0800 Subject: [PATCH] feat(chat): LLM-generated chat session titles with silent fallback (MUL-4295) (#5141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(chat): LLM-generated chat session titles with silent fallback (MUL-4295) Generate a concise, language-matched title for a chat session after the first user message, replacing the raw first-message-derived title. The work is best-effort and fully non-blocking: - Triggered on the first user message in SendChatMessage (detected via ChatSessionHasUserMessage before insert), run in a detached goroutine so it never delays the send or first response. - Reuses pkg/llm GenerateText on the configured default model (MULTICA_LLM_DEFAULT_MODEL, else gpt-4o-mini); no model from the client. - Self-hosted with no LLM key (h.LLM.Enabled()==false): silent no-op, the original title stands. Same on timeout / upstream error. - CAS write (UpdateChatSessionTitleIfCurrent) so a manual rename during generation is never clobbered and titling runs at most once. - Pushes chat:session_updated so the frontend refreshes in place. - sanitizeChatTitle strips quotes/brackets, 'Title:'/'标题：' prefixes, trailing punctuation, and caps at chatSessionTitleMaxLen. Tests cover all six cases: configured→semantic title, disabled→fallback, upstream error→fallback, manual rename→no clobber, empty output→fallback, idempotent second run, plus sanitize rules and the realtime push. Co-authored-by: multica-agent * fix(chat): panic-contain title goroutine + loop sanitizer to a fixed point (MUL-4295) Address PR #5141 review (张大彪 / multica-eve, Phase B): 1. The detached title-generation goroutine now has a defer recover() at the top of its body. It runs outside chi's Recoverer, so an unhandled panic in GenerateText / sanitize / the DB write / publish would crash the server process. Best-effort path: log and keep the original title. 2. sanitizeChatTitle now alternates prefix-stripping and wrapper-stripping in a loop until the string is stable, so a forbidden label hidden inside a wrapper ("Title: Fix login", 「标题：修复登录问题」) is fully cleaned regardless of nesting order. Added both cases to the sanitize test table. Co-authored-by: multica-agent * fix(chat): fold trailing-punctuation trim into sanitizer fixed-point loop (MUL-4295) Address PR #5141 follow-up review: the trailing-punctuation trim ran once AFTER the prefix/wrapper loop, so a trailing '.' / '。' left the closing wrapper unrecognized and the forbidden prefix untouched for inputs like "Title: Fix login". and 「标题：修复登录问题」。. Trailing trim now runs inside the same loop, so removing the trailing punctuation re-exposes the wrapper (and the prefix it hid) on the next pass. Added both cases to TestSanitizeChatTitle. Co-authored-by: multica-agent --------- Co-authored-by: multica-agent --- server/internal/handler/chat.go | 21 ++ server/internal/handler/chat_title.go | 259 ++++++++++++++ server/internal/handler/chat_title_test.go | 380 +++++++++++++++++++++ server/pkg/db/generated/chat.sql.go | 47 ++- server/pkg/db/queries/chat.sql | 13 + 5 files changed, 718 insertions(+), 2 deletions(-) create mode 100644 server/internal/handler/chat_title.go create mode 100644 server/internal/handler/chat_title_test.go diff --git a/server/internal/handler/chat.go b/server/internal/handler/chat.go index 079049ce2a..b2613157e8 100644 --- a/server/internal/handler/chat.go +++ b/server/internal/handler/chat.go @@ -580,6 +580,17 @@ func (h *Handler) SendChatMessage(w http.ResponseWriter, r *http.Request) { return } + // Detect whether this is the very first human message in the session, + // BEFORE we insert the new row. This scopes LLM auto-titling (MUL-4295) to + // the opening turn: we upgrade the default/original title exactly once, off + // the first user message, and never re-run it on every subsequent send. A + // query error here is treated as "not first" so we simply skip generation + // (best-effort — never block the send). + hadUserMessage := true + if existed, err := h.Queries.ChatSessionHasUserMessage(r.Context(), session.ID); err == nil { + hadUserMessage = existed + } + // Create the user message first so the daemon can always find it. msg, err := h.Queries.CreateChatMessage(r.Context(), db.CreateChatMessageParams{ ChatSessionID: session.ID, @@ -668,6 +679,16 @@ func (h *Handler) SendChatMessage(w http.ResponseWriter, r *http.Request) { CreatedAt: timestampToString(msg.CreatedAt), }) + // First user message → kick off best-effort LLM auto-titling (MUL-4295). + // Fire-and-forget and non-blocking: the response below is written whether + // or not a title is ever generated, and a disabled/failing LLM layer + // silently keeps the original first-message-derived title. session.Title + // is the default/original title observed here and drives the CAS so a + // manual rename mid-generation is never clobbered. + if !hadUserMessage { + h.maybeGenerateChatTitleAsync(workspaceID, userID, session.ID, session.Title, req.Content) + } + writeJSON(w, http.StatusCreated, SendChatMessageResponse{ MessageID: uuidToString(msg.ID), TaskID: uuidToString(task.ID), diff --git a/server/internal/handler/chat_title.go b/server/internal/handler/chat_title.go new file mode 100644 index 0000000000..939bf7c9c0 --- /dev/null +++ b/server/internal/handler/chat_title.go @@ -0,0 +1,259 @@ +package handler + +import ( + "context" + "errors" + "log/slog" + "strings" + "time" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + db "github.com/multica-ai/multica/server/pkg/db/generated" + "github.com/multica-ai/multica/server/pkg/protocol" +) + +// chatTitleGenTimeout bounds the whole best-effort title generation (LLM call +// + CAS write). It runs on a detached background context — decoupled from the +// originating HTTP request, which returns immediately — so this is the only +// thing keeping the goroutine from lingering if the upstream hangs. Kept short: +// a chat title is a nicety, not worth pinning a goroutine for a minute. +const chatTitleGenTimeout = 20 * time.Second + +// chatTitleSystemPrompt instructs the model to condense the opening of a +// conversation into a short, language-matched title. The rules mirror the +// acceptance criteria in MUL-4295: no quotes, no trailing punctuation, no +// "标题：" / "Title:" prefix, follow the conversation's language. sanitizeChatTitle +// re-applies these rules defensively in case the model ignores them. +const chatTitleSystemPrompt = `You write a very short title that summarizes the topic of a chat conversation, given the user's opening message. + +Rules: +- Output ONLY the title text — nothing else, no explanation. +- Keep it short: a few words, ideally under 8, never a full sentence. +- Write the title in the SAME language as the user's message (Chinese input → Chinese title, English input → English title). +- Do NOT wrap the title in quotes or brackets. +- Do NOT prefix it with "Title:", "标题：", or similar. +- Do NOT end with a period or any trailing punctuation.` + +// maybeGenerateChatTitleAsync kicks off best-effort LLM title generation for a +// chat session and returns immediately. It is the entry point wired into the +// first-user-message path of SendChatMessage. +// +// Design constraints from MUL-4295: +// - Non-blocking: never delays the user's send / first response. The work +// runs in a detached goroutine on context.Background() (the request context +// is cancelled the moment SendChatMessage returns). +// - Silent fallback: when the LLM layer is not configured (self-hosted with +// no key) or the call fails, we do nothing and leave the original +// first-message-derived title untouched — no error surfaces to the user. +// - No clobber: the update is a compare-and-swap against currentTitle, so a +// manual rename that lands during generation is never overwritten. +// +// currentTitle is the session's title as observed at trigger time (the +// default/original title). sourceText is the user's first message, which the +// model condenses into a title. +func (h *Handler) maybeGenerateChatTitleAsync(workspaceID, userID string, sessionID pgtype.UUID, currentTitle, sourceText string) { + // Short-circuit before spawning a goroutine when the LLM layer is disabled + // (self-hosted without MULTICA_LLM_API_KEY / MULTICA_LLM_BASE_URL): the + // original title is kept as-is, exactly matching pre-feature behavior. + if h.LLM == nil || !h.LLM.Enabled() { + return + } + if strings.TrimSpace(sourceText) == "" { + return + } + + go func() { + // Panic containment: this goroutine is detached from the HTTP request, + // so chi's Recoverer middleware is NOT in the call stack. A panic + // anywhere below (GenerateText, sanitizing, the DB write, publish) + // would otherwise crash the whole server process. This is a + // best-effort nicety — swallow the panic, log it, and leave the + // original title in place. + defer func() { + if rec := recover(); rec != nil { + slog.Error("chat title generation panicked; keeping original title", + "session_id", uuidToString(sessionID), + "panic", rec, + ) + } + }() + + ctx, cancel := context.WithTimeout(context.Background(), chatTitleGenTimeout) + defer cancel() + + updated, applied, err := h.generateChatSessionTitle(ctx, sessionID, currentTitle, sourceText) + if err != nil { + // Timeout, upstream 4xx/5xx, empty choices, etc. Log-and-forget: + // the send already succeeded and the original title stands. + slog.Warn("chat title generation failed; keeping original title", + "session_id", uuidToString(sessionID), + "error", err, + ) + return + } + if !applied { + // Either the model produced nothing usable, or the title changed + // under us (manual rename / already auto-titled). Nothing to push. + return + } + + // Reuse the existing chat:session_updated realtime channel so the + // frontend refreshes the title in place, identical to a manual rename. + resolvedSessionID := uuidToString(updated.ID) + h.publishChat(protocol.EventChatSessionUpdated, workspaceID, "member", userID, resolvedSessionID, protocol.ChatSessionUpdatedPayload{ + ChatSessionID: resolvedSessionID, + Title: updated.Title, + UpdatedAt: timestampToString(updated.UpdatedAt), + }) + }() +} + +// generateChatSessionTitle performs the synchronous core of title generation: +// call the LLM, sanitize the output, and compare-and-swap it onto the session. +// It is separated from the goroutine wrapper so it can be unit-tested directly. +// +// Return contract: +// - (session, true, nil): a new title was generated and written. +// - (zero, false, nil): generation produced nothing usable, OR the title +// had changed since currentTitle was observed (CAS miss / manual rename) — +// both are non-error "leave it alone" outcomes. +// - (zero, false, err): the LLM layer is disabled or the call failed, OR +// the CAS write hit a real DB error. Callers treat this as best-effort and +// keep the original title. +func (h *Handler) generateChatSessionTitle(ctx context.Context, sessionID pgtype.UUID, currentTitle, sourceText string) (db.ChatSession, bool, error) { + // DefaultModel() is used implicitly by GenerateText when model == "": a + // deployment configures MULTICA_LLM_DEFAULT_MODEL (or the built-in + // gpt-4o-mini fallback) — no model is threaded through from the frontend. + raw, err := h.LLM.GenerateText(ctx, "", chatTitleSystemPrompt, sourceText) + if err != nil { + return db.ChatSession{}, false, err + } + + title := sanitizeChatTitle(raw) + if title == "" { + // Model returned only quotes/punctuation/whitespace — treat as "no + // usable title" and fall back silently to the original. + return db.ChatSession{}, false, nil + } + + updated, err := h.Queries.UpdateChatSessionTitleIfCurrent(ctx, db.UpdateChatSessionTitleIfCurrentParams{ + ID: sessionID, + ExpectedTitle: currentTitle, + NewTitle: title, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + // Title changed since we observed currentTitle — a manual rename or + // a competing writer won. Do not clobber it. + return db.ChatSession{}, false, nil + } + return db.ChatSession{}, false, err + } + return updated, true, nil +} + +// chatTitleLabelPrefixes are leading labels a model sometimes prepends despite +// the prompt. Matched case-insensitively and only at the very start. +var chatTitleLabelPrefixes = []string{ + "title:", "title：", + "标题:", "标题：", + "题目:", "题目：", + "主题:", "主题：", +} + +// sanitizeChatTitle defensively enforces the title formatting rules regardless +// of how well the model followed the prompt: collapse whitespace, strip a +// leading "Title:" / "标题：" style label, remove surrounding quote/bracket +// pairs, drop trailing sentence punctuation, and hard-cap the length at +// chatSessionTitleMaxLen runes (the same ceiling the manual rename endpoint +// enforces). Returns "" when nothing meaningful remains. +// +// Prefix-stripping, wrapper-stripping, AND trailing-punctuation trimming all +// run in a single loop until the string stops changing. They interact: a model +// may nest them (e.g. `"Title: Fix login".` or `「标题：修复登录问题」。`), where a +// trailing `.` / `。` keeps the closing wrapper from being recognized, which in +// turn hides the forbidden prefix. Iterating all three to a fixed point peels +// them in any order. +func sanitizeChatTitle(raw string) string { + // Collapse all internal whitespace (including newlines/tabs the model may + // emit) into single spaces so a multi-line reply becomes one clean line. + s := strings.TrimSpace(strings.Join(strings.Fields(raw), " ")) + if s == "" { + return "" + } + + // Alternate stripping the leading label prefix, one layer of surrounding + // quotes/brackets, and trailing sentence punctuation until none of them + // changes anything. Bounded by the string only ever getting shorter, so it + // always terminates. + for { + before := s + s = strings.TrimSpace(stripChatTitleLabelPrefix(s)) + s = stripSurroundingQuotes(s) + // ASCII + common CJK/full-width sentence punctuation. Trailing trim is + // inside the loop so removing a trailing "." / "。" re-exposes a closing + // wrapper (and the prefix it hides) for the next pass. + s = strings.TrimSpace(strings.TrimRight(s, ".。!！?？,，;；:：、 ")) + if s == before || s == "" { + break + } + } + if s == "" { + return "" + } + + // Hard cap on rune length to match the manual-rename ceiling. + if runes := []rune(s); len(runes) > chatSessionTitleMaxLen { + s = strings.TrimSpace(string(runes[:chatSessionTitleMaxLen])) + } + return s +} + +// stripChatTitleLabelPrefix removes one leading label prefix ("Title:", +// "标题：", ...) when present, matched case-insensitively. Returns s unchanged +// when none matches. +func stripChatTitleLabelPrefix(s string) string { + lower := strings.ToLower(s) + for _, p := range chatTitleLabelPrefixes { + if strings.HasPrefix(lower, p) { + return s[len(p):] + } + } + return s +} + +// chatTitleQuotePairs maps an opening quote/bracket to its closing partner. +var chatTitleQuotePairs = map[rune]rune{ + '"': '"', + '\'': '\'', + '`': '`', + '“': '”', + '‘': '’', + '「': '」', + '『': '』', + '《': '》', + '（': '）', + '(': ')', + '【': '】', + '[': ']', +} + +// stripSurroundingQuotes removes matching opening/closing quote or bracket +// pairs that wrap the whole string, peeling repeatedly for nested wrappers. +func stripSurroundingQuotes(s string) string { + for { + runes := []rune(s) + if len(runes) < 2 { + return s + } + closer, ok := chatTitleQuotePairs[runes[0]] + if !ok || runes[len(runes)-1] != closer { + return s + } + s = strings.TrimSpace(string(runes[1 : len(runes)-1])) + if s == "" { + return s + } + } +} diff --git a/server/internal/handler/chat_title_test.go b/server/internal/handler/chat_title_test.go new file mode 100644 index 0000000000..789abe136d --- /dev/null +++ b/server/internal/handler/chat_title_test.go @@ -0,0 +1,380 @@ +package handler + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/multica-ai/multica/server/internal/events" + db "github.com/multica-ai/multica/server/pkg/db/generated" + "github.com/multica-ai/multica/server/pkg/llm" + "github.com/multica-ai/multica/server/pkg/protocol" +) + +// --------------------------------------------------------------------------- +// Test helpers for LLM chat auto-titling (MUL-4295) +// --------------------------------------------------------------------------- + +// stubLLMCompletion returns an httptest server that mimics the OpenAI +// chat-completions endpoint, replying with `content` as the assistant message. +// When status != 200 it returns that status (with an error-ish body) so callers +// can exercise the upstream-failure fallback. +func stubLLMCompletion(t *testing.T, status int, content string) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if status != http.StatusOK { + w.WriteHeader(status) + _, _ = io.WriteString(w, `{"error":{"message":"stub upstream error"}}`) + return + } + w.Header().Set("Content-Type", "application/json") + body := `{"id":"cmpl-1","object":"chat.completion","choices":[{"index":0,"message":{"role":"assistant","content":` + jsonString(content) + `},"finish_reason":"stop"}]}` + _, _ = io.WriteString(w, body) + })) + t.Cleanup(srv.Close) + return srv +} + +// jsonString escapes s into a JSON string literal (including surrounding +// quotes) so titles containing quotes/newlines embed cleanly in the stub body. +func jsonString(s string) string { + b := make([]byte, 0, len(s)+2) + b = append(b, '"') + for _, r := range s { + switch r { + case '"': + b = append(b, '\\', '"') + case '\\': + b = append(b, '\\', '\\') + case '\n': + b = append(b, '\\', 'n') + case '\t': + b = append(b, '\\', 't') + default: + b = append(b, string(r)...) + } + } + b = append(b, '"') + return string(b) +} + +// withStubLLM points testHandler.LLM at a client backed by srv for the duration +// of the test, restoring the original (disabled) client afterwards. +func withStubLLM(t *testing.T, srv *httptest.Server) { + t.Helper() + prev := testHandler.LLM + testHandler.LLM = llm.New(llm.Config{APIKey: "test-key", BaseURL: srv.URL}) + t.Cleanup(func() { testHandler.LLM = prev }) +} + +// chatTitleTestAgentID returns the seeded workspace test agent id. +func chatTitleTestAgentID(t *testing.T) pgtype.UUID { + t.Helper() + var agentID string + if err := testPool.QueryRow(context.Background(), + `SELECT id FROM agent WHERE workspace_id = $1 ORDER BY created_at ASC LIMIT 1`, + testWorkspaceID, + ).Scan(&agentID); err != nil { + t.Fatalf("load seeded agent: %v", err) + } + return parseUUID(agentID) +} + +// newChatTitleTestSession creates a chat session with the given (original) +// title and returns its row. Cleaned up via t.Cleanup. +func newChatTitleTestSession(t *testing.T, title string) db.ChatSession { + t.Helper() + session, err := testHandler.Queries.CreateChatSession(context.Background(), db.CreateChatSessionParams{ + WorkspaceID: parseUUID(testWorkspaceID), + AgentID: chatTitleTestAgentID(t), + CreatorID: parseUUID(testUserID), + Title: title, + }) + if err != nil { + t.Fatalf("create chat session: %v", err) + } + t.Cleanup(func() { + testPool.Exec(context.Background(), `DELETE FROM chat_session WHERE id = $1`, uuidToString(session.ID)) + }) + return session +} + +func chatSessionTitleFromDB(t *testing.T, sessionID pgtype.UUID) string { + t.Helper() + var title string + if err := testPool.QueryRow(context.Background(), + `SELECT title FROM chat_session WHERE id = $1`, uuidToString(sessionID), + ).Scan(&title); err != nil { + t.Fatalf("load session title: %v", err) + } + return title +} + +func requireDB(t *testing.T) { + t.Helper() + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } +} + +// --------------------------------------------------------------------------- +// Case 1: LLM configured → first-round title becomes a concise semantic title. +// --------------------------------------------------------------------------- + +func TestChatTitle_GeneratesSemanticTitleWhenConfigured(t *testing.T) { + requireDB(t) + withStubLLM(t, stubLLMCompletion(t, http.StatusOK, "修复登录跳转死循环")) + + original := "帮我看下为什么登录之后一直在几个页面之间来回跳转根本进不去首页" + session := newChatTitleTestSession(t, original) + + updated, applied, err := testHandler.generateChatSessionTitle(context.Background(), session.ID, session.Title, original) + if err != nil { + t.Fatalf("generateChatSessionTitle: unexpected error: %v", err) + } + if !applied { + t.Fatal("expected title to be applied") + } + if updated.Title != "修复登录跳转死循环" { + t.Fatalf("title = %q, want %q", updated.Title, "修复登录跳转死循环") + } + if got := chatSessionTitleFromDB(t, session.ID); got != "修复登录跳转死循环" { + t.Fatalf("DB title = %q, want %q", got, "修复登录跳转死循环") + } +} + +// --------------------------------------------------------------------------- +// Case 2: LLM disabled (self-hosted, no key) → silent fallback to original. +// --------------------------------------------------------------------------- + +func TestChatTitle_FallsBackWhenLLMDisabled(t *testing.T) { + requireDB(t) + // Force a disabled client regardless of ambient config. + prev := testHandler.LLM + testHandler.LLM = llm.New(llm.Config{}) + t.Cleanup(func() { testHandler.LLM = prev }) + + original := "please help debug my flaky test" + session := newChatTitleTestSession(t, original) + + _, applied, err := testHandler.generateChatSessionTitle(context.Background(), session.ID, session.Title, original) + if err != llm.ErrNotConfigured { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } + if applied { + t.Fatal("expected no title applied when LLM disabled") + } + if got := chatSessionTitleFromDB(t, session.ID); got != original { + t.Fatalf("DB title = %q, want original %q (must not change / blank)", got, original) + } + + // The async entry point must be a no-op (no panic, no change) when disabled. + testHandler.maybeGenerateChatTitleAsync(testWorkspaceID, testUserID, session.ID, session.Title, original) + time.Sleep(50 * time.Millisecond) + if got := chatSessionTitleFromDB(t, session.ID); got != original { + t.Fatalf("DB title changed after disabled async call: %q", got) + } +} + +// --------------------------------------------------------------------------- +// Case 3: LLM call fails (upstream 5xx / timeout) → silent fallback, no change. +// --------------------------------------------------------------------------- + +func TestChatTitle_SilentFallbackOnUpstreamError(t *testing.T) { + requireDB(t) + withStubLLM(t, stubLLMCompletion(t, http.StatusInternalServerError, "")) + + original := "why does my query return duplicate rows" + session := newChatTitleTestSession(t, original) + + _, applied, err := testHandler.generateChatSessionTitle(context.Background(), session.ID, session.Title, original) + if err == nil { + t.Fatal("expected an error from the failing upstream") + } + if applied { + t.Fatal("expected no title applied on upstream error") + } + if got := chatSessionTitleFromDB(t, session.ID); got != original { + t.Fatalf("DB title = %q, want unchanged original %q", got, original) + } +} + +// --------------------------------------------------------------------------- +// Case 4: user manually renamed the session → CAS miss, do not overwrite. +// --------------------------------------------------------------------------- + +func TestChatTitle_DoesNotClobberManualRename(t *testing.T) { + requireDB(t) + withStubLLM(t, stubLLMCompletion(t, http.StatusOK, "Generated Title")) + + original := "original auto title" + session := newChatTitleTestSession(t, original) + + // Simulate a manual rename landing before the async generator writes: the + // generator still holds the stale observed title (session.Title), but the + // DB now says something else. + manual := "My Renamed Chat" + if _, err := testHandler.Queries.UpdateChatSessionTitle(context.Background(), db.UpdateChatSessionTitleParams{ + ID: session.ID, + Title: manual, + }); err != nil { + t.Fatalf("simulate manual rename: %v", err) + } + + _, applied, err := testHandler.generateChatSessionTitle(context.Background(), session.ID, original, original) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if applied { + t.Fatal("expected CAS miss (applied=false) when title was manually renamed") + } + if got := chatSessionTitleFromDB(t, session.ID); got != manual { + t.Fatalf("DB title = %q, want manual rename %q preserved", got, manual) + } +} + +// --------------------------------------------------------------------------- +// Case 5: model returns empty / unusable output → fallback, no change. +// --------------------------------------------------------------------------- + +func TestChatTitle_FallsBackOnEmptyModelOutput(t *testing.T) { + requireDB(t) + // Model replies with only quotes + punctuation → sanitizes to "". + withStubLLM(t, stubLLMCompletion(t, http.StatusOK, `"。"`)) + + original := "some opening message" + session := newChatTitleTestSession(t, original) + + _, applied, err := testHandler.generateChatSessionTitle(context.Background(), session.ID, session.Title, original) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if applied { + t.Fatal("expected no title applied for empty/unusable model output") + } + if got := chatSessionTitleFromDB(t, session.ID); got != original { + t.Fatalf("DB title = %q, want unchanged original %q", got, original) + } +} + +// --------------------------------------------------------------------------- +// Case 6: auto-titling is idempotent — a second run does not re-title / clobber. +// --------------------------------------------------------------------------- + +func TestChatTitle_AutoTitlesOnlyOnce(t *testing.T) { + requireDB(t) + withStubLLM(t, stubLLMCompletion(t, http.StatusOK, "First Generated Title")) + + original := "original title text" + session := newChatTitleTestSession(t, original) + + // First generation applies against the observed original title. + _, applied, err := testHandler.generateChatSessionTitle(context.Background(), session.ID, original, original) + if err != nil || !applied { + t.Fatalf("first generation: applied=%v err=%v", applied, err) + } + if got := chatSessionTitleFromDB(t, session.ID); got != "First Generated Title" { + t.Fatalf("after first generation title = %q", got) + } + + // A second run still observing the ORIGINAL title (as the first-message + // trigger would) must be a no-op: the CAS no longer matches, so the + // already-generated title is left intact. + _, applied2, err2 := testHandler.generateChatSessionTitle(context.Background(), session.ID, original, original) + if err2 != nil { + t.Fatalf("second generation: unexpected error: %v", err2) + } + if applied2 { + t.Fatal("expected second generation to be a no-op (CAS miss)") + } + if got := chatSessionTitleFromDB(t, session.ID); got != "First Generated Title" { + t.Fatalf("second generation clobbered title: %q", got) + } +} + +// --------------------------------------------------------------------------- +// Async path: successful generation publishes chat:session_updated so the +// frontend refreshes the title in place (reuses the manual-rename channel). +// --------------------------------------------------------------------------- + +func TestChatTitle_AsyncPublishesSessionUpdated(t *testing.T) { + requireDB(t) + withStubLLM(t, stubLLMCompletion(t, http.StatusOK, "Async Semantic Title")) + + original := "async trigger original title" + session := newChatTitleTestSession(t, original) + + got := make(chan protocol.ChatSessionUpdatedPayload, 1) + testHandler.Bus.Subscribe(protocol.EventChatSessionUpdated, func(e events.Event) { + if p, ok := e.Payload.(protocol.ChatSessionUpdatedPayload); ok && p.ChatSessionID == uuidToString(session.ID) { + select { + case got <- p: + default: + } + } + }) + + testHandler.maybeGenerateChatTitleAsync(testWorkspaceID, testUserID, session.ID, session.Title, original) + + select { + case p := <-got: + if p.Title != "Async Semantic Title" { + t.Fatalf("event title = %q, want %q", p.Title, "Async Semantic Title") + } + case <-time.After(5 * time.Second): + t.Fatal("did not receive chat:session_updated event for generated title") + } + if dbTitle := chatSessionTitleFromDB(t, session.ID); dbTitle != "Async Semantic Title" { + t.Fatalf("DB title = %q, want %q", dbTitle, "Async Semantic Title") + } +} + +// --------------------------------------------------------------------------- +// sanitizeChatTitle unit tests: enforce the formatting rules regardless of how +// the model formats its reply (no quotes / no trailing punctuation / no label +// prefix / language-preserving / length cap). +// --------------------------------------------------------------------------- + +func TestSanitizeChatTitle(t *testing.T) { + longInput := "" + for i := 0; i < chatSessionTitleMaxLen+50; i++ { + longInput += "a" + } + + cases := []struct { + name string + in string + want string + }{ + {"plain", "Fix login bug", "Fix login bug"}, + {"surrounding double quotes", `"Fix login bug"`, "Fix login bug"}, + {"surrounding single quotes", `'Fix login bug'`, "Fix login bug"}, + {"smart quotes", "“修复登录问题”", "修复登录问题"}, + {"cjk brackets", "「优化查询性能」", "优化查询性能"}, + {"english label prefix", "Title: Fix login bug", "Fix login bug"}, + {"chinese label prefix", "标题：修复登录问题", "修复登录问题"}, + {"label then quotes", `标题："修复登录问题"`, "修复登录问题"}, + {"prefix wrapped in quotes", `"Title: Fix login"`, "Fix login"}, + {"prefix wrapped in cjk brackets", "「标题：修复登录问题」", "修复登录问题"}, + {"prefix in quotes with trailing period", `"Title: Fix login".`, "Fix login"}, + {"prefix in cjk brackets with trailing period", "「标题：修复登录问题」。", "修复登录问题"}, + {"trailing period", "Fix login bug.", "Fix login bug"}, + {"trailing cjk period", "修复登录问题。", "修复登录问题"}, + {"newlines collapsed", "Fix\nlogin\nbug", "Fix login bug"}, + {"leading trailing space", " Fix login bug ", "Fix login bug"}, + {"only punctuation empty", `"。"`, ""}, + {"blank", " ", ""}, + {"length cap", longInput, longInput[:chatSessionTitleMaxLen]}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := sanitizeChatTitle(tc.in); got != tc.want { + t.Fatalf("sanitizeChatTitle(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} diff --git a/server/pkg/db/generated/chat.sql.go b/server/pkg/db/generated/chat.sql.go index 3112fce485..c288f8d5d4 100644 --- a/server/pkg/db/generated/chat.sql.go +++ b/server/pkg/db/generated/chat.sql.go @@ -799,8 +799,9 @@ FOR UPDATE // their FK check after we commit the delete. func (q *Queries) LockChatSessionForDelete(ctx context.Context, id pgtype.UUID) (pgtype.UUID, error) { row := q.db.QueryRow(ctx, lockChatSessionForDelete, id) - err := row.Scan(&id) - return id, err + var id_2 pgtype.UUID + err := row.Scan(&id_2) + return id_2, err } const markChatSessionRead = `-- name: MarkChatSessionRead :exec @@ -965,3 +966,45 @@ func (q *Queries) UpdateChatSessionTitle(ctx context.Context, arg UpdateChatSess ) return i, err } + +const updateChatSessionTitleIfCurrent = `-- name: UpdateChatSessionTitleIfCurrent :one +UPDATE chat_session SET title = $1, updated_at = now() +WHERE id = $2 AND title = $3 +RETURNING id, workspace_id, agent_id, creator_id, title, session_id, work_dir, status, created_at, updated_at, runtime_id, last_read_at, is_agent_intro, pinned_at +` + +type UpdateChatSessionTitleIfCurrentParams struct { + NewTitle string `json:"new_title"` + ID pgtype.UUID `json:"id"` + ExpectedTitle string `json:"expected_title"` +} + +// Compare-and-swap the title: only overwrite it when it still equals the +// value the caller observed (@expected_title). This is the idempotency / +// no-clobber guard behind LLM auto-titling (MUL-4295): the async generator +// captures the session's current (default/original) title before calling the +// model, and this write lands only if a manual rename or a competing writer +// has not changed the title in the meantime. A mismatch returns pgx.ErrNoRows +// (zero rows updated), which the caller treats as "someone renamed it — leave +// it alone", NOT as an error. +func (q *Queries) UpdateChatSessionTitleIfCurrent(ctx context.Context, arg UpdateChatSessionTitleIfCurrentParams) (ChatSession, error) { + row := q.db.QueryRow(ctx, updateChatSessionTitleIfCurrent, arg.NewTitle, arg.ID, arg.ExpectedTitle) + var i ChatSession + err := row.Scan( + &i.ID, + &i.WorkspaceID, + &i.AgentID, + &i.CreatorID, + &i.Title, + &i.SessionID, + &i.WorkDir, + &i.Status, + &i.CreatedAt, + &i.UpdatedAt, + &i.RuntimeID, + &i.LastReadAt, + &i.IsAgentIntro, + &i.PinnedAt, + ) + return i, err +} diff --git a/server/pkg/db/queries/chat.sql b/server/pkg/db/queries/chat.sql index 5168c8beac..33903d2b5e 100644 --- a/server/pkg/db/queries/chat.sql +++ b/server/pkg/db/queries/chat.sql @@ -61,6 +61,19 @@ UPDATE chat_session SET title = $2, updated_at = now() WHERE id = $1 RETURNING *; +-- name: UpdateChatSessionTitleIfCurrent :one +-- Compare-and-swap the title: only overwrite it when it still equals the +-- value the caller observed (@expected_title). This is the idempotency / +-- no-clobber guard behind LLM auto-titling (MUL-4295): the async generator +-- captures the session's current (default/original) title before calling the +-- model, and this write lands only if a manual rename or a competing writer +-- has not changed the title in the meantime. A mismatch returns pgx.ErrNoRows +-- (zero rows updated), which the caller treats as "someone renamed it — leave +-- it alone", NOT as an error. +UPDATE chat_session SET title = @new_title, updated_at = now() +WHERE id = @id AND title = @expected_title +RETURNING *; + -- name: SetChatSessionPinned :one -- Pin/unpin a chat. Deliberately does NOT touch updated_at: pinning is a -- list-ordering preference, not activity, so it must not bump the session's