From 915956460afebaf80c617908661c28e294daa8af Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Mon, 11 May 2026 17:09:42 +0200 Subject: [PATCH] feat(coderd/x/chatd): add compact turn status labels (#25043) > Mux is acting on Mike's behalf. Changes chat turn-end summaries into compact status labels for the cached `last_turn_summary` and successful web push body. Uses a structured-output model call for successful turns, requiring a 2-5 word `label` and validating it to reject agent-centric phrasing. Pending and requires-action states keep deterministic status labels. Removes the earlier deterministic tool-signal pipeline in favor of the smaller structured-output path. --- coderd/x/chatd/chatd.go | 144 ++++++++------ coderd/x/chatd/chatd_test.go | 58 +++--- coderd/x/chatd/quickgen.go | 187 +++++++++++++++---- coderd/x/chatd/quickgen_test.go | 111 +++++++++-- coderd/x/chatd/turn_summary_internal_test.go | 11 +- 5 files changed, 377 insertions(+), 134 deletions(-) diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 32837ba6f1..e817e76f5a 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -71,7 +71,7 @@ const ( // cold-start agent's first MCP reload can settle before // chatd gives up. workspaceMCPDiscoveryTimeout = 35 * time.Second - turnSummaryWriteTimeout = 5 * time.Second + turnStatusLabelWriteTimeout = 5 * time.Second // defaultDialTimeout matches the timeout used by ~8 other // server-side AgentConn callers. defaultDialTimeout = 30 * time.Second @@ -5821,7 +5821,7 @@ func (p *Server) processChat(ctx context.Context, chat database.Chat) { if lastErrorPayload != nil { lastErrorMessage = lastErrorPayload.Message } - p.maybeFinalizeTurnSummaryAndPush( + p.maybeFinalizeTurnStatusLabelAndPush( cleanupCtx, finishResult.updatedChat, status, @@ -5938,7 +5938,7 @@ func (t *generatedChatTitle) Load() (string, bool) { type runChatResult struct { FinalAssistantText string - PushSummaryModel fantasy.LanguageModel + StatusLabelModel fantasy.LanguageModel ProviderKeys chatprovider.ProviderAPIKeys PendingDynamicToolCalls []chatloop.PendingToolCall FallbackProvider string @@ -6523,7 +6523,7 @@ func (p *Server) runChat( } chainInfo := chatopenai.ResolveChainMode(messages) - result.PushSummaryModel = model + result.StatusLabelModel = model result.ProviderKeys = providerKeys result.FallbackProvider = modelConfig.Provider result.FallbackModel = modelConfig.Model @@ -6534,7 +6534,7 @@ func (p *Server) runChat( // Snapshot model, logger, and ctx before launch; all three get // reassigned below (model = cuModel, logger = logger.With(...), // ctx = runCtx) and the goroutine captures by reference. - titleModel := result.PushSummaryModel + titleModel := model titleLogger := logger titleCtx := context.WithoutCancel(ctx) p.inflight.Add(1) @@ -8631,9 +8631,9 @@ func parseDynamicToolNames(raw pqtype.NullRawMessage) (map[string]bool, error) { return names, nil } -// maybeFinalizeTurnSummaryAndPush updates the cached turn summary for -// parent chats and optionally sends a web push notification. -func (p *Server) maybeFinalizeTurnSummaryAndPush( +// maybeFinalizeTurnStatusLabelAndPush updates the cached turn status label +// for parent chats and optionally sends a web push notification. +func (p *Server) maybeFinalizeTurnStatusLabelAndPush( ctx context.Context, chat database.Chat, status database.ChatStatus, @@ -8647,15 +8647,15 @@ func (p *Server) maybeFinalizeTurnSummaryAndPush( switch status { case database.ChatStatusWaiting: - p.finalizeSuccessfulTurnSummaryAndPush(ctx, chat, runResult, logger) + p.finalizeSuccessfulTurnStatusLabelAndPush(ctx, chat, status, runResult, logger) case database.ChatStatusPending: - p.finalizeSuccessfulTurnSummary(ctx, chat, runResult, logger) + p.setLastTurnSummaryAsync(ctx, chat, fallbackTurnStatusLabel(status), logger) case database.ChatStatusError: p.clearLastTurnSummaryAsync(ctx, chat, logger) if p.webpushConfigured() { - pushBody := "Agent encountered an error." + pushBody := fallbackTurnStatusLabel(status) if lastError != "" { pushBody = lastError } @@ -8663,87 +8663,101 @@ func (p *Server) maybeFinalizeTurnSummaryAndPush( } case database.ChatStatusRequiresAction: - p.clearLastTurnSummaryAsync(ctx, chat, logger) + p.setLastTurnSummaryAsync(ctx, chat, fallbackTurnStatusLabel(status), logger) default: // New statuses must be classified before they can safely - // preserve or finalize a cached turn summary. + // preserve or finalize a cached turn status label. p.clearLastTurnSummaryAsync(ctx, chat, logger) } } -func (p *Server) finalizeSuccessfulTurnSummary( +func (p *Server) finalizeSuccessfulTurnStatusLabelAndPush( ctx context.Context, chat database.Chat, + status database.ChatStatus, runResult runChatResult, logger slog.Logger, ) { - p.finalizeSuccessfulTurnSummaryWithAfterFunc(ctx, chat, runResult, logger, func(context.Context, string) {}) -} - -func (p *Server) finalizeSuccessfulTurnSummaryAndPush( - ctx context.Context, - chat database.Chat, - runResult runChatResult, - logger slog.Logger, -) { - p.finalizeSuccessfulTurnSummaryWithAfterFunc(ctx, chat, runResult, logger, func(finalizeCtx context.Context, summary string) { - p.dispatchSuccessfulTurnPush(finalizeCtx, chat, summary, logger) + p.finalizeSuccessfulTurnStatusLabelWithAfterFunc(ctx, chat, status, runResult, logger, func(finalizeCtx context.Context, statusLabel string) { + p.dispatchSuccessfulTurnPush(finalizeCtx, chat, statusLabel, logger) }) } -func (p *Server) finalizeSuccessfulTurnSummaryWithAfterFunc( +func (p *Server) finalizeSuccessfulTurnStatusLabelWithAfterFunc( ctx context.Context, chat database.Chat, + status database.ChatStatus, runResult runChatResult, logger slog.Logger, afterFinalize func(context.Context, string), ) { - debugSvc := p.existingDebugService() // This helper runs during processChat cleanup, while processChat is // still counted in p.inflight. Do not take inflightMu here because // drainInflight holds it while waiting. p.inflight.Go(func() { finalizeCtx := context.WithoutCancel(ctx) - summary := "" - assistantText := strings.TrimSpace(runResult.FinalAssistantText) - if assistantText != "" && runResult.PushSummaryModel != nil { - summary = strings.TrimSpace(generatePushSummary( - finalizeCtx, - chat, - assistantText, - runResult.FallbackProvider, - runResult.FallbackModel, - runResult.PushSummaryModel, - runResult.ProviderKeys, - logger, - debugSvc, - runResult.TriggerMessageID, - runResult.HistoryTipMessageID, - )) - } + statusLabel := p.generateFinalTurnStatusLabel(finalizeCtx, chat, status, runResult, logger) + logger.Debug(finalizeCtx, "generated chat turn status label", + slog.F("chat_id", chat.ID), + slog.F("status", status), + slog.F("label_length", len(statusLabel)), + ) - shouldPersistSummary := summary != "" || chat.LastTurnSummary.Valid - if shouldPersistSummary { - p.updateLastTurnSummary(finalizeCtx, chat, chat.UpdatedAt, summary, logger) - } + p.updateLastTurnSummary(finalizeCtx, chat, chat.UpdatedAt, statusLabel, logger) - afterFinalize(finalizeCtx, summary) + afterFinalize(finalizeCtx, statusLabel) }) } +func (p *Server) generateFinalTurnStatusLabel( + ctx context.Context, + chat database.Chat, + status database.ChatStatus, + runResult runChatResult, + logger slog.Logger, +) string { + if status != database.ChatStatusWaiting { + return fallbackTurnStatusLabel(status) + } + + assistantText := strings.TrimSpace(runResult.FinalAssistantText) + if assistantText == "" || runResult.StatusLabelModel == nil { + return fallbackTurnStatusLabel(status) + } + + statusLabel := generateTurnStatusLabel( + ctx, + chat, + status, + assistantText, + runResult.FallbackProvider, + runResult.FallbackModel, + runResult.StatusLabelModel, + runResult.ProviderKeys, + logger, + p.existingDebugService(), + runResult.TriggerMessageID, + runResult.HistoryTipMessageID, + ) + if statusLabel == "" { + return fallbackTurnStatusLabel(status) + } + return statusLabel +} + func (p *Server) dispatchSuccessfulTurnPush( ctx context.Context, chat database.Chat, - summary string, + statusLabel string, logger slog.Logger, ) { if !p.webpushConfigured() { return } - pushBody := "Agent has finished running." - if summary != "" { - pushBody = summary + pushBody := fallbackTurnStatusLabel(database.ChatStatusWaiting) + if statusLabel != "" { + pushBody = statusLabel } p.dispatchPush(ctx, chat, pushBody, database.ChatStatusWaiting, logger) } @@ -8759,6 +8773,28 @@ func (p *Server) maybeClearLastTurnSummaryAsync( p.clearLastTurnSummaryAsync(ctx, chat, logger) } +func (p *Server) setLastTurnSummaryAsync( + ctx context.Context, + chat database.Chat, + summary string, + logger slog.Logger, +) { + summary = strings.TrimSpace(summary) + if summary == "" { + p.clearLastTurnSummaryAsync(ctx, chat, logger) + return + } + if chat.LastTurnSummary.Valid && strings.TrimSpace(chat.LastTurnSummary.String) == summary { + return + } + // This helper runs during processChat cleanup, while processChat is + // still counted in p.inflight. Do not take inflightMu here because + // drainInflight holds it while waiting. + p.inflight.Go(func() { + p.updateLastTurnSummary(context.WithoutCancel(ctx), chat, chat.UpdatedAt, summary, logger) + }) +} + func (p *Server) clearLastTurnSummaryAsync( ctx context.Context, chat database.Chat, @@ -8790,7 +8826,7 @@ func (p *Server) updateLastTurnSummary( //nolint:gocritic // Narrow daemon access for best-effort summary cache writes. updateCtx := dbauthz.AsChatd(ctx) - updateCtx, cancel := context.WithTimeout(updateCtx, turnSummaryWriteTimeout) + updateCtx, cancel := context.WithTimeout(updateCtx, turnStatusLabelWriteTimeout) defer cancel() affected, err := p.db.UpdateChatLastTurnSummary(updateCtx, database.UpdateChatLastTurnSummaryParams{ diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index 2cd3c9cc74..413ea148d0 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -4057,7 +4057,7 @@ func TestPersistToolResultWithBinaryData(t *testing.T) { require.True(t, foundToolResultInSecondCall, "expected second streamed model call to include execute tool output") } -func TestRequiresActionChatClearsLastTurnSummary(t *testing.T) { +func TestRequiresActionChatPersistsWaitingStatusLabel(t *testing.T) { t.Parallel() db, ps := dbtestutil.NewDB(t) @@ -4108,7 +4108,7 @@ func TestRequiresActionChatClearsLastTurnSummary(t *testing.T) { chat, err := server.CreateChat(ctx, chatd.CreateOptions{ OrganizationID: org.ID, OwnerID: user.ID, - Title: "requires-action-summary-clear", + Title: "requires-action-status-label", ModelConfigID: model.ID, InitialUserContent: []codersdk.ChatMessagePart{ codersdk.ChatMessageText("Please call the dynamic tool."), @@ -4131,15 +4131,16 @@ func TestRequiresActionChatClearsLastTurnSummary(t *testing.T) { return true } return got.Status == database.ChatStatusRequiresAction && - !got.LastTurnSummary.Valid + got.LastTurnSummary.Valid && + got.LastTurnSummary.String == "Waiting for user input" }, testutil.IntervalFast) chatd.WaitUntilIdleForTest(server) require.Equal(t, database.ChatStatusRequiresAction, fromDB.Status, "expected requires_action, got %s (last_error=%q)", fromDB.Status, string(fromDB.LastError.RawMessage)) - require.False(t, fromDB.LastTurnSummary.Valid, - "requires action chats should clear cached turn summaries") + require.Equal(t, sql.NullString{String: "Waiting for user input", Valid: true}, fromDB.LastTurnSummary, + "requires action chats should persist a waiting status label") require.Equal(t, int32(0), mockPush.dispatchCount.Load(), "expected no web push dispatch for a requires_action chat") } @@ -6525,13 +6526,16 @@ func TestSuccessfulChatSendsWebPushWithSummary(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) const assistantText = "I have completed the task successfully and all tests are passing now." - const summaryText = "Completed task and verified all tests pass." + const summaryText = "Finished unit tests" var nonStreamingRequests atomic.Int32 openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse { if !req.Stream { - nonStreamingRequests.Add(1) - return chattest.OpenAINonStreamingResponse(summaryText) + if strings.Contains(string(req.RawBody), "propose_turn_status_label") { + nonStreamingRequests.Add(1) + return chattest.OpenAINonStreamingResponse(fmt.Sprintf(`{"label":%q}`, summaryText)) + } + return chattest.OpenAINonStreamingResponse(`{"title":"Summary push test"}`) } return chattest.OpenAIStreamingResponse( chattest.OpenAITextChunks(assistantText)..., @@ -6579,13 +6583,11 @@ func TestSuccessfulChatSendsWebPushWithSummary(t *testing.T) { msg := mockPush.getLastMessage() require.Equal(t, summaryText, fromDB.LastTurnSummary.String, - "last turn summary should be the LLM-generated summary") + "last turn summary should be the LLM-generated status label") require.Equal(t, fromDB.LastTurnSummary.String, msg.Body, - "push body should reuse the persisted generated summary") - require.NotEqual(t, "Agent has finished running.", msg.Body, - "push body should not use the default fallback text") + "push body should reuse the persisted generated status label") require.Equal(t, int32(1), nonStreamingRequests.Load(), - "expected exactly one non-streaming request for push summary generation") + "expected exactly one non-streaming request for status label generation") } func TestSuccessfulChatPersistsTurnSummaryWithoutWebPush(t *testing.T) { @@ -6595,13 +6597,16 @@ func TestSuccessfulChatPersistsTurnSummaryWithoutWebPush(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) const assistantText = "I fixed the bug and added regression coverage." - const summaryText = "Fixed the bug and added regression coverage." + const summaryText = "Fixed regression bug" var nonStreamingRequests atomic.Int32 openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse { if !req.Stream { - nonStreamingRequests.Add(1) - return chattest.OpenAINonStreamingResponse(summaryText) + if strings.Contains(string(req.RawBody), "propose_turn_status_label") { + nonStreamingRequests.Add(1) + return chattest.OpenAINonStreamingResponse(fmt.Sprintf(`{"label":%q}`, summaryText)) + } + return chattest.OpenAINonStreamingResponse(`{"title":"Summary push test"}`) } return chattest.OpenAIStreamingResponse( chattest.OpenAITextChunks(assistantText)..., @@ -6630,9 +6635,9 @@ func TestSuccessfulChatPersistsTurnSummaryWithoutWebPush(t *testing.T) { }, testutil.IntervalFast) require.Equal(t, summaryText, fromDB.LastTurnSummary.String, - "summary should persist even when web push is unavailable") + "status label should persist even when web push is unavailable") require.Equal(t, int32(1), nonStreamingRequests.Load(), - "expected exactly one non-streaming request for summary generation") + "expected exactly one non-streaming request for status label generation") } func TestSuccessfulChatSendsWebPushFallbackWithoutSummaryForEmptyAssistantText(t *testing.T) { @@ -6644,8 +6649,11 @@ func TestSuccessfulChatSendsWebPushFallbackWithoutSummaryForEmptyAssistantText(t var nonStreamingRequests atomic.Int32 openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse { if !req.Stream { - nonStreamingRequests.Add(1) - return chattest.OpenAINonStreamingResponse("unexpected summary request") + if strings.Contains(string(req.RawBody), "propose_turn_status_label") { + nonStreamingRequests.Add(1) + return chattest.OpenAINonStreamingResponse(`{"label":"Unexpected label"}`) + } + return chattest.OpenAINonStreamingResponse(`{"title":"Empty summary push test"}`) } return chattest.OpenAIStreamingResponse( chattest.OpenAITextChunks(" ")..., @@ -6689,14 +6697,14 @@ func TestSuccessfulChatSendsWebPushFallbackWithoutSummaryForEmptyAssistantText(t fromDB, err := db.GetChatByID(ctx, chat.ID) require.NoError(t, err) - require.False(t, fromDB.LastTurnSummary.Valid, - "fallback push text should not be persisted") + require.Equal(t, sql.NullString{String: "Finished latest turn", Valid: true}, fromDB.LastTurnSummary, + "fallback status label should be persisted") msg := mockPush.getLastMessage() - require.Equal(t, "Agent has finished running.", msg.Body, + require.Equal(t, "Finished latest turn", msg.Body, "push body should fall back when the final assistant text is empty") require.Equal(t, int32(0), nonStreamingRequests.Load(), - "push summary should not be requested when final assistant text has no usable text") + "status label model should not run when final assistant text has no usable text") } func TestErroredChatClearsLastTurnSummaryAndSendsWebPush(t *testing.T) { @@ -6757,7 +6765,7 @@ func TestErroredChatClearsLastTurnSummaryAndSendsWebPush(t *testing.T) { "errored chats should clear cached turn summaries") msg := mockPush.getLastMessage() - require.NotEqual(t, "Agent encountered an error.", msg.Body) + require.NotEqual(t, "Hit an error", msg.Body) require.Contains(t, msg.Body, "OpenAI returned an unexpected error") } diff --git a/coderd/x/chatd/quickgen.go b/coderd/x/chatd/quickgen.go index 13a7feba70..0acfd7a941 100644 --- a/coderd/x/chatd/quickgen.go +++ b/coderd/x/chatd/quickgen.go @@ -103,6 +103,10 @@ type generatedTitle struct { Title string `json:"title" description:"Short descriptive chat title"` } +type generatedTurnStatusLabel struct { + Label string `json:"label" description:"Compact 2-5 word current chat status label"` +} + // maybeGenerateChatTitle generates an AI title for the chat when // appropriate (first user message, no assistant reply yet, and the // current title is either empty or still the fallback truncation). @@ -802,19 +806,24 @@ func generateManualTitle( return title, usage, nil } -const pushSummaryPrompt = "You are a notification assistant. Given a chat title " + - "and the agent's last message, write a single short sentence (under 100 characters) " + - "summarizing what the agent did. This will be shown as a push notification body. " + - "Return plain text only — no quotes, no emoji, no markdown." +const turnStatusLabelPrompt = "You write compact chat status labels for a sidebar or push notification. " + + "Given a chat title, current chat state, and the agent's latest message, populate the label field with a 2-5 word status label. " + + "Describe the chat's current state, not the agent. " + + "Good examples: Finished unit tests, Submitted PR, Still working on API, Waiting for user input. " + + "Do not start with Agent, I, We, It, The agent, or The chat. " + + "Avoid phrases like Agent asked, Agent identified, Agent found, or Agent explained. " + + "Prefer short action or state phrases such as Finished, Submitted, Fixed, Testing, Still working, or Waiting for. " + + "No quotes, emoji, markdown, or trailing punctuation." -// generatePushSummary calls a cheap model to produce a short push -// notification body from the chat title and the last assistant +// generateTurnStatusLabel calls a cheap model to produce a short status +// label from the chat title, current state, and last assistant // message text. It follows the same candidate-selection strategy // as title generation: try preferred lightweight models first, then // fall back to the provided model. Returns "" on any failure. -func generatePushSummary( +func generateTurnStatusLabel( ctx context.Context, chat database.Chat, + status database.ChatStatus, assistantText string, fallbackProvider string, fallbackModelName string, @@ -827,11 +836,13 @@ func generatePushSummary( ) string { debugEnabled := debugSvc != nil && debugSvc.IsEnabled(ctx, chat.ID, chat.OwnerID) - summaryCtx, cancel := context.WithTimeout(ctx, 30*time.Second) + labelCtx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() assistantText = truncateRunes(assistantText, maxConversationContextRunes) - input := "Chat title: " + chat.Title + "\n\nAgent's last message:\n" + assistantText + input := "Current chat state: " + turnStatusLabelStateContext(status) + + "\nChat title: " + chat.Title + + "\n\nAgent's latest message:\n" + assistantText candidates := make([]shortTextCandidate, 0, len(preferredTitleModels)+1) for _, c := range preferredTitleModels { @@ -854,15 +865,15 @@ func generatePushSummary( lm: fallbackModel, }) - pushSeedSummary := chatdebug.SeedSummary("Push summary") + statusSeedSummary := chatdebug.SeedSummary("Turn status label") for _, candidate := range candidates { - candidateCtx := summaryCtx + candidateCtx := labelCtx candidateModel := candidate.lm finishDebugRun := func(error) {} if debugEnabled { candidateCtx, candidateModel, finishDebugRun = prepareQuickgenDebugCandidate( - summaryCtx, + labelCtx, chat, keys, debugSvc, @@ -870,42 +881,41 @@ func generatePushSummary( chatdebug.KindQuickgen, triggerMessageID, historyTipMessageID, - pushSeedSummary, + statusSeedSummary, logger, ) } - summary, err := generateShortText( + generatedLabel, err := generateStructuredTurnStatusLabel( candidateCtx, candidateModel, - pushSummaryPrompt, + turnStatusLabelPrompt, input, ) finishDebugRun(err) if err != nil { - logger.Debug(ctx, "push summary model candidate failed", + logger.Debug(ctx, "turn status label model candidate failed", slog.Error(err), ) continue } - if summary != "" { - return summary - } + return generatedLabel } return "" } -// generateShortText calls a model with a system prompt and user -// input, returning a cleaned-up short text response. It reuses the -// same retry logic as title generation. Retries can therefore -// produce multiple debug steps for a single quickgen run. -func generateShortText( +func generateStructuredTurnStatusLabel( ctx context.Context, model fantasy.LanguageModel, systemPrompt string, userInput string, ) (string, error) { - prompt := []fantasy.Message{ + userInput = strings.TrimSpace(userInput) + if userInput == "" { + return "", xerrors.New("turn status label input was empty") + } + + prompt := fantasy.Prompt{ { Role: fantasy.MessageRoleSystem, Content: []fantasy.MessagePart{ @@ -920,27 +930,128 @@ func generateShortText( }, } - var maxOutputTokens int64 = 256 - - var response *fantasy.Response + var maxOutputTokens int64 = 64 + var result *fantasy.ObjectResult[generatedTurnStatusLabel] err := chatretry.Retry(ctx, func(retryCtx context.Context) error { var genErr error - response, genErr = model.Generate(retryCtx, fantasy.Call{ - Prompt: prompt, - MaxOutputTokens: &maxOutputTokens, + result, genErr = object.Generate[generatedTurnStatusLabel](retryCtx, model, fantasy.ObjectCall{ + Prompt: prompt, + SchemaName: "propose_turn_status_label", + SchemaDescription: "Propose a compact chat status label.", + MaxOutputTokens: &maxOutputTokens, }) return genErr }, nil) if err != nil { - return "", xerrors.Errorf("generate short text: %w", err) + return "", xerrors.Errorf("generate structured turn status label: %w", err) } - responseParts := make([]codersdk.ChatMessagePart, 0, len(response.Content)) - for _, block := range response.Content { - if p := chatprompt.PartFromContent(block); p.Type != "" { - responseParts = append(responseParts, p) + label, ok := normalizeTurnStatusLabel(result.Object.Label) + if !ok { + return "", xerrors.New("generated turn status label was invalid") + } + return label, nil +} + +func turnStatusLabelStateContext(status database.ChatStatus) string { + switch status { + case database.ChatStatusWaiting: + return "The turn finished and the chat is idle." + case database.ChatStatusPending: + return "Another user message is queued and the chat will continue." + case database.ChatStatusRequiresAction: + return "The chat is waiting for user input or action." + case database.ChatStatusError: + return "The chat ended with an error." + default: + return "The chat state is unknown." + } +} + +func fallbackTurnStatusLabel(status database.ChatStatus) string { + switch status { + case database.ChatStatusWaiting: + return "Finished latest turn" + case database.ChatStatusPending: + return "Still working on request" + case database.ChatStatusRequiresAction: + return "Waiting for user input" + case database.ChatStatusError: + return "Hit an error" + default: + return "Updated chat status" + } +} + +func normalizeTurnStatusLabel(text string) (string, bool) { + text = strings.TrimSpace(text) + if text == "" { + return "", false + } + + text = strings.Trim(text, "\"'`") + text = strings.TrimSpace(text) + if text == "" || strings.ContainsAny(text, "\r\n") { + return "", false + } + text = strings.TrimRight(text, ".!?") + text = strings.Join(strings.Fields(text), " ") + if text == "" || hasSentenceBoundary(text) { + return "", false + } + + words := strings.Fields(text) + if len(words) < 2 || len(words) > 5 { + return "", false + } + + lower := strings.ToLower(text) + if hasDisallowedTurnStatusLabelSubject(lower) { + return "", false + } + + disallowedPhrases := []string{ + "agent asked", + "agent identified", + "agent found", + "agent explained", + } + for _, phrase := range disallowedPhrases { + if strings.Contains(lower, phrase) { + return "", false } } - text := normalizeShortTextOutput(contentBlocksToText(responseParts)) - return text, nil + + return text, true +} + +func hasDisallowedTurnStatusLabelSubject(text string) bool { + subject := leadingLetters(text) + switch subject { + case "agent", "i", "it", "the", "we": + return true + default: + return false + } +} + +func leadingLetters(text string) string { + for i, r := range text { + if r < 'a' || r > 'z' { + return text[:i] + } + } + return text +} + +func hasSentenceBoundary(text string) bool { + for i, r := range text { + switch r { + case '.', '!', '?': + if i+1 < len(text) && text[i+1] == ' ' { + return true + } + } + } + return false } diff --git a/coderd/x/chatd/quickgen_test.go b/coderd/x/chatd/quickgen_test.go index fb87a8a73b..8371d0bc0f 100644 --- a/coderd/x/chatd/quickgen_test.go +++ b/coderd/x/chatd/quickgen_test.go @@ -595,23 +595,108 @@ func Test_selectPreferredConfiguredShortTextModelConfig(t *testing.T) { }) } -func Test_generateShortText_NormalizesQuotedOutput(t *testing.T) { +func TestNormalizeTurnStatusLabel(t *testing.T) { t.Parallel() - model := &chattest.FakeModel{ - GenerateFn: func(_ context.Context, _ fantasy.Call) (*fantasy.Response, error) { - return &fantasy.Response{ - Content: fantasy.ResponseContent{ - fantasy.TextContent{Text: " \"Quoted summary\" "}, - }, - Usage: fantasy.Usage{InputTokens: 3, OutputTokens: 2, TotalTokens: 5}, - }, nil - }, + tests := []struct { + name string + input string + want string + ok bool + }{ + {name: "accepts short label", input: "Finished unit tests", want: "Finished unit tests", ok: true}, + {name: "accepts two word label", input: "Submitted PR", want: "Submitted PR", ok: true}, + {name: "trims quotes and trailing punctuation", input: `"Submitted PR."`, want: "Submitted PR", ok: true}, + {name: "keeps version punctuation", input: "Updated v2.1 config", want: "Updated v2.1 config", ok: true}, + {name: "accepts five word label", input: "Updated workspace proxy routing rules", want: "Updated workspace proxy routing rules", ok: true}, + {name: "rejects agent phrasing", input: "Agent identified failing tests", ok: false}, + {name: "rejects agent possessive", input: "Agent's findings reviewed", ok: false}, + {name: "rejects i contraction", input: "I've fixed tests", ok: false}, + {name: "rejects it contraction", input: "It's still running", ok: false}, + {name: "rejects we contraction", input: "We're almost done", ok: false}, + {name: "rejects agent phrase without prefix", input: "Found agent identified bugs", ok: false}, + {name: "rejects chat phrasing", input: "The chat is waiting now", ok: false}, + {name: "rejects multiline labels", input: "Fixed bug\nAdded tests", ok: false}, + {name: "rejects multi sentence labels", input: "Fixed bug. Added tests", ok: false}, + {name: "rejects single word", input: "Fixed", ok: false}, + {name: "rejects long labels", input: "Fixed the bug and added tests", ok: false}, } - text, err := generateShortText(context.Background(), model, "system", "user") - require.NoError(t, err) - require.Equal(t, "Quoted summary", text) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, ok := normalizeTurnStatusLabel(tt.input) + require.Equal(t, tt.ok, ok) + require.Equal(t, tt.want, got) + }) + } +} + +func TestFallbackTurnStatusLabel(t *testing.T) { + t.Parallel() + + tests := []struct { + status database.ChatStatus + want string + }{ + {status: database.ChatStatusWaiting, want: "Finished latest turn"}, + {status: database.ChatStatusPending, want: "Still working on request"}, + {status: database.ChatStatusRequiresAction, want: "Waiting for user input"}, + {status: database.ChatStatusError, want: "Hit an error"}, + {status: database.ChatStatus("unknown"), want: "Updated chat status"}, + } + + for _, tt := range tests { + t.Run(string(tt.status), func(t *testing.T) { + t.Parallel() + require.Equal(t, tt.want, fallbackTurnStatusLabel(tt.status)) + }) + } +} + +func TestGenerateStructuredTurnStatusLabel(t *testing.T) { + t.Parallel() + + t.Run("returns compact label", func(t *testing.T) { + t.Parallel() + + model := &chattest.FakeModel{ + GenerateObjectFn: func(_ context.Context, call fantasy.ObjectCall) (*fantasy.ObjectResponse, error) { + require.Equal(t, "propose_turn_status_label", call.SchemaName) + return &fantasy.ObjectResponse{ + Object: map[string]any{"label": "Submitted PR"}, + }, nil + }, + } + + label, err := generateStructuredTurnStatusLabel(context.Background(), model, turnStatusLabelPrompt, "done") + require.NoError(t, err) + require.Equal(t, "Submitted PR", label) + }) + + t.Run("rejects narrative label", func(t *testing.T) { + t.Parallel() + + model := &chattest.FakeModel{ + GenerateObjectFn: func(_ context.Context, _ fantasy.ObjectCall) (*fantasy.ObjectResponse, error) { + return &fantasy.ObjectResponse{ + Object: map[string]any{"label": "Agent identified failing tests"}, + }, nil + }, + } + + _, err := generateStructuredTurnStatusLabel(context.Background(), model, turnStatusLabelPrompt, "done") + require.ErrorContains(t, err, "generated turn status label was invalid") + }) + + t.Run("rejects empty input", func(t *testing.T) { + t.Parallel() + + model := &chattest.FakeModel{} + _, err := generateStructuredTurnStatusLabel(context.Background(), model, turnStatusLabelPrompt, " ") + require.ErrorContains(t, err, "turn status label input was empty") + }) } func mustChatMessage( diff --git a/coderd/x/chatd/turn_summary_internal_test.go b/coderd/x/chatd/turn_summary_internal_test.go index fab0ed3e7f..87abdaf881 100644 --- a/coderd/x/chatd/turn_summary_internal_test.go +++ b/coderd/x/chatd/turn_summary_internal_test.go @@ -135,14 +135,16 @@ func TestPendingChatPersistsSummaryButSkipsWebPush(t *testing.T) { }) require.NoError(t, err) - const summary = "Finished the queued turn." + const summary = "Still working on request" + var generateCalls atomic.Int32 model := &chattest.FakeModel{ ProviderName: "openai", ModelName: "test-model", GenerateFn: func(_ context.Context, _ fantasy.Call) (*fantasy.Response, error) { + generateCalls.Add(1) return &fantasy.Response{ Content: fantasy.ResponseContent{ - fantasy.TextContent{Text: summary}, + fantasy.TextContent{Text: "Unexpected label"}, }, }, nil }, @@ -151,14 +153,14 @@ func TestPendingChatPersistsSummaryButSkipsWebPush(t *testing.T) { dispatcher := &recordingWebpushDispatcher{} logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}) server := &Server{db: db, webpushDispatcher: dispatcher} - server.maybeFinalizeTurnSummaryAndPush( + server.maybeFinalizeTurnStatusLabelAndPush( context.WithoutCancel(ctx), chat, database.ChatStatusPending, "", runChatResult{ FinalAssistantText: "I finished the queued turn.", - PushSummaryModel: model, + StatusLabelModel: model, FallbackProvider: model.Provider(), FallbackModel: model.Model(), }, @@ -169,6 +171,7 @@ func TestPendingChatPersistsSummaryButSkipsWebPush(t *testing.T) { fetched, err := db.GetChatByID(ctx, chat.ID) require.NoError(t, err) require.Equal(t, sql.NullString{String: summary, Valid: true}, fetched.LastTurnSummary) + require.Equal(t, int32(0), generateCalls.Load()) require.Equal(t, int32(0), dispatcher.dispatchCount.Load()) }