From 41f5caafec6dc406ad5b009d083666dbaee6a196 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:53:07 +0200 Subject: [PATCH] fix: fold web searches, answer reasoning, and OpenAI narration into working blocks (#30529) --- coderd/apidoc/docs.go | 4 + coderd/apidoc/swagger.json | 4 + coderd/x/chatd/chatd_test.go | 77 ++- coderd/x/chatd/chatloop/chatloop.go | 33 +- .../chatd/chatloop/chatloop_internal_test.go | 44 ++ coderd/x/chatd/chatprompt/chatprompt.go | 14 + coderd/x/chatd/chattest/openai.go | 20 +- coderd/x/chatd/integration_responses_test.go | 84 +++ codersdk/chats.go | 4 + docs/reference/api/chats.md | 11 + docs/reference/api/schemas.md | 15 + go.mod | 12 +- go.sum | 4 +- site/src/api/typesGenerated.ts | 6 + .../ConversationTimeline.test.tsx | 53 ++ .../ChatConversation/ConversationTimeline.tsx | 93 ++- ...ationTimelineLiveWorkingBlocks.stories.tsx | 53 ++ ...versationTimelineWorkingBlocks.stories.tsx | 91 +++ .../WorkingBlockDisclosure.stories.tsx | 15 + .../WorkingBlockDisclosure.tsx | 9 +- .../components/ChatConversation/blockUtils.ts | 25 +- .../ChatConversation/messageHelpers.ts | 7 +- .../ChatConversation/messageParsing.ts | 24 +- .../ChatConversation/storyFixtures.ts | 132 ++++ .../ChatConversation/streamState.test.ts | 13 +- .../ChatConversation/streamState.ts | 22 +- .../components/ChatConversation/types.ts | 12 + .../workingBlockGrouping.test.ts | 608 ++++++++++++++++++ .../ChatConversation/workingBlockGrouping.ts | 225 +++++-- 29 files changed, 1569 insertions(+), 145 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 8d5a2bd8b9..a372be96f3 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -21301,6 +21301,10 @@ const docTemplate = `{ "name": { "type": "string" }, + "narration": { + "description": "Narration marks assistant text that the model labeled as commentary\non its work rather than its answer. Only providers that label their\noutput set it.", + "type": "boolean" + }, "parsed_commands": { "description": "ParsedCommands holds parsed programs from an execute tool call's\nshell command, one entry per simple command in source order. Each\nentry is [program] or [program, arg] where arg is the first non-flag\npositional argument. Program names are normalized to their base\nname (e.g. /usr/bin/go becomes go). Only populated when ToolName\nis \"execute\" and the command parses successfully; nil otherwise.", "type": "array", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index d927c0421a..e1324ac12f 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -19231,6 +19231,10 @@ "name": { "type": "string" }, + "narration": { + "description": "Narration marks assistant text that the model labeled as commentary\non its work rather than its answer. Only providers that label their\noutput set it.", + "type": "boolean" + }, "parsed_commands": { "description": "ParsedCommands holds parsed programs from an execute tool call's\nshell command, one entry per simple command in source order. Each\nentry is [program] or [program, arg] where arg is the first non-flag\npositional argument. Program names are normalized to their base\nname (e.g. /usr/bin/go becomes go). Only populated when ToolName\nis \"execute\" and the command parses successfully; nil otherwise.", "type": "array", diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index 54cb13f2e4..64b2843a12 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -8864,36 +8864,57 @@ func TestActiveServer_AnthropicDropsUnpairedProviderToolBeforePersist(t *testing func TestActiveServer_AnthropicKeepsPairedWebSearchBeforePersist(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitLong) - db, ps := dbtestutil.NewDB(t) - requests := newAnthropicRequestRecorder() - anthropicURL := chattest.NewAnthropic(t, func(req *chattest.AnthropicRequest) chattest.AnthropicResponse { - requests.record(req) - return chattest.AnthropicStreamingResponse( - anthropicWebSearchPairChunks("ws-1", `{"query":"coder"}`, "search done", "end_turn")..., - ) - }) - user, org, model := seedAnthropicChatDependencies(t, db, anthropicURL) - model = enableAnthropicWebSearchForTest(t, db, model) + for _, tt := range []struct { + name string + noResults bool + }{ + {name: "WithResults"}, + // Clients see a search that found nothing while it streams, so it + // must persist like any other. + {name: "NoResults", noResults: true}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() - server := newActiveTestServer(t, db, ps, func(cfg *chatd.Config) { - cfg.AIBridgeTransportFactory = chatAIGatewayTransportFactoryPointer(chattest.NewMockAIBridgeTransport(t, anthropicURL, chattest.WithPreservePath())) - }) - chat := createChatThroughServer(ctx, t, db, server, org.ID, user.ID, model.ID, "search for coder") - waitForChatStatus(ctx, t, db, chat.ID, database.ChatStatusWaiting) + chunks := anthropicWebSearchPairChunks("ws-1", `{"query":"coder"}`, "search done", "end_turn") + if tt.noResults { + for i := range chunks { + if chunks[i].ContentBlock.Type == "web_search_tool_result" { + chunks[i].ContentBlock.Content = []map[string]any{} + } + } + } - generationRequests := filterAnthropicStreamingRequests(requests.all()) - require.Len(t, generationRequests, 1) - parts := chatToolParts(ctx, t, db, chat.ID) - toolCall := requireToolCallPart(t, parts, "web_search") - require.Equal(t, "ws-1", toolCall.ToolCallID) - require.True(t, toolCall.ProviderExecuted) - toolResult := requireToolResultPart(t, parts, "web_search") - require.Equal(t, "ws-1", toolResult.ToolCallID) - require.True(t, toolResult.ProviderExecuted) - require.NotEmpty(t, toolResult.ProviderMetadata) - messages := chatMessages(ctx, t, db, chat.ID) - requireTextPart(t, messages[len(messages)-1], "search done") + ctx := testutil.Context(t, testutil.WaitLong) + db, ps := dbtestutil.NewDB(t) + requests := newAnthropicRequestRecorder() + anthropicURL := chattest.NewAnthropic(t, func(req *chattest.AnthropicRequest) chattest.AnthropicResponse { + requests.record(req) + return chattest.AnthropicStreamingResponse(chunks...) + }) + user, org, model := seedAnthropicChatDependencies(t, db, anthropicURL) + model = enableAnthropicWebSearchForTest(t, db, model) + + server := newActiveTestServer(t, db, ps, func(cfg *chatd.Config) { + cfg.AIBridgeTransportFactory = chatAIGatewayTransportFactoryPointer(chattest.NewMockAIBridgeTransport(t, anthropicURL, chattest.WithPreservePath())) + }) + chat := createChatThroughServer(ctx, t, db, server, org.ID, user.ID, model.ID, "search for coder") + waitForChatStatus(ctx, t, db, chat.ID, database.ChatStatusWaiting) + + generationRequests := filterAnthropicStreamingRequests(requests.all()) + require.Len(t, generationRequests, 1) + parts := chatToolParts(ctx, t, db, chat.ID) + toolCall := requireToolCallPart(t, parts, "web_search") + require.Equal(t, "ws-1", toolCall.ToolCallID) + require.True(t, toolCall.ProviderExecuted) + toolResult := requireToolResultPart(t, parts, "web_search") + require.Equal(t, "ws-1", toolResult.ToolCallID) + require.True(t, toolResult.ProviderExecuted) + require.NotEmpty(t, toolResult.ProviderMetadata) + messages := chatMessages(ctx, t, db, chat.ID) + requireTextPart(t, messages[len(messages)-1], "search done") + }) + } } // TestActiveServer_AnthropicWebSearchFollowUpHasNoSyntheticCancellation diff --git a/coderd/x/chatd/chatloop/chatloop.go b/coderd/x/chatd/chatloop/chatloop.go index 9c52709538..bd5dca11aa 100644 --- a/coderd/x/chatd/chatloop/chatloop.go +++ b/coderd/x/chatd/chatloop/chatloop.go @@ -308,6 +308,12 @@ type reasoningState struct { startedAt time.Time } +type textState struct { + text string + options fantasy.ProviderMetadata + narration bool +} + // GenerateAssistant performs one assistant model stream and returns the // durable assistant-side content. It does not execute tools, retry, or persist. func GenerateAssistant(ctx context.Context, opts GenerateAssistantOptions) (_ AssistantOutcome, retErr error) { @@ -973,7 +979,7 @@ func processStepStream( var result stepResult providerExecutedCalls := make(map[string]bool) - activeTextContent := make(map[string]string) + activeTextContent := make(map[string]textState) activeReasoningContent := make(map[string]reasoningState) // Track tool names by ID for input delta publishing. toolNames := make(map[string]string) @@ -982,19 +988,30 @@ func processStepStream( for part := range stream { switch part.Type { case fantasy.StreamPartTypeTextStart: - activeTextContent[part.ID] = "" + // OpenAI labels narration when the text starts, so every delta + // can carry the label before the text ends. + activeTextContent[part.ID] = textState{ + options: part.ProviderMetadata, + narration: chatprompt.IsNarration(part.ProviderMetadata), + } case fantasy.StreamPartTypeTextDelta: - if _, exists := activeTextContent[part.ID]; exists { - activeTextContent[part.ID] += part.Delta + textPart := codersdk.ChatMessageText(part.Delta) + if active, exists := activeTextContent[part.ID]; exists { + active.text += part.Delta + activeTextContent[part.ID] = active + textPart.Narration = active.narration } - publishMessagePart(codersdk.ChatMessageRoleAssistant, codersdk.ChatMessageText(part.Delta)) + publishMessagePart(codersdk.ChatMessageRoleAssistant, textPart) case fantasy.StreamPartTypeTextEnd: - if text, exists := activeTextContent[part.ID]; exists { + if active, exists := activeTextContent[part.ID]; exists { + if len(part.ProviderMetadata) > 0 { + active.options = part.ProviderMetadata + } result.content = append(result.content, fantasy.TextContent{ - Text: text, - ProviderMetadata: part.ProviderMetadata, + Text: active.text, + ProviderMetadata: active.options, }) delete(activeTextContent, part.ID) } diff --git a/coderd/x/chatd/chatloop/chatloop_internal_test.go b/coderd/x/chatd/chatloop/chatloop_internal_test.go index 28425cc1e4..1c4664feed 100644 --- a/coderd/x/chatd/chatloop/chatloop_internal_test.go +++ b/coderd/x/chatd/chatloop/chatloop_internal_test.go @@ -6,6 +6,7 @@ import ( "charm.land/fantasy" fantasyanthropic "charm.land/fantasy/providers/anthropic" + fantasyopenai "charm.land/fantasy/providers/openai" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/codersdk" @@ -79,3 +80,46 @@ func TestProcessStepStreamPersistsRedactedThinkingOnEnd(t *testing.T) { require.NotNil(t, metadata) require.Equal(t, "redacted-payload", metadata.RedactedData) } + +func TestProcessStepStreamLabelsNarration(t *testing.T) { + t.Parallel() + + phase := func(itemID, phase string) fantasy.ProviderMetadata { + return fantasy.ProviderMetadata{ + fantasyopenai.Name: &fantasyopenai.ResponsesTextMetadata{ItemID: itemID, Phase: phase}, + } + } + stream := iter.Seq[fantasy.StreamPart](func(yield func(fantasy.StreamPart) bool) { + // The narration's end carries no metadata, so the start's label + // must persist. + yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeTextStart, ID: "msg_1", ProviderMetadata: phase("msg_1", "commentary")}) + yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeTextDelta, ID: "msg_1", Delta: "Reading"}) + yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeTextDelta, ID: "msg_1", Delta: " the file."}) + yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeTextEnd, ID: "msg_1", ProviderMetadata: fantasy.ProviderMetadata{}}) + yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeTextStart, ID: "msg_2", ProviderMetadata: phase("msg_2", "final_answer")}) + yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeTextDelta, ID: "msg_2", Delta: "It is flaky."}) + yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeTextEnd, ID: "msg_2", ProviderMetadata: phase("msg_2", "final_answer")}) + yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop}) + }) + + var published []codersdk.ChatMessagePart + result, err := processStepStream(stream, quartz.NewMock(t), func(_ codersdk.ChatMessageRole, part codersdk.ChatMessagePart) { + published = append(published, part) + }) + require.NoError(t, err) + + narration := codersdk.ChatMessageText("Reading") + narration.Narration = true + rest := codersdk.ChatMessageText(" the file.") + rest.Narration = true + require.Equal(t, []codersdk.ChatMessagePart{ + narration, + rest, + codersdk.ChatMessageText("It is flaky."), + }, published) + + require.Equal(t, []fantasy.Content{ + fantasy.TextContent{Text: "Reading the file.", ProviderMetadata: phase("msg_1", "commentary")}, + fantasy.TextContent{Text: "It is flaky.", ProviderMetadata: phase("msg_2", "final_answer")}, + }, result.content) +} diff --git a/coderd/x/chatd/chatprompt/chatprompt.go b/coderd/x/chatd/chatprompt/chatprompt.go index 8684965641..82a32eb0b7 100644 --- a/coderd/x/chatd/chatprompt/chatprompt.go +++ b/coderd/x/chatd/chatprompt/chatprompt.go @@ -11,6 +11,7 @@ import ( "strings" "charm.land/fantasy" + fantasyopenai "charm.land/fantasy/providers/openai" "github.com/dustin/go-humanize" "github.com/google/uuid" "github.com/sqlc-dev/pqtype" @@ -651,12 +652,14 @@ func sdkPartFromContent( return codersdk.ChatMessagePart{ Type: codersdk.ChatMessagePartTypeText, Text: value.Text, + Narration: IsNarration(value.ProviderMetadata), ProviderMetadata: marshalProviderMetadata(value.ProviderMetadata), } case *fantasy.TextContent: return codersdk.ChatMessagePart{ Type: codersdk.ChatMessagePartTypeText, Text: value.Text, + Narration: IsNarration(value.ProviderMetadata), ProviderMetadata: marshalProviderMetadata(value.ProviderMetadata), } case fantasy.ReasoningContent: @@ -732,6 +735,17 @@ func sdkPartFromContent( } } +// IsNarration reports whether the provider labeled text as commentary on +// the model's work rather than its answer. +func IsNarration(metadata fantasy.ProviderMetadata) bool { + for _, data := range metadata { + if text, ok := data.(*fantasyopenai.ResponsesTextMetadata); ok && text != nil { + return text.Phase == "commentary" + } + } + return false +} + // toolResultContentToPart converts a fantasy ToolResultContent into a // ChatMessagePart. func toolResultContentToPart( diff --git a/coderd/x/chatd/chattest/openai.go b/coderd/x/chatd/chattest/openai.go index 313f6551dc..f6169bc89a 100644 --- a/coderd/x/chatd/chattest/openai.go +++ b/coderd/x/chatd/chattest/openai.go @@ -158,6 +158,9 @@ type OpenAIChunkChoice struct { ReasoningDelta string `json:"reasoning_delta,omitempty"` ToolCalls []OpenAIToolCall `json:"tool_calls,omitempty"` FinishReason string `json:"finish_reason,omitempty"` + // Phase labels the Responses API message item that the first delta + // of an output index opens. + Phase string `json:"phase,omitempty"` } // OpenAIChunk represents a streaming chunk from OpenAI. @@ -520,6 +523,7 @@ func writeResponsesAPIStreaming(t testing.TB, w http.ResponseWriter, r *http.Req itemType string // "message" or "function_call" itemID string text string // accumulated text for message items + phase string // phase for message items callID string // call_id for function_call items toolName string // function name for function_call items arguments string // accumulated arguments for function_call items @@ -742,7 +746,7 @@ func writeResponsesAPIStreaming(t testing.TB, w http.ResponseWriter, r *http.Req } if !writeEvent("response.output_item.done", map[string]interface{}{ "output_index": outputIndex, - "item": map[string]interface{}{ + "item": withMessagePhase(map[string]interface{}{ "type": "message", "id": state.itemID, "role": "assistant", @@ -753,7 +757,7 @@ func writeResponsesAPIStreaming(t testing.TB, w http.ResponseWriter, r *http.Req "text": state.text, }, }, - }, + }, state.phase), }) { return } @@ -835,17 +839,18 @@ func writeResponsesAPIStreaming(t testing.TB, w http.ResponseWriter, r *http.Req state = &outputItemState{ itemType: "message", itemID: fmt.Sprintf("msg_%s", uuid.New().String()[:8]), + phase: choice.Phase, } outputs[outputIndex] = state if !writeEvent("response.output_item.added", map[string]interface{}{ "output_index": outputIndex, - "item": map[string]interface{}{ + "item": withMessagePhase(map[string]interface{}{ "type": "message", "id": state.itemID, "role": "assistant", "status": "in_progress", "content": []interface{}{}, - }, + }, state.phase), }) { return } @@ -875,6 +880,13 @@ func writeResponsesAPIStreaming(t testing.TB, w http.ResponseWriter, r *http.Req } } +func withMessagePhase(item map[string]interface{}, phase string) map[string]interface{} { + if phase != "" { + item["phase"] = phase + } + return item +} + func (s *openAIServer) writeChatCompletionsNonStreaming(w http.ResponseWriter, resp *OpenAICompletion) { w.Header().Set("Content-Type", "application/json") if err := json.NewEncoder(w).Encode(resp); err != nil { diff --git a/coderd/x/chatd/integration_responses_test.go b/coderd/x/chatd/integration_responses_test.go index 9510e1666e..39e189bcdc 100644 --- a/coderd/x/chatd/integration_responses_test.go +++ b/coderd/x/chatd/integration_responses_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbtestutil" dbpubsub "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/x/chatd" @@ -235,6 +236,89 @@ func TestOpenAIResponsesFullReplayPairsReasoningAndWebSearch(t *testing.T) { requirePromptItemReferenceOrder(t, followup.Prompt, reasoningID, webSearchID) } +func TestOpenAIResponsesNarrationKeepsPhase(t *testing.T) { + t.Parallel() + + db, ps := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitLong) + + var recorder responsesRequestRecorder + openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse { + if !req.Stream { + return chattest.OpenAINonStreamingResponse("title") + } + if recorder.record(req) > 1 { + return chattest.OpenAIStreamingResponse(chattest.OpenAITextChunks("follow-up answer")...) + } + return chattest.OpenAIStreamingResponse( + chattest.OpenAIChunk{Choices: []chattest.OpenAIChunkChoice{{Index: 0, Delta: "Checking the docs.", Phase: "commentary"}}}, + chattest.OpenAIChunk{Choices: []chattest.OpenAIChunkChoice{{Index: 1, Delta: "The docs cover it.", Phase: "final_answer"}}}, + ) + }) + + user, org, _ := seedChatDependenciesWithProvider(t, db, "openai", openAIURL) + model := insertOpenAIResponsesModelConfig(t, db, user.ID, true, false) + factory := chattest.NewMockAIBridgeTransport(t, openAIURL) + server := newOpenAIResponsesTestServer(t, db, ps, func(cfg *chatd.Config) { + cfg.AIBridgeTransportFactory = chatAIGatewayTransportFactoryPointer(factory) + }) + + chat, err := server.CreateChat(ctx, chatd.CreateOptions{ + OrganizationID: org.ID, + OwnerID: user.ID, + Title: uniqueResponsesTitle(t, "narration"), + ModelConfigID: model.ID, + InitialUserContent: []codersdk.ChatMessagePart{ + codersdk.ChatMessageText("do the docs cover this?"), + }, + }) + require.NoError(t, err) + waitForChatProcessed(ctx, t, db, chat.ID, server) + requireResponsesChatWaiting(ctx, t, db, chat.ID) + + messages, err := db.GetChatMessagesByChatID(ctx, database.GetChatMessagesByChatIDParams{ChatID: chat.ID}) + require.NoError(t, err) + var assistant []codersdk.ChatMessagePart + for _, message := range messages { + if message.Role == database.ChatMessageRoleAssistant { + assistant = append(assistant, db2sdk.ChatMessage(message).Content...) + } + } + narration := codersdk.ChatMessageText("Checking the docs.") + narration.Narration = true + require.Equal(t, []codersdk.ChatMessagePart{ + narration, + codersdk.ChatMessageText("The docs cover it."), + }, assistant) + + _, err = server.SendMessage(ctx, chatd.SendMessageOptions{ + ChatID: chat.ID, + CreatedBy: user.ID, + ModelConfigID: model.ID, + Content: []codersdk.ChatMessagePart{ + codersdk.ChatMessageText("thanks"), + }, + }) + require.NoError(t, err) + waitForChatProcessed(ctx, t, db, chat.ID, server) + requireResponsesChatWaiting(ctx, t, db, chat.ID) + + requests := recorder.all() + require.Len(t, requests, 2) + var replayed [][2]any + for _, item := range requests[1].Prompt { + itemMap, ok := item.(map[string]interface{}) + if !ok || chattest.StringResponseField(itemMap, "role") != "assistant" { + continue + } + replayed = append(replayed, [2]any{itemMap["content"], itemMap["phase"]}) + } + require.Equal(t, [][2]any{ + {"Checking the docs.", "commentary"}, + {"The docs cover it.", "final_answer"}, + }, replayed) +} + type recordedResponsesRequest struct { Prompt []interface{} Store *bool diff --git a/codersdk/chats.go b/codersdk/chats.go index 55f7f1d8ef..b88a56fb86 100644 --- a/codersdk/chats.go +++ b/codersdk/chats.go @@ -511,6 +511,10 @@ type ChatMessagePart struct { ProviderExecuted bool `json:"provider_executed,omitempty" variants:"tool-call?,tool-result?"` // HookRewritten indicates that a lifecycle hook replaced model-proposed tool input. HookRewritten bool `json:"hook_rewritten,omitempty" variants:"tool-call?"` + // Narration marks assistant text that the model labeled as commentary + // on its work rather than its answer. Only providers that label their + // output set it. + Narration bool `json:"narration,omitempty" variants:"text?"` // CreatedAt is the timestamp this part carries. The semantics // depend on the part type: for tool-call and tool-result parts // it is the time the call was emitted or the result was diff --git a/docs/reference/api/chats.md b/docs/reference/api/chats.md index 2411e05f38..402460875b 100644 --- a/docs/reference/api/chats.md +++ b/docs/reference/api/chats.md @@ -2898,6 +2898,7 @@ curl -X GET http://coder-server:8080/api/v2/chats/{chat}/messages \ "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -2979,6 +2980,7 @@ curl -X GET http://coder-server:8080/api/v2/chats/{chat}/messages \ "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -3134,6 +3136,7 @@ curl -X POST http://coder-server:8080/api/v2/chats/{chat}/messages \ "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -3214,6 +3217,7 @@ curl -X POST http://coder-server:8080/api/v2/chats/{chat}/messages \ "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -3295,6 +3299,7 @@ curl -X POST http://coder-server:8080/api/v2/chats/{chat}/messages \ "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -3435,6 +3440,7 @@ curl -X PATCH http://coder-server:8080/api/v2/chats/{chat}/messages/{message} \ "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -3515,6 +3521,7 @@ curl -X PATCH http://coder-server:8080/api/v2/chats/{chat}/messages/{message} \ "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -4106,6 +4113,7 @@ curl -X GET http://coder-server:8080/api/v2/chats/{chat}/stream \ "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -4185,6 +4193,7 @@ curl -X GET http://coder-server:8080/api/v2/chats/{chat}/stream \ "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -4251,6 +4260,7 @@ curl -X GET http://coder-server:8080/api/v2/chats/{chat}/stream \ "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -4357,6 +4367,7 @@ Status Code **200** | `»»» mcp_server_config_id` | string(uuid) | false | | | | `»»» media_type` | string | false | | | | `»»» name` | string | false | | | +| `»»» narration` | boolean | false | | Narration marks assistant text that the model labeled as commentary on its work rather than its answer. Only providers that label their output set it. | | `»»» parsed_commands` | array | false | | Parsed commands holds parsed programs from an execute tool call's shell command, one entry per simple command in source order. Each entry is [program] or [program, arg] where arg is the first non-flag positional argument. Program names are normalized to their base name (e.g. /usr/bin/go becomes go). Only populated when ToolName is "execute" and the command parses successfully; nil otherwise. | | `»»» provider_executed` | boolean | false | | Provider executed indicates the tool call was executed by the provider (e.g. Anthropic computer use). | | `»»» provider_metadata` | array | false | | Provider metadata holds provider-specific response metadata (e.g. Anthropic cache control hints) as raw JSON. Internal only: stripped by db2sdk before API responses. | diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index d3c98f1066..97767c6d2a 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -3233,6 +3233,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -3328,6 +3329,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -3388,6 +3390,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `mcp_server_config_id` | string | false | | | | `media_type` | string | false | | | | `name` | string | false | | | +| `narration` | boolean | false | | Narration marks assistant text that the model labeled as commentary on its work rather than its answer. Only providers that label their output set it. | | `parsed_commands` | array of array | false | | Parsed commands holds parsed programs from an execute tool call's shell command, one entry per simple command in source order. Each entry is [program] or [program, arg] where arg is the first non-flag positional argument. Program names are normalized to their base name (e.g. /usr/bin/go becomes go). Only populated when ToolName is "execute" and the command parses successfully; nil otherwise. | | `provider_executed` | boolean | false | | Provider executed indicates the tool call was executed by the provider (e.g. Anthropic computer use). | | `provider_metadata` | array of integer | false | | Provider metadata holds provider-specific response metadata (e.g. Anthropic cache control hints) as raw JSON. Internal only: stripped by db2sdk before API responses. | @@ -3502,6 +3505,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -3583,6 +3587,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -4964,6 +4969,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -5127,6 +5133,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -5206,6 +5213,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -5272,6 +5280,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -5387,6 +5396,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -6245,6 +6255,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -6325,6 +6336,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -6406,6 +6418,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -9331,6 +9344,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" @@ -9411,6 +9425,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "mcp_server_config_id": "a9f436ed-69e7-459c-8308-a67ff5387e3e", "media_type": "string", "name": "string", + "narration": true, "parsed_commands": [ [ "string" diff --git a/go.mod b/go.mod index 9d0f3c9413..06643a97a3 100644 --- a/go.mod +++ b/go.mod @@ -105,10 +105,14 @@ replace github.com/spf13/afero => github.com/aslilac/afero v0.0.0-20250403163713 // and gpt-6+ reasoning classification in getResponsesModelConfig; also // carries upstream #354 (gpt-6+ in IsResponsesModel). // 10) coder/fantasy#58, keep media tool results in the Gemini prompt. -// 11) coder/fantasy mike/mcp-tool-media-batch-order (unmerged), keep OpenAI -// tool-result batches contiguous before attaching user media. -// See: https://github.com/coder/fantasy/commits/9a3598480a71 -replace charm.land/fantasy => github.com/coder/fantasy v0.0.0-20260917071228-9a3598480a71 +// 11) coder/fantasy#64, keep OpenAI tool-result batches contiguous before +// attaching user media. +// 12) coder/fantasy#65, attach empty metadata to Anthropic web searches +// with no results so chatd keeps them in saved history. +// 13) coder/fantasy#66, capture OpenAI Responses message phase on text +// parts and replay it on assistant input messages in both store modes. +// See: https://github.com/coder/fantasy/commits/351055280157 +replace charm.land/fantasy => github.com/coder/fantasy v0.0.0-20261009165329-351055280157 // coder/coder uses a fork of charmbracelet's fork of the Anthropic Go SDK // with performance improvements and Bedrock header cleanup. diff --git a/go.sum b/go.sum index d41cd9f1c4..c15e3e2712 100644 --- a/go.sum +++ b/go.sum @@ -333,8 +333,8 @@ github.com/coder/bubbletea v1.2.2-0.20241212190825-007a1cdb2c41 h1:SBN/DA63+ZHwu github.com/coder/bubbletea v1.2.2-0.20241212190825-007a1cdb2c41/go.mod h1:I9ULxr64UaOSUv7hcb3nX4kowodJCVS7vt7VVJk/kW4= github.com/coder/clistat v1.2.1 h1:P9/10njXMyj5cWzIU5wkRsSy5LVQH49+tcGMsAgWX0w= github.com/coder/clistat v1.2.1/go.mod h1:m7SC0uj88eEERgvF8Kn6+w6XF21BeSr+15f7GoLAw0A= -github.com/coder/fantasy v0.0.0-20260917071228-9a3598480a71 h1:kJzgNKoyf+duEF2ClR5UPeCZldi6jR+rM8F2nbNbStY= -github.com/coder/fantasy v0.0.0-20260917071228-9a3598480a71/go.mod h1:Hd5DBPecZ+CGc8O9bFAV03S7jbEmqd9xXVmjKCzpD7g= +github.com/coder/fantasy v0.0.0-20261009165329-351055280157 h1:t1DbZyWlLOPISsLzcFFnKfvSToWpXFIL1pkpFovlNh0= +github.com/coder/fantasy v0.0.0-20261009165329-351055280157/go.mod h1:Hd5DBPecZ+CGc8O9bFAV03S7jbEmqd9xXVmjKCzpD7g= github.com/coder/flog v1.1.0 h1:kbAes1ai8fIS5OeV+QAnKBQE22ty1jRF/mcAwHpLBa4= github.com/coder/flog v1.1.0/go.mod h1:UQlQvrkJBvnRGo69Le8E24Tcl5SJleAAR7gYEHzAmdQ= github.com/coder/go-httpstat v0.0.0-20230801153223-321c88088322 h1:m0lPZjlQ7vdVpRBPKfYIFlmgevoTkBxB10wv6l2gOaU= diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 6d266f59b2..e2551de0e6 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3756,6 +3756,12 @@ export interface ChatSystemPromptResponse { export interface ChatTextPart { readonly type: "text"; readonly text: string; + /** + * Narration marks assistant text that the model labeled as commentary + * on its work rather than its answer. Only providers that label their + * output set it. + */ + readonly narration?: boolean; } // From codersdk/chats.go diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.test.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.test.tsx index c20e970e31..78b299213a 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.test.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.test.tsx @@ -21,6 +21,7 @@ import { buildStreamRenderState, MockCollapsedStepsPreferences, MockLongTurnPageLoads, + MockWebSearchAnswerMessages, MockWorkingMessages, pinFixtureClock, workingFixtureTime, @@ -107,6 +108,27 @@ const focusCopyCommand = (messageId: number) => { }; describe("ConversationTimeline working blocks", () => { + it("copies the whole message from the answer below its expanded block", async () => { + const user = userEvent.setup(); + const writeText = vi + .spyOn(navigator.clipboard, "writeText") + .mockResolvedValue(); + renderTimeline({ messages: MockWebSearchAnswerMessages }); + + await user.click( + screen.getByRole("button", { name: "Worked for 15s (3 steps)" }), + ); + await user.click( + within(screen.getByTestId("chat-message-message:6")).getByRole("button", { + name: "Copy message", + }), + ); + + expect(writeText).toHaveBeenCalledWith( + "Looking up the release notes.The workspace runs the latest Coder release.", + ); + }); + it("keeps an open block mounted as older pages join it", async () => { const user = userEvent.setup(); const { rerenderStage } = renderTimeline({ @@ -334,6 +356,37 @@ describe("ConversationTimeline live working blocks", () => { expect(copyCommand).toHaveFocus(); }); + it("keeps the live reasoning mounted in an open block when the answer starts", async () => { + const user = userEvent.setup(); + const messages = MockWorkingMessages.slice(0, 5); + const reasoning = { + type: "reasoning", + text: "Summarizing the inspection", + created_at: workingFixtureTime(13), + } as const; + const { rerenderStage } = renderTimeline({ + messages, + chatStatus: "running", + ...buildStreamRenderState([reasoning]), + }); + + await user.click(screen.getByRole("button", { name: "Working for 12s" })); + const thinking = await screen.findByRole("button", { + name: /Summarizing/, + }); + thinking.focus(); + + rerenderStage({ + messages, + chatStatus: "running", + ...buildStreamRenderState([ + reasoning, + { type: "text", text: "The workspace looks healthy." }, + ]), + }); + expect(thinking).toHaveFocus(); + }); + it("keeps the live disclosure mounted while the next step starts", () => { const messages = MockWorkingMessages.slice(0, 5); const { rerenderStage } = renderTimeline({ diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx index 666d95d3e3..fcaa9e3678 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx @@ -40,6 +40,7 @@ import { buildDisplayMessages, deriveEvictedFileIds, deriveMessageDisplayState, + getVisibleContent, } from "./messageHelpers"; import { getEditableUserMessagePayload } from "./messageParsing"; import { assignTimelineRows } from "./timelineRows"; @@ -52,7 +53,11 @@ import type { } from "./types"; import { UserMessageContent } from "./UserMessageContent"; import { WorkingBlockDisclosure } from "./WorkingBlockDisclosure"; -import { groupWorkingBlocks } from "./workingBlockGrouping"; +import { + groupWorkingBlocks, + type RowSection, + splitRowBlocks, +} from "./workingBlockGrouping"; const getChatMessageTextContent = ( content: readonly TypesGen.ChatMessagePart[] | undefined, @@ -103,6 +108,7 @@ const ChatMessageItem = memo<{ renderKey: string; // Durable messages and live assistant output share one rendering path. message?: TypesGen.ChatMessage; + section?: RowSection; parsed?: ParsedMessageContent; liveStatus?: LiveStatusModel; // Live blocks and tools are normalized at the live row callsite, so this @@ -145,6 +151,7 @@ const ChatMessageItem = memo<{ organizationId, renderKey, message, + section, parsed, liveStatus, liveBlocks = [], @@ -228,10 +235,26 @@ const ChatMessageItem = memo<{ const conversationItemProps: { role: "user" | "assistant" } = { role: isUser ? "user" : "assistant", }; + const isWorkSection = section === "work"; + const rowBlocks = parsed?.blocks ?? liveBlocks; + const rowTools = parsed?.tools ?? liveTools; + const blocks = + section === undefined + ? rowBlocks + : splitRowBlocks(rowBlocks, rowTools)[section]; + // An answer whose only work was a search without citations folds + // nothing, and an empty row would still add a gap inside the block. + if ( + isWorkSection && + getVisibleContent(blocks, rowTools).visibleBlocks.length === 0 + ) { + return null; + } + const outputLiveStatus = isWorkSection ? undefined : liveStatus; return (
( ? askUserQuestionResponseTextByToolId : undefined; - const rowContents = renderRows.map((row, index) => { + // The work section keeps the row's key, so a row that becomes an answer + // row inside an expanded block does not remount its reasoning. + const renderRowContent = (index: number, section?: RowSection) => { + const row = renderRows[index]; if (row.type === "live") { return ( ( const { message, parsed } = row.entry; const isUser = message.role === "user"; const neighbors = userNeighborsByKey.get(row.key); - // A block's item dims and inerts its rows as a whole. + // A block's item dims and inerts its rows as a whole; the answer + // section renders outside it, so it dims itself. const isAfterEditingMessage = - !groupedRows.has(index) && afterEditingMessageIds.has(message.id); + (section === "answer" || !groupedRows.has(index)) && + afterEditingMessageIds.has(message.id); return ( ( organizationId={organizationId} renderKey={row.key} message={message} + section={section} parsed={parsed} onEditUserMessage={isUser ? onEditUserMessage : undefined} editingMessageId={editingMessageId} @@ -650,7 +681,9 @@ export const ConversationTimeline = memo( hasUserResponseAfterAskQuestion={hasUserResponseAfterAskQuestion} urlTransform={urlTransform} isAfterEditingMessage={isAfterEditingMessage} - hideActions={!isUser && !row.isLastInAssistantChain} + hideActions={ + section === "work" || (!isUser && !row.isLastInAssistantChain) + } hasActiveStream={hasActiveStream} isAwaitingFirstStreamChunk={isAwaitingFirstStreamChunk} isLastMessage={row.isLastMessage} @@ -663,11 +696,11 @@ export const ConversationTimeline = memo( onJumpToUserMessage={isUser ? jumpToUserMessage : undefined} /> ); - }); + }; return ( - {renderRows.map((row, index) => { + {renderRows.flatMap((row, index) => { const block = blockByFirstRow.get(index); if (block) { // Keyed on durable member rows and the item key, excluding the live @@ -687,8 +720,11 @@ export const ConversationTimeline = memo( const isAfterEditingMessage = row.type === "message" && afterEditingMessageIds.has(row.entry.message.id); + const answerRowIndex = block.endsWithAnswer + ? block.rowIndices.at(-1) + : undefined; - return ( + return [ ( } > {expanded && - block.rowIndices.map((rowIndex) => rowContents[rowIndex])} + block.rowIndices.map((rowIndex) => + renderRowContent( + rowIndex, + rowIndex === answerRowIndex ? "work" : undefined, + ), + )} - - ); + , + ...(answerRowIndex === undefined + ? [] + : [ + + {renderRowContent(answerRowIndex, "answer")} + , + ]), + ]; } if (groupedRows.has(index)) { - return null; + return []; } const suppressInitialAnchor = row.type === "message" && initialActiveTurnMaxMessageId !== undefined && row.entry.message.id <= initialActiveTurnMaxMessageId; - return ( + return [ ( row.key === anchorUserRowKey && !suppressInitialAnchor } > - {rowContents[index]} - - ); + {renderRowContent(index)} + , + ]; })} ); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimelineLiveWorkingBlocks.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimelineLiveWorkingBlocks.stories.tsx index a4f1dbe1c1..f184d256c3 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimelineLiveWorkingBlocks.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimelineLiveWorkingBlocks.stories.tsx @@ -131,6 +131,59 @@ export const ReasoningBeforeFirstToolFolds: Story = { }, }; +// Once the final step starts its answer, its reasoning stays in the live +// block and the text streams after it. +export const AnswerStreamsAfterBlock: Story = { + args: { + parsedMessages: parseMessagesWithMergedTools( + MockWorkingMessages.slice(0, 5), + ), + ...buildStreamRenderState([ + { + type: "reasoning", + text: "Summarizing the inspection", + created_at: workingFixtureTime(13), + }, + { type: "text", text: "The workspace looks healthy." }, + ]), + }, + play: expandBlock("Working for 12s"), +}; + +// Narration before a web search stays in the live block, and the answer, cited +// as it streams, renders after it. +export const SearchAnswerStreamsAfterBlock: Story = { + args: { + parsedMessages: parseMessagesWithMergedTools( + MockWorkingMessages.slice(0, 1), + ), + ...buildStreamRenderState([ + { type: "text", text: "I'll look up the latest Coder release." }, + { + type: "tool-call", + tool_call_id: "search", + tool_name: "web_search", + provider_executed: true, + created_at: workingFixtureTime(1), + }, + { + type: "tool-result", + tool_call_id: "search", + tool_name: "web_search", + provider_executed: true, + }, + { type: "text", text: "The latest Coder release" }, + { + type: "source", + url: "https://releases.example.com", + title: "Coder releases", + }, + { type: "text", text: " is v2.30." }, + ]), + }, + play: expandBlock("Working for 12s"), +}; + export const RequiresActionCompletesBlock: Story = { args: { chatStatus: "requires_action", diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimelineWorkingBlocks.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimelineWorkingBlocks.stories.tsx index 3ae784a57b..e47012c871 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimelineWorkingBlocks.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimelineWorkingBlocks.stories.tsx @@ -11,8 +11,11 @@ import { parseMessagesWithMergedTools } from "./messageParsing"; import { buildLiveStatus, MockCollapsedStepsPreferences, + MockLeadInSearchMessages, MockLongTurnPageLoads, MockQuestionCallMessage, + MockUncitedSearchAnswerMessages, + MockWebSearchAnswerMessages, MockWorkingMessages, pinFixtureClock, workingFixtureTime, @@ -73,6 +76,87 @@ export const Expanded: Story = { }, }; +// Only the answer's final text shows after the summary. +export const AnswerWorkFolds: Story = { + args: { + parsedMessages: parseMessagesWithMergedTools(MockWebSearchAnswerMessages), + }, +}; + +export const AnswerWorkFoldsExpanded: Story = { + args: { + parsedMessages: parseMessagesWithMergedTools(MockWebSearchAnswerMessages), + }, + play: async ({ canvasElement }) => { + await userEvent.click( + within(canvasElement).getByRole("button", { + name: "Worked for 15s (3 steps)", + }), + ); + }, +}; + +export const SearchOnlyAnswerFolds: Story = { + args: { + parsedMessages: parseMessagesWithMergedTools([ + MockWebSearchAnswerMessages[0], + ...MockWebSearchAnswerMessages.slice(5), + ]), + }, +}; + +export const UncitedSearchAnswerFoldsExpanded: Story = { + args: { + parsedMessages: parseMessagesWithMergedTools( + MockUncitedSearchAnswerMessages, + ), + }, + play: async ({ canvasElement }) => { + await userEvent.click( + within(canvasElement).getByRole("button", { + name: "Worked for 14s (3 steps)", + }), + ); + }, +}; + +export const LabeledNarrationFolds: Story = { + args: { + parsedMessages: parseMessagesWithMergedTools([ + MockWorkingMessages[0], + { + ...MockChatMessage, + id: 2, + role: "assistant", + created_at: workingFixtureTime(4), + content: [ + { + type: "reasoning", + text: "Finding the docs page", + created_at: workingFixtureTime(1), + completed_at: workingFixtureTime(2), + }, + { type: "text", text: "Checking the docs.", narration: true }, + { type: "text", text: "The docs cover workspace sharing." }, + ], + }, + ]), + }, +}; + +export const LeadInBeforeSearchFolds: Story = { + args: { + parsedMessages: parseMessagesWithMergedTools(MockLeadInSearchMessages), + }, + play: async ({ canvasElement }) => { + await userEvent.click( + within(canvasElement).getByRole("button", { + name: "Worked for 2s (1 step)", + }), + ); + }, +}; + export const Paginated: Story = { args: { hasMoreMessages: true, @@ -210,3 +294,10 @@ export const EditingPrecedingMessage: Story = { await userEvent.click(canvas.getByRole("button", { name: "Edit prompt" })); }, }; + +export const EditingBeforeAnswerWorkFolds: Story = { + args: { + parsedMessages: parseMessagesWithMergedTools(MockWebSearchAnswerMessages), + editingMessageId: 1, + }, +}; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/WorkingBlockDisclosure.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/WorkingBlockDisclosure.stories.tsx index b81b2b294f..29385e3f69 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/WorkingBlockDisclosure.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/WorkingBlockDisclosure.stories.tsx @@ -54,6 +54,21 @@ export const UnknownDuration: Story = { }, }; +export const WithoutSteps: Story = { + args: { block: { ...MockWorkingBlock, stepCount: 0 } }, +}; + +export const WithoutStepsUnknownDuration: Story = { + args: { + block: { + ...MockWorkingBlock, + stepCount: 0, + startedAt: undefined, + endedAt: undefined, + }, + }, +}; + export const Live: Story = { args: { block: { diff --git a/site/src/pages/AgentsPage/components/ChatConversation/WorkingBlockDisclosure.tsx b/site/src/pages/AgentsPage/components/ChatConversation/WorkingBlockDisclosure.tsx index b32effdb86..dada1d7887 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/WorkingBlockDisclosure.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/WorkingBlockDisclosure.tsx @@ -68,15 +68,18 @@ const LiveLabel: React.FC = ({ block }) => { }; const getCompletedWorkingLabel = (block: WorkingBlock): string => { - const steps = countLabel(block, block.stepCount, "step"); + // A block of narration without tool calls has no steps to count. + const steps = + block.stepCount > 0 ? countLabel(block, block.stepCount, "step") : ""; if (block.startedAt === undefined || block.endedAt === undefined) { - return `Completed ${steps}`; + return steps ? `Completed ${steps}` : "Completed"; } const duration = humanDurationShort( Math.max(0, block.endedAt - block.startedAt), ); - return `Worked for ${atLeast(block)}${duration} (${steps})`; + const worked = `Worked for ${atLeast(block)}${duration}`; + return steps ? `${worked} (${steps})` : worked; }; const getScrollParent = (element: HTMLElement): HTMLElement | null => { diff --git a/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.ts b/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.ts index aeb3442be1..2b570259eb 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/blockUtils.ts @@ -6,28 +6,41 @@ export const asNonEmptyString = (value: unknown): string | undefined => { return next.length > 0 ? next : undefined; }; +const textBlock = ( + type: "response" | "thinking", + text: string, + narration: boolean, +): RenderBlock => + type === "response" && narration ? { type, text, narration } : { type, text }; + /** * Append a text or thinking block to a render block list, merging - * with the previous block when the types match. + * with the previous block when the types match. Text never merges + * across a provider-executed call or between narration and an answer. */ export const appendTextBlock = ( blocks: RenderBlock[], type: "response" | "thinking", text: string, + narration = false, ): RenderBlock[] => { if (!text.trim()) { return blocks; } const nextBlocks = [...blocks]; const last = nextBlocks[nextBlocks.length - 1]; - if (last && last.type === type) { - nextBlocks[nextBlocks.length - 1] = { + const keepsApart = + last?.type === "response" && + (last.beforeProviderTool || (last.narration ?? false) !== narration); + if (last && last.type === type && !keepsApart) { + nextBlocks[nextBlocks.length - 1] = textBlock( type, - text: `${last.text}${text}`, - }; + `${last.text}${text}`, + narration, + ); return nextBlocks; } - nextBlocks.push({ type, text }); + nextBlocks.push(textBlock(type, text, narration)); return nextBlocks; }; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts index 54e17e07d6..482c936635 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts @@ -54,11 +54,12 @@ const isWorkspaceFileReferenceBlock = ( ): block is WorkspaceFileReferenceBlock => block.type === "workspace-file-reference"; +export const isProviderToolResult = (part: TypesGen.ChatMessagePart): boolean => + part.type === "tool-result" && part.provider_executed === true; + const isProviderToolResultOnlyMessage = ( parts: readonly TypesGen.ChatMessagePart[], -): boolean => - parts.length > 0 && - parts.every((part) => part.type === "tool-result" && part.provider_executed); +): boolean => parts.length > 0 && parts.every(isProviderToolResult); const isMetadataOnlyMessage = ( parts: readonly TypesGen.ChatMessagePart[], diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts index f366f4b40a..7bde510ac0 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageParsing.ts @@ -86,6 +86,22 @@ export const ensureToolBlock = ( return [...blocks, { type: "tool", id }]; }; +/** + * Provider-executed calls render no block, so without this mark the text + * before a web search would render as the answer. It skips citations that + * streamed after the text. + */ +export const markTextBeforeProviderTool = ( + blocks: RenderBlock[], +): RenderBlock[] => { + const index = blocks.findLastIndex((block) => block.type !== "sources"); + const block = blocks[index]; + if (block?.type !== "response" || block.beforeProviderTool) { + return blocks; + } + return blocks.with(index, { ...block, beforeProviderTool: true }); +}; + const isToolCallPart = ( part: TypesGen.ChatMessagePart, ): part is TypesGen.ChatToolCallPart => part.type === "tool-call"; @@ -226,7 +242,12 @@ export const parseMessageContent = ( switch (part.type) { case "text": { parsed.markdown = appendText(parsed.markdown, part.text); - parsed.blocks = appendTextBlock(parsed.blocks, "response", part.text); + parsed.blocks = appendTextBlock( + parsed.blocks, + "response", + part.text, + part.narration, + ); break; } case "reasoning": { @@ -240,6 +261,7 @@ export const parseMessageContent = ( // tool card UI and let the sources component render // their results. if (part.provider_executed) { + parsed.blocks = markTextBeforeProviderTool(parsed.blocks); break; } const id = part.tool_call_id || `tool-call-${index}`; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/storyFixtures.ts b/site/src/pages/AgentsPage/components/ChatConversation/storyFixtures.ts index f0eab5f2a1..b321c48a24 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/storyFixtures.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/storyFixtures.ts @@ -123,6 +123,7 @@ export const MockWorkingBlock: WorkingBlock = { startedAt: FIXTURE_NOW - 12_000, endedAt: FIXTURE_NOW, stepCount: 2, + endsWithAnswer: false, isLive: false, isPartial: false, }; @@ -187,6 +188,137 @@ export const MockWorkingMessages: TypesGen.ChatMessage[] = [ }, ]; +/** + * MockWorkingMessages whose final answer also reasoned, narrated, and searched + * the web from 13s to 16s before writing its text. + */ +export const MockWebSearchAnswerMessages: TypesGen.ChatMessage[] = [ + ...MockWorkingMessages.slice(0, 5), + { + ...MockChatMessage, + id: 6, + role: "assistant", + created_at: workingFixtureTime(16), + content: [ + { + type: "reasoning", + text: "Checking the release notes", + created_at: workingFixtureTime(13), + completed_at: workingFixtureTime(14), + }, + { type: "text", text: "Looking up the release notes." }, + { + type: "tool-call", + tool_call_id: "search", + tool_name: "web_search", + args: { query: "coder release notes" }, + provider_executed: true, + created_at: workingFixtureTime(14), + }, + { + type: "tool-result", + tool_call_id: "search", + tool_name: "web_search", + result: {}, + provider_executed: true, + created_at: workingFixtureTime(15), + }, + { + type: "source", + url: "https://changelog.example.com", + title: "Coder changelog", + }, + { + type: "source", + url: "https://releases.example.com", + title: "Coder releases", + }, + { + type: "reasoning", + text: "Comparing the two sources", + created_at: workingFixtureTime(15), + completed_at: workingFixtureTime(16), + }, + { + type: "text", + text: "The workspace runs the latest Coder release.", + }, + ], + }, +]; + +/** + * MockWorkingMessages whose final answer searched the web from 14s to 15s and + * cited nothing. + */ +export const MockUncitedSearchAnswerMessages: TypesGen.ChatMessage[] = [ + ...MockWorkingMessages.slice(0, 5), + { + ...MockChatMessage, + id: 6, + role: "assistant", + created_at: workingFixtureTime(16), + content: [ + { + type: "tool-call", + tool_call_id: "search", + tool_name: "web_search", + args: { query: "coder release notes" }, + provider_executed: true, + created_at: workingFixtureTime(14), + }, + { + type: "tool-result", + tool_call_id: "search", + tool_name: "web_search", + result: {}, + provider_executed: true, + created_at: workingFixtureTime(15), + }, + { type: "text", text: "No release notes mention the workspace." }, + ], + }, +]; + +/** + * A turn without tools whose answer narrates, then searches the web from 1s to + * 3s before writing its text, with no reasoning. + */ +export const MockLeadInSearchMessages: TypesGen.ChatMessage[] = [ + MockWorkingMessages[0], + { + ...MockChatMessage, + id: 2, + role: "assistant", + created_at: workingFixtureTime(4), + content: [ + { type: "text", text: "I'll look up the latest Coder release." }, + { + type: "tool-call", + tool_call_id: "search", + tool_name: "web_search", + args: { query: "latest coder release" }, + provider_executed: true, + created_at: workingFixtureTime(1), + }, + { + type: "source", + url: "https://releases.example.com", + title: "Coder releases", + }, + { + type: "tool-result", + tool_call_id: "search", + tool_name: "web_search", + result: {}, + provider_executed: true, + created_at: workingFixtureTime(3), + }, + { type: "text", text: "The latest Coder release is v2.30." }, + ], + }, +]; + /** A question the agent asks after the first step of MockWorkingMessages. */ export const MockQuestionCallMessage: TypesGen.ChatMessage = { ...MockChatMessage, diff --git a/site/src/pages/AgentsPage/components/ChatConversation/streamState.test.ts b/site/src/pages/AgentsPage/components/ChatConversation/streamState.test.ts index 9a9d726bc0..b557df0267 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/streamState.test.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/streamState.test.ts @@ -716,17 +716,20 @@ describe("applyMessagePartToStreamState", () => { expect(prev.toolCalls).toEqual({}); }); - it("skips provider_executed tool-result parts", () => { - const prev = createEmptyStreamState(); - const result = applyMessagePartToStreamState(prev, { + it("records a provider_executed tool-result part without a tool card", () => { + const part: ChatMessagePart = { type: "tool-result", tool_name: "web_search", tool_call_id: "tc-1", provider_executed: true, result: { output: "search results" }, + }; + const state = applyMessagePartToStreamState(null, part); + expect(state).toEqual({ + ...createEmptyStreamState(), + providerToolRan: true, }); - expect(result).toBe(prev); - expect(prev.toolResults).toEqual({}); + expect(applyMessagePartToStreamState(state, part)).toBe(state); }); it("adds a file block from a file part with data", () => { diff --git a/site/src/pages/AgentsPage/components/ChatConversation/streamState.ts b/site/src/pages/AgentsPage/components/ChatConversation/streamState.ts index e18010d172..a11f6de9c4 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/streamState.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/streamState.ts @@ -3,6 +3,7 @@ import { appendTextBlock } from "./blockUtils"; import { ensureToolBlock, getToolResultStatus, + markTextBeforeProviderTool, parseToolResultIsError, } from "./messageParsing"; import { mergeStreamPayload } from "./streamingJson"; @@ -33,7 +34,12 @@ export const applyMessagePartToStreamState = ( } return { ...nextState, - blocks: appendTextBlock(nextState.blocks, "response", part.text), + blocks: appendTextBlock( + nextState.blocks, + "response", + part.text, + part.narration, + ), }; } case "reasoning": { @@ -51,7 +57,11 @@ export const applyMessagePartToStreamState = ( // handled natively by the provider — skip rendering them // as tool cards. if (part.provider_executed) { - return prev; + const blocks = markTextBeforeProviderTool(nextState.blocks); + const startedAt = nextState.startedAt ?? part.created_at; + return blocks === nextState.blocks && startedAt === nextState.startedAt + ? prev + : { ...nextState, startedAt, blocks }; } const existingByName = Object.values(nextState.toolCalls).find( (call) => call.name === part.tool_name, @@ -96,9 +106,13 @@ export const applyMessagePartToStreamState = ( }; } case "tool-result": { - // Skip synthetic results for provider-executed tools. + // Provider-executed results render no card, but the search they end + // is a step. Recording it may create an empty state, which still + // shows the Thinking indicator. if (part.provider_executed) { - return prev; + return nextState.providerToolRan + ? nextState + : { ...nextState, providerToolRan: true }; } const existingByName = Object.values(nextState.toolResults).find( (result) => result.name === part.tool_name, diff --git a/site/src/pages/AgentsPage/components/ChatConversation/types.ts b/site/src/pages/AgentsPage/components/ChatConversation/types.ts index 74628a82d6..2ab768ec23 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/types.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/types.ts @@ -44,6 +44,13 @@ export type RenderBlock = | { type: "response"; text: string; + /** + * A provider-executed call, such as web search, followed this text, + * so later text starts a new block. + */ + beforeProviderTool?: boolean; + /** The model labeled this text as commentary on its work. */ + narration?: boolean; } | { type: "thinking"; @@ -121,4 +128,9 @@ export type StreamState = { toolResults: Record; sources: Array<{ url: string; title: string }>; startedAt?: string; + /** + * Set by a provider-executed result, so a search counts as a step even + * before or without citations. + */ + providerToolRan?: boolean; }; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/workingBlockGrouping.test.ts b/site/src/pages/AgentsPage/components/ChatConversation/workingBlockGrouping.test.ts index afbf667210..471014b3bb 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/workingBlockGrouping.test.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/workingBlockGrouping.test.ts @@ -11,6 +11,7 @@ import { assignTimelineRows, type TimelineRow } from "./timelineRows"; import { type GroupWorkingBlocksOptions, groupWorkingBlocks, + splitRowBlocks, type WorkingBlock, } from "./workingBlockGrouping"; @@ -32,6 +33,11 @@ const text = (value: string): TypesGen.ChatMessagePart => ({ type: "text", text: value, }); +const narration = (value: string): TypesGen.ChatMessagePart => ({ + type: "text", + text: value, + narration: true, +}); const reasoning = ( value: string, createdAt?: string, @@ -42,6 +48,32 @@ const reasoning = ( created_at: createdAt, completed_at: completedAt, }); +const source = (url: string, title: string): TypesGen.ChatMessagePart => ({ + type: "source", + url, + title, +}); +const citation = source("https://go.example.com", "Go"); +const searchCall = ( + id: string, + createdAt?: string, +): TypesGen.ChatMessagePart => ({ + type: "tool-call", + tool_call_id: id, + tool_name: "web_search", + provider_executed: true, + created_at: createdAt, +}); +const searchResult = ( + id: string, + createdAt?: string, +): TypesGen.ChatMessagePart => ({ + type: "tool-result", + tool_call_id: id, + tool_name: "web_search", + provider_executed: true, + created_at: createdAt, +}); const call = ( id: string, createdAt?: string, @@ -164,11 +196,133 @@ describe("groupWorkingBlocks", () => { expect(rowIds(rows, blocks[0].rowIndices)).toEqual([ steps[0].id, steps[2].id, + answer.id, ]); // Reasoning start counts toward the wall-clock span. expect(blocks[0].startedAt).toBe(WORKING_FIXTURE_START + 500); }); + it("folds the final answer's reasoning and web search into the block it ends", () => { + const prompt = user("Go"); + const steps = [...step("a", 1, 2), ...step("b", 3, 4)]; + const answer = message( + "assistant", + [ + reasoning("Check the docs", at(5), at(9)), + source("https://example.com/a", "A"), + source("https://example.com/b", "B"), + text("Done."), + ], + at(10), + ); + const { rows, blocks } = group([prompt, ...steps, answer]); + + expect(blocks).toHaveLength(1); + expect(rowIds(rows, blocks[0].rowIndices)).toEqual([ + steps[0].id, + steps[2].id, + answer.id, + ]); + expect(blocks[0]).toMatchObject({ + stepCount: 3, + endsWithAnswer: true, + endedAt: WORKING_FIXTURE_START + 9000, + key: `working:through:message:${answer.id}`, + }); + }); + + it("treats an answer whose citations trail its text as the block's answer", () => { + const prompt = user("Go"); + const steps = step("a", 1, 2); + const answer = message("assistant", [ + text("Done."), + source("https://example.com", "Example"), + ]); + const { blocks } = group([prompt, ...steps, answer]); + + expect(blocks[0]).toMatchObject({ stepCount: 2, endsWithAnswer: true }); + }); + + it("starts a one-step block from an answer that searched in a turn without tools", () => { + const prompt = user("Go"); + const answer = message("assistant", [ + reasoning("Look it up"), + source("https://example.com", "Example"), + text("Found it."), + ]); + const { rows, blocks } = group([prompt, answer]); + + expect(blocks).toHaveLength(1); + expect(rowIds(rows, blocks[0].rowIndices)).toEqual([answer.id]); + expect(blocks[0]).toMatchObject({ stepCount: 1, endsWithAnswer: true }); + }); + + it("counts a reissued search once, ignoring its unanswered first call", () => { + const prompt = user("Go"); + const steps = step("a", 1, 2, { leading: [searchCall("unanswered")] }); + const answer = message("assistant", [ + searchCall("search"), + citation, + searchResult("search"), + text("Go 1.27 is out."), + ]); + const { blocks } = group([prompt, ...steps, answer]); + + expect(blocks).toMatchObject([{ stepCount: 2, endsWithAnswer: true }]); + }); + + it.each([ + { name: "no answer", answers: [] }, + { + name: "a cited answer", + answers: [[text("Go 1.27"), citation, text(" is out."), citation]], + }, + ])( + "keeps the step of a search an interrupt committed apart from its row with $name", + ({ answers }) => { + const prompt = user("Go"); + const steps = step("a", 1, 2); + // chatd commits the call and result as separate messages that render + // no row. + const search = [ + message("assistant", [searchCall("s", at(3))]), + message("tool", [searchResult("s", at(4))]), + ]; + const after = answers.map((parts) => message("assistant", parts)); + + expect( + group([prompt, ...steps, ...search, ...after]).blocks, + ).toMatchObject([{ stepCount: 2 }]); + }, + ); + + it("counts a search that rendered no row before the block's first step", () => { + const prompt = user("Go"); + // The answer's own search stays out of the block that follows it. + const answer = message("assistant", [ + searchCall("first", at(1)), + searchResult("first", at(2)), + text("Nothing found."), + ]); + const search = message("assistant", [ + searchCall("second", at(3)), + searchResult("second", at(4)), + ]); + const steps = step("a", 5, 6); + + expect(group([prompt, answer, search, ...steps]).blocks).toMatchObject([ + { stepCount: 2, startedAt: WORKING_FIXTURE_START + 3000 }, + ]); + }); + + it("leaves an answer's reasoning unfolded in a turn without steps", () => { + const prompt = user("Go"); + const answer = message("assistant", [reasoning("Easy"), text("Done.")]); + const { blocks } = group([prompt, answer]); + + expect(blocks).toEqual([]); + }); + it("leaves a standalone reasoning-only row unfolded", () => { const prompt = user("Go"); const thinking = message("assistant", [reasoning("Just thinking", at(1))]); @@ -194,6 +348,68 @@ describe("groupWorkingBlocks", () => { expect(blocks[0].stepCount).toBe(2); }); + it("folds a web search row that sits between tool steps", () => { + const prompt = user("Go"); + const first = step("a", 1, 2); + const search = message("assistant", [ + reasoning("Search"), + source("https://example.com", "Example"), + ]); + const second = step("b", 5, 6); + const { rows, blocks } = group([prompt, ...first, search, ...second]); + + expect(blocks).toHaveLength(1); + expect(rowIds(rows, blocks[0].rowIndices)).toEqual([ + first[0].id, + search.id, + second[0].id, + ]); + expect(blocks[0]).toMatchObject({ stepCount: 3, endsWithAnswer: false }); + }); + + it("starts a new block after an answer row closes one", () => { + const prompt = user("Go"); + const first = step("a", 1, 2); + const interlude = message("assistant", [ + reasoning("Halfway"), + text("Halfway there."), + ]); + const second = step("b", 4, 5); + const { rows, blocks } = group([prompt, ...first, interlude, ...second]); + + expect(blocks).toHaveLength(2); + expect(rowIds(rows, blocks[0].rowIndices)).toEqual([ + first[0].id, + interlude.id, + ]); + expect(blocks[0].endsWithAnswer).toBe(true); + expect(rowIds(rows, blocks[1].rowIndices)).toEqual([second[0].id]); + }); + + it("counts a search between two blocks once", () => { + const prompt = user("Go"); + const first = step("a", 1, 2); + const interlude = message("assistant", [ + reasoning("Halfway"), + text("Halfway there."), + ]); + const search = [ + message("assistant", [searchCall("s", at(3))]), + message("tool", [searchResult("s", at(4))]), + ]; + const second = step("b", 5, 6); + const { blocks } = group([ + prompt, + ...first, + interlude, + ...search, + ...second, + ]); + + expect(blocks).toHaveLength(2); + expect(blocks.reduce((total, block) => total + block.stepCount, 0)).toBe(3); + }); + it("splits a turn at an interleaved answer row", () => { const prompt = user("Go"); const first = step("a", 1, 2); @@ -414,6 +630,256 @@ describe("groupWorkingBlocks", () => { expect(blocks[0].isLive).toBe(true); }); + it("keeps a live answer row with reasoning in the live block", () => { + const prompt = user("Go"); + const steps = step("a", 1, 2); + const { rows, blocks } = groupLive( + [prompt, ...steps], + [reasoning("Wrap up", at(3)), text("Here is what I found")], + ); + + expect(blocks).toHaveLength(1); + expect(rowIds(rows, blocks[0].rowIndices)).toEqual([steps[0].id, "live"]); + expect(blocks[0]).toMatchObject({ isLive: true, endsWithAnswer: true }); + }); + + it("folds an answer's search without citations into the block it ends", () => { + const prompt = user("Go"); + const steps = step("a", 1, 2); + const parts = [ + searchCall("s", at(3)), + searchResult("s", at(4)), + text("Nothing new."), + ]; + const answer = message("assistant", parts); + const live = groupLive([prompt, ...steps], parts); + const persisted = group([prompt, ...steps, answer]); + + expect(rowIds(live.rows, live.blocks[0].rowIndices)).toEqual([ + steps[0].id, + "live", + ]); + expect(live.blocks).toMatchObject([ + { stepCount: 2, endsWithAnswer: true, isLive: true }, + ]); + expect(rowIds(persisted.rows, persisted.blocks[0].rowIndices)).toEqual([ + steps[0].id, + answer.id, + ]); + expect(persisted.blocks).toMatchObject([ + { + stepCount: 2, + endsWithAnswer: true, + endedAt: WORKING_FIXTURE_START + 4000, + }, + ]); + }); + + it("starts no block from a search without citations alone", () => { + const prompt = user("Go"); + const parts = [searchCall("s", at(1)), searchResult("s"), text("Hi.")]; + + for (const index of parts.keys()) { + expect(groupLive([prompt], parts.slice(0, index + 1)).blocks).toEqual( + [], + ); + } + expect(group([prompt, message("assistant", parts)]).blocks).toEqual([]); + }); + + it("unfolds a tool-less live turn's reasoning once its answer starts", () => { + const { blocks } = groupLive( + [user("Go")], + [reasoning("Planning", at(1)), text("Here you go")], + ); + + expect(blocks).toEqual([]); + }); + + it("folds labeled narration into the live block from its first delta", () => { + const prompt = user("Go"); + const parts = [ + reasoning("Planning", at(1)), + narration("Now I'll "), + narration("update the tests."), + call("a", at(2)), + ]; + + for (const index of parts.keys()) { + const { rows, blocks } = groupLive([prompt], parts.slice(0, index + 1)); + expect(blocks).toMatchObject([ + { + key: `working:live:message:${prompt.id}:0`, + endsWithAnswer: false, + }, + ]); + expect(rowIds(rows, blocks[0].rowIndices)).toEqual(["live"]); + } + }); + + it("keeps a tool-less turn's labeled narration folded above its answer", () => { + const prompt = user("Go"); + const parts = [narration("Checking the docs."), text("They cover it.")]; + const live = groupLive([prompt], parts); + const persisted = group([prompt, message("assistant", parts)]); + + for (const { blocks } of [live, persisted]) { + expect(blocks).toMatchObject([{ stepCount: 0, endsWithAnswer: true }]); + } + }); + + // OpenAI streams a search's citations inside the answer text that follows + // it, and stores them before that text. + it.each([ + { + name: "reasoning before the search", + live: [ + reasoning("Look it up", at(1)), + searchCall("a"), + searchResult("a"), + text("Go 1.27"), + citation, + text(" is out."), + ], + persisted: [ + reasoning("Look it up", at(1), at(2)), + searchCall("a"), + searchResult("a"), + citation, + text("Go 1.27 is out."), + ], + stepCount: 1, + }, + { + name: "the search before its reasoning", + live: [ + searchCall("a", at(1)), + searchResult("a"), + reasoning("Compare", at(2)), + text("Go 1.27"), + citation, + text(" is out."), + ], + persisted: [ + searchCall("a", at(1)), + searchResult("a"), + reasoning("Compare", at(2), at(3)), + citation, + text("Go 1.27 is out."), + ], + stepCount: 1, + }, + { + name: "narration before the search", + live: [ + text("Searching."), + searchCall("a", at(1)), + searchResult("a"), + text("Go 1.27"), + citation, + text(" is out."), + ], + persisted: [ + text("Searching."), + searchCall("a", at(1)), + searchResult("a"), + citation, + text("Go 1.27 is out."), + ], + stepCount: 1, + }, + { + name: "two searches", + live: [ + reasoning("Look it up", at(1)), + searchCall("a"), + searchResult("a"), + searchCall("b"), + searchResult("b"), + text("Go 1.27"), + citation, + text(" is out."), + ], + persisted: [ + reasoning("Look it up", at(1), at(2)), + searchCall("a"), + searchResult("a"), + searchCall("b"), + searchResult("b"), + citation, + text("Go 1.27 is out."), + ], + stepCount: 2, + }, + { + name: "a search without citations", + live: [ + reasoning("Look it up", at(1)), + searchCall("a"), + searchResult("a"), + text("Nothing new."), + ], + persisted: [ + reasoning("Look it up", at(1), at(2)), + searchCall("a"), + searchResult("a"), + text("Nothing new."), + ], + stepCount: 1, + }, + ])( + "keeps one search block through streaming and persistence with $name", + ({ live, persisted, stepCount }) => { + const prompt = user("Go"); + const liveKey = `working:live:message:${prompt.id}:0`; + const prefixBlocks = live.map( + (_, index) => groupLive([prompt], live.slice(0, index + 1)).blocks, + ); + const firstBlockIndex = prefixBlocks.findIndex( + (blocks) => blocks.length > 0, + ); + + expect(firstBlockIndex).not.toBe(-1); + for (const blocks of prefixBlocks.slice(firstBlockIndex)) { + expect(blocks).toMatchObject([ + { + key: liveKey, + isLive: true, + startedAt: WORKING_FIXTURE_START + 1000, + }, + ]); + } + expect( + group([prompt, message("assistant", persisted)]).blocks, + ).toMatchObject([ + { + liveKey, + stepCount, + endsWithAnswer: true, + isLive: false, + startedAt: WORKING_FIXTURE_START + 1000, + }, + ]); + }, + ); + + it("completes a block once its answer row persists while the chat still runs", () => { + const prompt = user("Go"); + const steps = step("a", 1, 2); + const answer = message( + "assistant", + [reasoning("Wrap up", at(3), at(4)), text("Done.")], + at(5), + ); + const { blocks } = group([prompt, ...steps, answer], { isWorking: true }); + + expect(blocks).toHaveLength(1); + expect(blocks[0]).toMatchObject({ + isLive: false, + endedAt: WORKING_FIXTURE_START + 4000, + }); + }); + it("folds an idle live row into the block it follows", () => { const prompt = user("Go"); const steps = step("a", 1, 2); @@ -587,3 +1053,145 @@ describe("groupWorkingBlocks", () => { }); }); }); + +describe("splitRowBlocks", () => { + const answerOf = (content: TypesGen.ChatMessagePart[]) => { + const [{ parsed }] = parseMessagesWithMergedTools([ + message("assistant", content), + ]); + return splitRowBlocks(parsed.blocks, parsed.tools).answer; + }; + + it("folds narration that precedes the answer's last reasoning", () => { + expect( + answerOf([ + reasoning("Plan"), + text("Looking it up."), + source("https://example.com", "Example"), + reasoning("Compare"), + text("Done."), + ]), + ).toEqual([{ type: "response", text: "Done." }]); + }); + + // Persisted rows store text at its end, after the citations streamed in it. + it.each([ + { + name: "narration before search results", + persisted: [ + text("Searching."), + searchCall("a"), + citation, + searchResult("a"), + text("Go 1.27 is out."), + ], + live: [ + text("Searching."), + searchCall("a"), + citation, + searchResult("a"), + text("Go 1.27 is out."), + ], + answer: "Go 1.27 is out.", + }, + { + name: "narration before cited text", + persisted: [ + text("Searching."), + searchCall("a"), + searchResult("a"), + citation, + text("Go 1.27 is out. Rust 1.98 is out."), + ], + live: [ + text("Searching."), + searchCall("a"), + searchResult("a"), + text("Go 1.27 is out."), + citation, + text(" Rust 1.98 is out."), + ], + answer: "Go 1.27 is out. Rust 1.98 is out.", + }, + { + name: "cited text before a search", + persisted: [ + citation, + text("Go 1.27 is out."), + searchCall("a"), + searchResult("a"), + text("Rust 1.98 is out."), + ], + live: [ + text("Go 1.27 is out."), + citation, + searchCall("a"), + searchResult("a"), + text("Rust 1.98 is out."), + ], + answer: "Rust 1.98 is out.", + }, + { + name: "cited text without a search call", + persisted: [citation, text("Go 1.27 is out. Rust 1.98 is out.")], + live: [text("Go 1.27 is out."), citation, text(" Rust 1.98 is out.")], + answer: "Go 1.27 is out. Rust 1.98 is out.", + }, + { + name: "labeled narration before the answer", + persisted: [narration("Checking the docs."), text("They cover it.")], + live: [ + narration("Checking "), + narration("the docs."), + text("They cover it."), + ], + answer: "They cover it.", + }, + ])( + "splits $name the same live and persisted", + ({ persisted, live, answer }) => { + const { streamState, streamTools } = buildStreamRenderState(live); + if (!streamState) { + throw new Error("The live parts built no stream state."); + } + const expected = [{ type: "response", text: answer }]; + + expect(answerOf(persisted)).toEqual(expected); + expect(splitRowBlocks(streamState.blocks, streamTools).answer).toEqual( + expected, + ); + }, + ); + + const hiddenCall = (id: string): TypesGen.ChatMessagePart => ({ + type: "tool-call", + tool_call_id: id, + tool_name: "execute", + args: { command: "" }, + }); + + it("ignores hidden tools when splitting the answer", () => { + expect( + answerOf([ + reasoning("Plan"), + hiddenCall("before"), + text("Done."), + hiddenCall("after"), + ]), + ).toEqual([{ type: "response", text: "Done." }]); + }); + + it("keeps answer text on either side of a hidden tool apart", () => { + expect( + answerOf([ + reasoning("Plan"), + text("Checked the logs."), + hiddenCall("between"), + text("The fix is in."), + ]), + ).toEqual([ + { type: "response", text: "Checked the logs." }, + { type: "response", text: "The fix is in." }, + ]); + }); +}); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/workingBlockGrouping.ts b/site/src/pages/AgentsPage/components/ChatConversation/workingBlockGrouping.ts index 8b24c48f1f..a8f809b056 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/workingBlockGrouping.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/workingBlockGrouping.ts @@ -1,4 +1,6 @@ -import { getVisibleContent } from "./messageHelpers"; +import type * as TypesGen from "#/api/typesGenerated"; +import { appendTextBlock } from "./blockUtils"; +import { getVisibleContent, isProviderToolResult } from "./messageHelpers"; import type { TimelineRow } from "./timelineRows"; import type { MergedTool, @@ -22,8 +24,10 @@ export type WorkingBlock = { * at the front when history is prepended into that row. */ memberIds: number[]; - /** Distinct visible tools, not rows. */ + /** Distinct visible tools plus web searches, not rows. */ stepCount: number; + /** The last row's answer renders after the block; only its work folds in. */ + endsWithAnswer: boolean; isLive: boolean; /** Unloaded older history may hold earlier rows of this block. */ isPartial: boolean; @@ -53,19 +57,74 @@ const UNCOLLAPSIBLE_TOOLS: ReadonlySet = new Set([ "chat_cleared", ]); -type RowContent = ReturnType; +/** + * A row that ends a working block renders its work inside the block and its + * answer after it. + */ +export type RowSection = "work" | "answer"; /** - * A step row is assistant output that ends in tool activity or reasoning - * rather than an answer. Text that precedes a tool call is narration and - * folds with it; text that ends a row is an answer and stays visible. A - * live row with no output yet is the turn working on its next step. + * The answer is the content after a row's last reasoning, tool call, or + * labeled narration, web searches included; text before it is narration. + * Sources fold without splitting the answer, since OpenAI streams citations + * between the deltas of one text part. */ -const getStepRowContent = ( +export const splitRowBlocks = ( + blocks: readonly RenderBlock[], + tools: readonly MergedTool[], +) => { + const visible = new Set(getVisibleContent(blocks, tools).visibleBlocks); + const lastWorkIndex = blocks.findLastIndex( + (block) => + visible.has(block) && + (block.type === "thinking" || + block.type === "tool" || + (block.type === "response" && + (block.beforeProviderTool || block.narration))), + ); + const work: RenderBlock[] = []; + let answer: RenderBlock[] = []; + let previous: RenderBlock | undefined; + for (const [index, block] of blocks.entries()) { + if ( + index <= lastWorkIndex || + !visible.has(block) || + block.type === "sources" + ) { + work.push(block); + } else if (block.type === "response" && previous?.type === "response") { + answer = appendTextBlock(answer, "response", block.text); + } else { + answer = [...answer, block]; + } + if (block.type !== "sources") { + previous = block; + } + } + + return { work, answer } satisfies Record; +}; + +type RowContent = ReturnType; + +type MemberRow = { + content: RowContent; + endsWithAnswer: boolean; + showsWork: boolean; + narrates: boolean; +}; + +/** + * A row ending in an answer joins only when it also did work, even a search + * without citations, and then closes the block. A live row with no output yet + * is the turn working on its next step. + */ +const getMemberRow = ( row: TimelineRow, options: GroupWorkingBlocksOptions, -): RowContent | undefined => { +): MemberRow | undefined => { let content: RowContent; + let searched: boolean; if (row.type === "live") { if (!options.isLiveRowCollapsible) { @@ -73,6 +132,7 @@ const getStepRowContent = ( } content = getVisibleContent(options.liveBlocks, options.liveTools); + searched = options.streamState?.providerToolRan ?? false; } else { const { message, parsed } = row.entry; if (message.role !== "assistant" || parsed.hookNotices.length > 0) { @@ -80,11 +140,14 @@ const getStepRowContent = ( } content = getVisibleContent(parsed.blocks, parsed.tools); + searched = (message.content ?? []).some(isProviderToolResult); } const { visibleBlocks, visibleTools } = content; if (visibleBlocks.length === 0) { - return row.type === "live" ? content : undefined; + return row.type === "live" + ? { content, endsWithAnswer: false, showsWork: false, narrates: false } + : undefined; } if (visibleTools.some((tool) => UNCOLLAPSIBLE_TOOLS.has(tool.name))) { @@ -100,12 +163,17 @@ const getStepRowContent = ( return undefined; } - const last = visibleBlocks[visibleBlocks.length - 1]; - if (last.type !== "tool" && last.type !== "thinking") { - return undefined; + const { work, answer } = splitRowBlocks(visibleBlocks, visibleTools); + const narrates = work.some( + (block) => block.type === "response" && block.narration, + ); + if (answer.length === 0) { + return { content, endsWithAnswer: false, showsWork: true, narrates }; } - return content; + return work.length > 0 || searched + ? { content, endsWithAnswer: true, showsWork: work.length > 0, narrates } + : undefined; }; /** @@ -126,6 +194,13 @@ const getPartTimestamps = (entry: ParsedMessageEntry): string[] => }) .filter((timestamp) => timestamp !== undefined); +/** + * Results, not calls: a step can end with a provider call unanswered, and the + * next step issues that call again. + */ +const countProviderResults = (message: TypesGen.ChatMessage): number => + (message.content ?? []).filter(isProviderToolResult).length; + const rowMessageIds = (row: TimelineRow): readonly number[] => row.type === "live" ? [] : (row.entry.mergedFrom ?? [row.entry.message.id]); @@ -141,9 +216,12 @@ export const groupWorkingBlocks = ( type Draft = { rowIndices: number[]; toolIds: Set; + sourceBlocks: number; + endsWithAnswer: boolean; anchorKey?: string; ordinal: number; containsLiveRow: boolean; + narrates: boolean; }; const drafts: Draft[] = []; @@ -152,8 +230,8 @@ export const groupWorkingBlocks = ( let ordinal = 0; for (const [index, row] of rows.entries()) { - const content = getStepRowContent(row, options); - if (!content) { + const member = getMemberRow(row, options); + if (!member) { current = undefined; if (row.type === "message" && row.entry.message.role !== "assistant") { anchorKey = row.key; @@ -163,19 +241,22 @@ export const groupWorkingBlocks = ( } if (!current) { - // The stream opens empty before every step. That row extends a - // block that is already working but never starts one, so a turn's - // first moments keep the plain thinking indicator. - if (content.visibleBlocks.length === 0) { + // Rows that render nothing, like the empty stream before each step or + // an uncited search, extend a block but never start one: a turn's + // first moments keep the thinking indicator, and no block is empty. + if (!member.showsWork) { continue; } current = { rowIndices: [], toolIds: new Set(), + sourceBlocks: 0, + endsWithAnswer: false, anchorKey, ordinal, containsLiveRow: false, + narrates: false, }; ordinal += 1; drafts.push(current); @@ -183,19 +264,20 @@ export const groupWorkingBlocks = ( current.rowIndices.push(index); current.containsLiveRow ||= row.type === "live"; + current.narrates ||= member.narrates; - for (const tool of content.visibleTools) { + for (const tool of member.content.visibleTools) { current.toolIds.add(tool.id); } - } + current.sourceBlocks += member.content.visibleBlocks.filter( + (block) => block.type === "sources", + ).length; - // A completed block is a run of tool activity; a reasoning-only row on - // its own stays visible. The live turn folds from its first reasoning, - // so thinking never shows and then vanishes once a tool call arrives. - const blockDrafts = drafts.filter( - (draft) => - draft.toolIds.size > 0 || (draft.containsLiveRow && options.isTurnActive), - ); + if (member.endsWithAnswer) { + current.endsWithAnswer = true; + current = undefined; + } + } const lastMessageRowIndex = rows.findLastIndex( (row) => row.type === "message", @@ -212,58 +294,89 @@ export const groupWorkingBlocks = ( return Number.POSITIVE_INFINITY; }; - // Entries and blocks are both in ascending message ID order and block - // spans never overlap, so one cursor walks the entries once. - let entryIndex = 0; + // Each span starts where the previous one ended, so no entry counts toward + // two blocks. + let spanStartId = Number.NEGATIVE_INFINITY; + const blocks: WorkingBlock[] = []; - return blockDrafts.map((draft) => { + for (const draft of drafts) { const firstRowIndex = draft.rowIndices[0]; const lastRowIndex = draft.rowIndices[draft.rowIndices.length - 1]; const memberIds = draft.rowIndices.flatMap((i) => rowMessageIds(rows[i])); + // A persisted answer completes its block even while the chat still runs. const isLive = options.isWorking && + !(draft.endsWithAnswer && rows[lastRowIndex].type === "message") && (draft.containsLiveRow || lastRowIndex >= lastMessageRowIndex); - // The span covers hidden tool-result messages up to the next row. + // The span covers hidden tool-result messages up to the next row and, + // before the first row, searches that rendered nothing since the + // previous row. const fromId = Math.min(...memberIds); const toId = messageIdAfter(lastRowIndex); - while ( - entryIndex < entries.length && - entries[entryIndex].message.id < fromId - ) { - entryIndex++; - } + const previousRowId = Math.max( + ...(firstRowIndex > 0 ? rowMessageIds(rows[firstRowIndex - 1]) : []), + ); - const spanTimestamps: string[] = []; - while ( - entryIndex < entries.length && - entries[entryIndex].message.id < toId - ) { - spanTimestamps.push(...getPartTimestamps(entries[entryIndex])); - entryIndex++; + const spanEntries = entries.filter(({ message }) => { + const { id } = message; + if (id < spanStartId || id >= toId) { + return false; + } + return ( + id >= fromId || + (id > previousRowId && countProviderResults(message) > 0) + ); + }); + spanStartId = toId; + + // Until the step persists, one flag stands in for its searches. + const searches = spanEntries.reduce( + (count, { message }) => count + countProviderResults(message), + draft.containsLiveRow && options.streamState?.providerToolRan ? 1 : 0, + ); + + // Source blocks count as searches only when no provider result does: + // some providers cite without a search, and an interrupt commits a + // search's result apart from the answer citing it. + const stepCount = draft.toolIds.size + (searches || draft.sourceBlocks); + + // Completed reasoning alone stays visible. The live turn folds from its + // first reasoning, so thinking never shows and then vanishes once a tool + // call arrives, and unfolds only when a turn without steps answers. + // Labeled narration keeps its block so it never reads as the answer. + const foldsLiveTurn = + draft.containsLiveRow && options.isTurnActive && !draft.endsWithAnswer; + const keepsBlock = stepCount > 0 || foldsLiveTurn || draft.narrates; + if (!keepsBlock) { + continue; } const streamStartedAt = options.streamState?.startedAt; - if (draft.containsLiveRow && streamStartedAt !== undefined) { - spanTimestamps.push(streamStartedAt); - } - - const times = spanTimestamps.map((timestamp) => Date.parse(timestamp)); + const times = [ + ...spanEntries.flatMap(getPartTimestamps), + ...(draft.containsLiveRow && streamStartedAt !== undefined + ? [streamStartedAt] + : []), + ].map((timestamp) => Date.parse(timestamp)); const liveKey = `working:live:${draft.anchorKey ?? "head"}:${draft.ordinal}`; const key = isLive ? liveKey : `working:through:${rows[lastRowIndex].key}`; - return { + blocks.push({ key, liveKey, rowIndices: draft.rowIndices, memberIds, - stepCount: draft.toolIds.size, + stepCount, + endsWithAnswer: draft.endsWithAnswer, isLive, isPartial: options.hasMoreMessages && firstRowIndex === 0, startedAt: times.length > 0 ? Math.min(...times) : undefined, endedAt: isLive || times.length === 0 ? undefined : Math.max(...times), - }; - }); + }); + } + + return blocks; };