From 027db81fd98af2c88ef1b1c33d37b64ca3bb648d Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 7 Jul 2026 12:01:09 +0000 Subject: [PATCH] fix(coderd/x/chatd/chatadvisor): textualize advisor prompt tool exchanges The nested advisor call defines no tools, but BuildAdvisorMessages forwarded raw tool_use/tool_result blocks from the parent conversation. Models mimic the forwarded pattern and spend the step committing to a tool call they cannot make, ending it with no text output. Fold each exchange into a plain-text user-role note instead, and include the finish reason and content-part kinds in the no-text-output error so the failure mode is diagnosable from field reports. --- coderd/x/chatd/chatadvisor/handoff.go | 111 +++++++++----- coderd/x/chatd/chatadvisor/runner.go | 50 ++++++- coderd/x/chatd/chatadvisor/runner_test.go | 168 ++++++++++++++++++---- 3 files changed, 266 insertions(+), 63 deletions(-) diff --git a/coderd/x/chatd/chatadvisor/handoff.go b/coderd/x/chatd/chatadvisor/handoff.go index 3fe311a8087ca..1d63f7c942f2f 100644 --- a/coderd/x/chatd/chatadvisor/handoff.go +++ b/coderd/x/chatd/chatadvisor/handoff.go @@ -2,6 +2,7 @@ package chatadvisor import ( "encoding/json" + "fmt" "maps" "slices" "strings" @@ -111,63 +112,101 @@ func BuildAdvisorMessages( remainingBudget -= messageBytes } slices.Reverse(recent) - recent = dropOrphanToolMessages(recent) + recent = textualizeToolExchanges(recent) messages = append(messages, recent...) messages = append(messages, textMessage(fantasy.MessageRoleUser, trimmedQuestion)) return messages } -// dropOrphanToolMessages removes tool-role messages whose tool-call references -// have been truncated out of the recent window. Providers reject prompts with -// tool_result blocks that do not have a matching tool_use, so a truncation cut -// that lands between an assistant tool-call message and its tool-result message -// would otherwise produce a provider error rather than advice. The backward -// walk always picks up tool results before their originating assistant -// message, so orphan results can only appear at the leading edge of the -// recent window. A single forward pass tracking known tool-call IDs is -// sufficient to drop them. -func dropOrphanToolMessages(recent []fantasy.Message) []fantasy.Message { - if len(recent) == 0 { - return recent +// textualizeToolExchanges rewrites tool activity as inline text notes. +// Assistant tool-call parts are removed, with their inputs folded into the +// note rendered for the matching tool result, and tool-role messages become +// user-role notes. The nested advisor call defines no tools, so +// assistant-authored tool artifacts in the transcript prime the model to +// imitate them instead of answering: raw tool_use/tool_result blocks yield +// an empty step ("advisor produced no text output"), and a bare +// "[tool call: ...]" text line yields that literal line back as advice. +// Folding each exchange into a single note leaves no assistant tool-call +// pattern to complete while keeping the activity visible, and it removes +// the provider requirement that tool_result blocks pair with a tool_use in +// the same request, so results whose calls were truncated out of the +// window can be kept instead of dropped. +func textualizeToolExchanges(recent []fantasy.Message) []fantasy.Message { + // Tool results carry only the call ID, so record each call's name and + // input as the forward walk scrubs assistant messages. + type callInfo struct { + name string + input string } - known := make(map[string]struct{}) + calls := make(map[string]callInfo) result := make([]fantasy.Message, 0, len(recent)) for _, msg := range recent { - if msg.Role == fantasy.MessageRoleAssistant { + switch msg.Role { + case fantasy.MessageRoleAssistant: + parts := make([]fantasy.MessagePart, 0, len(msg.Content)) for _, part := range msg.Content { call, ok := fantasy.AsMessagePart[fantasy.ToolCallPart](part) if !ok { + parts = append(parts, part) continue } - known[call.ToolCallID] = struct{}{} + calls[call.ToolCallID] = callInfo{name: call.ToolName, input: call.Input} } + if len(parts) == 0 { + // The message carried only tool calls; the folded + // result notes preserve the information. + continue + } + msg.Content = parts result = append(result, msg) - continue - } - if msg.Role != fantasy.MessageRoleTool { + case fantasy.MessageRoleTool: + parts := make([]fantasy.MessagePart, 0, len(msg.Content)) + for _, part := range msg.Content { + tr, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](part) + if !ok { + parts = append(parts, part) + continue + } + output := renderToolResultOutput(tr.Output) + note := fmt.Sprintf("[A tool run by the parent agent returned: %s]", output) + if call, known := calls[tr.ToolCallID]; known { + note = fmt.Sprintf( + "[The parent agent ran the %s tool with input %s. Result: %s]", + call.name, call.input, output, + ) + } + parts = append(parts, fantasy.TextPart{Text: note}) + } + msg.Role = fantasy.MessageRoleUser + msg.Content = parts + result = append(result, msg) + default: result = append(result, msg) - continue } + } + return result +} - kept := make([]fantasy.MessagePart, 0, len(msg.Content)) - for _, part := range msg.Content { - tr, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](part) - if !ok { - kept = append(kept, part) - continue - } - if _, matched := known[tr.ToolCallID]; matched { - kept = append(kept, part) - } +// renderToolResultOutput flattens a tool result payload into text for the +// advisor transcript. Media payloads are summarized instead of inlined +// because base64 data adds prompt bulk without helping a text-only advisor. +func renderToolResultOutput(output fantasy.ToolResultOutputContent) string { + switch typed := output.(type) { + case fantasy.ToolResultOutputContentText: + return typed.Text + case fantasy.ToolResultOutputContentError: + if typed.Error != nil { + return "error: " + typed.Error.Error() } - if len(kept) == 0 { - continue + return "error" + case fantasy.ToolResultOutputContentMedia: + if typed.Text != "" { + return fmt.Sprintf("[%s media] %s", typed.MediaType, typed.Text) } - trimmed := msg - trimmed.Content = kept - result = append(result, trimmed) + return fmt.Sprintf("[%s media]", typed.MediaType) + default: + return "" } - return result } func textMessage(role fantasy.MessageRole, text string) fantasy.Message { diff --git a/coderd/x/chatd/chatadvisor/runner.go b/coderd/x/chatd/chatadvisor/runner.go index 4247f385dd7e7..2df22118302e8 100644 --- a/coderd/x/chatd/chatadvisor/runner.go +++ b/coderd/x/chatd/chatadvisor/runner.go @@ -2,6 +2,7 @@ package chatadvisor import ( "context" + "fmt" "strings" "time" @@ -92,8 +93,11 @@ func (rt *Runtime) RunAdvisor( // as not consuming a use. rt.release() return AdvisorResult{ - Type: ResultTypeError, - Error: "advisor produced no text output", + Type: ResultTypeError, + Error: fmt.Sprintf( + "advisor produced no text output (%s)", + describeTextlessOutcome(outcome), + ), RemainingUses: rt.RemainingUses(), }, nil } @@ -121,3 +125,45 @@ func extractAdvisorText(step chatloop.PersistedStep) string { } return strings.TrimSpace(strings.Join(parts, "\n\n")) } + +// describeTextlessOutcome summarizes a step that yielded no usable advice +// text so the error pinpoints the failure mode. A reasoning-only step means +// the model spent its turn deciding on an action (such as a tool call it +// cannot perform in this tool-less run) without answering; a length finish +// means the output was truncated before any text was produced. +func describeTextlessOutcome(outcome chatloop.AssistantOutcome) string { + var text, reasoning, toolCalls, other int + for _, content := range outcome.Step.Content { + switch content.(type) { + case fantasy.TextContent: + text++ + case fantasy.ReasoningContent: + reasoning++ + case fantasy.ToolCallContent: + toolCalls++ + default: + other++ + } + } + if len(outcome.ToolCalls) > toolCalls { + toolCalls = len(outcome.ToolCalls) + } + + kinds := make([]string, 0, 4) + appendKind := func(name string, count int) { + if count > 0 { + kinds = append(kinds, fmt.Sprintf("%s=%d", name, count)) + } + } + // Text parts can only reach here blank, so label them accordingly. + appendKind("blank_text", text) + appendKind("reasoning", reasoning) + appendKind("tool_call", toolCalls) + appendKind("other", other) + + summary := "none" + if len(kinds) > 0 { + summary = strings.Join(kinds, ", ") + } + return fmt.Sprintf("finish_reason=%s; parts: %s", outcome.FinishReason, summary) +} diff --git a/coderd/x/chatd/chatadvisor/runner_test.go b/coderd/x/chatd/chatadvisor/runner_test.go index c4a4ff96e54e5..c3830ec4fee66 100644 --- a/coderd/x/chatd/chatadvisor/runner_test.go +++ b/coderd/x/chatd/chatadvisor/runner_test.go @@ -308,6 +308,87 @@ func TestAdvisorRunError(t *testing.T) { require.Equal(t, 0, retried.RemainingUses) } +func TestAdvisorRunTextlessOutcomeDiagnostics(t *testing.T) { + t.Parallel() + + // A step without usable text collapses into one error result. The + // error must describe what the model actually returned so failure + // modes (tool-call mimicry, reasoning-only turns, truncation) are + // distinguishable from field reports alone. + tests := []struct { + name string + parts []fantasy.StreamPart + wantError string + }{ + { + name: "ReasoningOnly", + parts: []fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeReasoningStart, ID: "r-1"}, + {Type: fantasy.StreamPartTypeReasoningDelta, ID: "r-1", Delta: "I should call the advisor tool."}, + {Type: fantasy.StreamPartTypeReasoningEnd, ID: "r-1"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop}, + }, + wantError: "advisor produced no text output (finish_reason=stop; parts: reasoning=1)", + }, + { + name: "ToolCallOnly", + parts: []fantasy.StreamPart{ + { + Type: fantasy.StreamPartTypeToolCall, + ID: "call-1", + ToolCallName: "advisor", + ToolCallInput: `{"question":"hi"}`, + }, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonToolCalls}, + }, + wantError: "advisor produced no text output (finish_reason=tool-calls; parts: tool_call=1)", + }, + { + name: "BlankText", + parts: []fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeTextStart, ID: "text-1"}, + {Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: " "}, + {Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop}, + }, + wantError: "advisor produced no text output (finish_reason=stop; parts: blank_text=1)", + }, + { + name: "Empty", + parts: []fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonLength}, + }, + wantError: "advisor produced no text output (finish_reason=length; parts: none)", + }, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + runtime, err := chatadvisor.NewRuntime(chatadvisor.RuntimeConfig{ + Model: &chattest.FakeModel{ + ProviderName: "test-provider", + ModelName: "test-model", + StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) { + return streamFromParts(testCase.parts), nil + }, + }, + MaxUsesPerRun: 1, + MaxOutputTokens: 64, + }) + require.NoError(t, err) + + result, err := runtime.RunAdvisor(t.Context(), "what should I do?", nil, nil) + require.NoError(t, err) + require.Equal(t, chatadvisor.ResultTypeError, result.Type) + require.Equal(t, testCase.wantError, result.Error) + // A text-free run must refund its use so the parent can retry. + require.Equal(t, 1, result.RemainingUses) + }) + } +} + func TestNewRuntimeValidation(t *testing.T) { t.Parallel() @@ -617,15 +698,14 @@ func TestBuildAdvisorMessagesPrefersNewestSystemDirectivesUnderBudget(t *testing require.Equal(t, "Need advice", singleText(t, messages[3])) } -func TestBuildAdvisorMessagesDropsOrphanToolResults(t *testing.T) { +func TestBuildAdvisorMessagesTextualizesOrphanToolResult(t *testing.T) { t.Parallel() // Simulate a truncation cut that lands between the assistant tool-call - // message and its tool-result. The resulting recent window should not - // contain an orphan tool_result referencing a missing tool_use block. - // Building the window with only [tool_result, assistant_reply] mimics - // the state produced by the backward walk hitting its byte budget right - // before the tool-call assistant message. + // message and its tool-result. The result keeps its context value as a + // text note; because no raw tool blocks reach the nested call, there + // is no provider pairing constraint left to violate. The originating + // call is unknown, so the note uses the generic form. snapshot := []fantasy.Message{ toolResultMessage("call-1", "ok"), textMessage(fantasy.MessageRoleAssistant, "final reply"), @@ -633,41 +713,79 @@ func TestBuildAdvisorMessagesDropsOrphanToolResults(t *testing.T) { messages := chatadvisor.BuildAdvisorMessages("Need advice", snapshot) - // Advisor system + assistant reply + question. The orphan tool result - // must not appear in the advisor prompt. - require.Len(t, messages, 3) + // Advisor system + result note + assistant reply + question. + require.Len(t, messages, 4) require.Equal(t, fantasy.MessageRoleSystem, messages[0].Role) require.Contains(t, singleText(t, messages[0]), "parent agent") - require.Equal(t, fantasy.MessageRoleAssistant, messages[1].Role) - require.Equal(t, "final reply", singleText(t, messages[1])) - require.Equal(t, fantasy.MessageRoleUser, messages[2].Role) - require.Equal(t, "Need advice", singleText(t, messages[2])) + require.Equal(t, fantasy.MessageRoleUser, messages[1].Role) + require.Equal(t, "[A tool run by the parent agent returned: ok]", singleText(t, messages[1])) + require.Equal(t, fantasy.MessageRoleAssistant, messages[2].Role) + require.Equal(t, "final reply", singleText(t, messages[2])) + require.Equal(t, fantasy.MessageRoleUser, messages[3].Role) + require.Equal(t, "Need advice", singleText(t, messages[3])) - for _, msg := range messages { - require.NotEqual(t, fantasy.MessageRoleTool, msg.Role) - } + requireNoRawToolContent(t, messages) } -func TestBuildAdvisorMessagesKeepsPairedToolCallAndResult(t *testing.T) { +func TestBuildAdvisorMessagesTextualizesToolExchanges(t *testing.T) { t.Parallel() + // The nested advisor call defines no tools, so assistant-authored tool + // artifacts must not reach it: the model imitates them instead of + // answering. Each call/result pair folds into a single user-role note, + // assistant text survives, and an assistant message that carried only + // tool calls disappears entirely. snapshot := []fantasy.Message{ - toolCallAssistantMessage("call-1", "search", `{"q":"x"}`), + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "let me look"}, + fantasy.ToolCallPart{ToolCallID: "call-1", ToolName: "search", Input: `{"q":"x"}`}, + }, + }, toolResultMessage("call-1", "ok"), + toolCallAssistantMessage("call-2", "search", `{"q":"y"}`), + toolResultMessage("call-2", "nope"), textMessage(fantasy.MessageRoleAssistant, "done"), } messages := chatadvisor.BuildAdvisorMessages("Need advice", snapshot) - // Advisor system + assistant tool call + tool result + assistant reply - // + question. The matched pair must survive. - require.Len(t, messages, 5) + // Advisor system + assistant text + note 1 + note 2 + assistant reply + // + question. The call-only assistant message is gone; its input is + // preserved inside note 2. + require.Len(t, messages, 6) require.Equal(t, fantasy.MessageRoleSystem, messages[0].Role) require.Equal(t, fantasy.MessageRoleAssistant, messages[1].Role) - require.Equal(t, fantasy.MessageRoleTool, messages[2].Role) - require.Equal(t, fantasy.MessageRoleAssistant, messages[3].Role) - require.Equal(t, "done", singleText(t, messages[3])) - require.Equal(t, fantasy.MessageRoleUser, messages[4].Role) + require.Equal(t, "let me look", singleText(t, messages[1])) + require.Equal(t, fantasy.MessageRoleUser, messages[2].Role) + require.Equal(t, + `[The parent agent ran the search tool with input {"q":"x"}. Result: ok]`, + singleText(t, messages[2])) + require.Equal(t, fantasy.MessageRoleUser, messages[3].Role) + require.Equal(t, + `[The parent agent ran the search tool with input {"q":"y"}. Result: nope]`, + singleText(t, messages[3])) + require.Equal(t, fantasy.MessageRoleAssistant, messages[4].Role) + require.Equal(t, "done", singleText(t, messages[4])) + require.Equal(t, fantasy.MessageRoleUser, messages[5].Role) + + requireNoRawToolContent(t, messages) +} + +// requireNoRawToolContent asserts that no tool-role message and no raw tool +// call/result part reaches the nested advisor prompt. +func requireNoRawToolContent(t *testing.T, messages []fantasy.Message) { + t.Helper() + for _, msg := range messages { + require.NotEqual(t, fantasy.MessageRoleTool, msg.Role) + for _, part := range msg.Content { + _, isCall := fantasy.AsMessagePart[fantasy.ToolCallPart](part) + require.False(t, isCall, "raw tool call part leaked into advisor prompt") + _, isResult := fantasy.AsMessagePart[fantasy.ToolResultPart](part) + require.False(t, isResult, "raw tool result part leaked into advisor prompt") + } + } } func streamFromParts(parts []fantasy.StreamPart) fantasy.StreamResponse {