fix(coderd/x/chatd/chatadvisor): textualize advisor prompt tool exchanges (#27059)

Closes CODAGT-592.

## Problem

The advisor tool sometimes fails with the opaque error `advisor produced
no text output`. Live reproduction against `claude-sonnet-4-6` showed
the cause: `BuildAdvisorMessages` forwards the parent conversation's raw
`tool_use`/`tool_result` blocks into the nested advisor call, which
defines no tools. The nested model imitates the forwarded pattern and
spends its turn committing to a tool call it cannot make (captured
reasoning from a failing run: "The user wants me to make another tool
call to the advisor about writing a poem about cucumbers."), so the step
ends with reasoning-only or empty content and no advice. Because each
chat step currently rebuilds the advisor runtime and snapshot
(CODAGT-593), the second advisor call in a run reliably sees the first
call's exchange, which is why the first call succeeds and later ones
fail.

## Fix

- `BuildAdvisorMessages` rewrites tool activity as plain-text notes:
assistant tool-call parts are removed and folded, together with their
matching result, into a single user-role note of the form `[The parent
agent ran the X tool with input {...}. Result: ...]`. No raw tool blocks
and no bare call lines reach the tool-less nested request. This also
removes the provider requirement that `tool_result` blocks pair with a
`tool_use`, so results orphaned by window truncation are kept as notes
instead of dropped.
- The `advisor produced no text output` error now appends the finish
reason and content-part kinds, e.g. `advisor produced no text output
(finish_reason=stop; parts: reasoning=1)`, so field reports distinguish
tool-call mimicry, reasoning-only turns, and truncation.

Validated live by driving the production `RunAdvisor` path against
`claude-sonnet-4-6` through the dev.coder.com AI gateway: the failing
scenario went from 3/3 errors to 6/6 genuine advice (with and without
extended thinking), with the control scenario unaffected.

Related: CODAGT-593 (per-step advisor runtime recreation, addressed
separately) and CODAGT-742 (advisor tool call design).

<details>
<summary>Investigation and validation details</summary>

### Reproduction

A CLI prototype constructed the exact conversation snapshot the
generation preparer hands the advisor tool and called the real
`chatadvisor.NewRuntime` / `Runtime.RunAdvisor` / `BuildAdvisorMessages`
/ `chatloop.GenerateAssistant` chain against live `claude-sonnet-4-6`,
with a stream-teeing model wrapper capturing what `runner.go` discards
(finish reason, part kinds, reasoning text).

| Scenario (snapshot contents) | Thinking | Before fix | After fix |
|---|---|---|---|
| control: call #1 state, no prior advisor exchange | on | 3/3 advice |
2/2 advice |
| repro: call #2 state, prior advisor `tool_use`/`tool_result` pair
forwarded | on | 3/3 `advisor produced no text output` | 3/3 genuine
advice |
| repro | off | 2/3 same error, 1/3 degenerate advice ("I'll ask the
advisor...") | 3/3 genuine advice |

Every failing response was a tiny thinking block, zero text, zero
tool-call stream parts, finish reason `stop`; the model's own reasoning
text showed it deciding to "make the second tool call" in a request with
`tools=0`. The refunded `remaining_uses: 1200` in the failing
tool-result JSON matches the original issue screenshot.

### Decision log

- Tool exchanges are folded into a single user-role note per call/result
pair. A first attempt rendered assistant-authored `[tool call:
name(input)]` text lines plus separate result messages; live runs then
returned the literal `[tool call: advisor(...)]` line as the advice 6/6
times. The bare assistant call line is itself an imitable pattern, so no
assistant-authored tool artifact may survive the handoff. The folded
user-role note produced 6/6 genuine advice.
- An assistant message that carried only tool calls is dropped entirely;
the folded notes preserve the information.
- `dropOrphanToolMessages` was removed: without raw tool blocks there is
no provider pairing constraint, and an orphaned result note retains
context value.
- A reasoning-budget-starvation hypothesis (thinking budget consuming
`MaxOutputTokens`) did not reproduce on `claude-sonnet-4-6`; the model
adapts thinking length to the cap. The enriched error would identify
such cases on other models via `finish_reason=length`.
- CODAGT-593 (persisting the advisor runtime across steps) is
intentionally not addressed here; it shrinks the priming window but the
handoff fix is what removes the failure mode.

</details>

---

*This PR was generated by Coder Agents on behalf of @ThomasK33 (Linear
agent session for CODAGT-592).*
This commit is contained in:
Thomas Kosiewski
2026-07-21 13:38:20 +02:00
committed by GitHub
parent 6014a44c85
commit aa89801ee5
3 changed files with 267 additions and 64 deletions
+76 -37
View File
@@ -2,6 +2,7 @@ package chatadvisor
import (
"encoding/json"
"fmt"
"maps"
"slices"
"strings"
@@ -111,65 +112,103 @@ 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}
}
result = append(result, msg)
continue
}
if msg.Role != fantasy.MessageRoleTool {
result = append(result, msg)
continue
}
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)
if len(parts) == 0 {
// The message carried only tool calls; the folded
// result notes preserve the information.
continue
}
if _, matched := known[tr.ToolCallID]; matched {
kept = append(kept, part)
msg.Content = parts
result = append(result, msg)
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)
}
if len(kept) == 0 {
continue
}
trimmed := msg
trimmed.Content = kept
result = append(result, trimmed)
}
return result
}
// 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()
}
return "error"
case fantasy.ToolResultOutputContentMedia:
if typed.Text != "" {
return fmt.Sprintf("[%s media] %s", typed.MediaType, typed.Text)
}
return fmt.Sprintf("[%s media]", typed.MediaType)
default:
return ""
}
}
func textMessage(role fantasy.MessageRole, text string) fantasy.Message {
return fantasy.Message{
Role: role,
+48 -2
View File
@@ -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)
}
+143 -25
View File
@@ -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 {