Skip to content

Commit d1b5b3e

Browse files
rumplrumpl
committed
provider/anthropic: enforce tool sequencing, remove self-repair/validation
Stop inserting synthetic tool_result blocks and remove preflight sequencing validators. Instead, enforce Anthropic tool_use/tool_result adjacency and ID matching during message conversion (and preserve tool-call blocks during history trimming) so invalid histories fail fast with clear errors. Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
1 parent d7f1f6e commit d1b5b3e

File tree

6 files changed

+247
-413
lines changed

6 files changed

+247
-413
lines changed

‎pkg/model/provider/anthropic/beta_client.go‎

Lines changed: 0 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,6 @@ func (c *Client) createBetaStream(
4343
slog.Error("Failed to convert messages for Anthropic Beta request", "error", err)
4444
return nil, err
4545
}
46-
if err := validateAnthropicSequencingBeta(converted); err != nil {
47-
slog.Warn("Invalid message sequencing for Anthropic Beta API detected, attempting self-repair", "error", err)
48-
converted = repairAnthropicSequencingBeta(converted)
49-
if err2 := validateAnthropicSequencingBeta(converted); err2 != nil {
50-
slog.Error("Failed to self-repair Anthropic Beta sequencing", "error", err2)
51-
return nil, err
52-
}
53-
}
5446

5547
sys := extractBetaSystemBlocks(messages)
5648

@@ -146,114 +138,6 @@ func (c *Client) createBetaStream(
146138
return ad, nil
147139
}
148140

149-
// validateAnthropicSequencingBeta performs the same validation as standard API but for Beta payloads
150-
func validateAnthropicSequencingBeta(msgs []anthropic.BetaMessageParam) error {
151-
for i := range msgs {
152-
m, ok := marshalToMapBeta(msgs[i])
153-
if !ok || m["role"] != "assistant" {
154-
continue
155-
}
156-
157-
toolUseIDs := collectToolUseIDs(contentArrayBeta(m))
158-
if len(toolUseIDs) == 0 {
159-
continue
160-
}
161-
162-
if i+1 >= len(msgs) {
163-
slog.Warn("Anthropic (beta) sequencing invalid: assistant tool_use present but no next user tool_result message", "assistant_index", i)
164-
return errors.New("assistant tool_use present but no subsequent user message with tool_result blocks (beta)")
165-
}
166-
167-
next, ok := marshalToMapBeta(msgs[i+1])
168-
if !ok || next["role"] != "user" {
169-
slog.Warn("Anthropic (beta) sequencing invalid: next message after assistant tool_use is not user", "assistant_index", i, "next_role", next["role"])
170-
return errors.New("assistant tool_use must be followed by a user message containing corresponding tool_result blocks (beta)")
171-
}
172-
173-
toolResultIDs := collectToolResultIDs(contentArrayBeta(next))
174-
missing := differenceIDs(toolUseIDs, toolResultIDs)
175-
if len(missing) > 0 {
176-
slog.Warn("Anthropic (beta) sequencing invalid: missing tool_result for tool_use id in next user message", "assistant_index", i, "tool_use_id", missing[0], "missing_count", len(missing))
177-
return fmt.Errorf("missing tool_result for tool_use id %s in the next user message (beta)", missing[0])
178-
}
179-
}
180-
return nil
181-
}
182-
183-
// repairAnthropicSequencingBeta inserts a synthetic user message with tool_result blocks
184-
// for any assistant tool_use blocks that don't have corresponding tool_result blocks
185-
// in the immediate next user message.
186-
func repairAnthropicSequencingBeta(msgs []anthropic.BetaMessageParam) []anthropic.BetaMessageParam {
187-
if len(msgs) == 0 {
188-
return msgs
189-
}
190-
repaired := make([]anthropic.BetaMessageParam, 0, len(msgs)+2)
191-
for i := range msgs {
192-
m, ok := marshalToMapBeta(msgs[i])
193-
if !ok || m["role"] != "assistant" {
194-
repaired = append(repaired, msgs[i])
195-
continue
196-
}
197-
198-
toolUseIDs := collectToolUseIDs(contentArrayBeta(m))
199-
if len(toolUseIDs) == 0 {
200-
repaired = append(repaired, msgs[i])
201-
continue
202-
}
203-
204-
// Check if the next message is a user message with tool_results
205-
needsSyntheticMessage := true
206-
if i+1 < len(msgs) {
207-
if next, ok := marshalToMapBeta(msgs[i+1]); ok && next["role"] == "user" {
208-
toolResultIDs := collectToolResultIDs(contentArrayBeta(next))
209-
// Remove tool_use IDs that have corresponding tool_results
210-
for id := range toolResultIDs {
211-
delete(toolUseIDs, id)
212-
}
213-
// If all tool_use IDs have results, no synthetic message needed
214-
if len(toolUseIDs) == 0 {
215-
needsSyntheticMessage = false
216-
}
217-
}
218-
}
219-
220-
// Append the assistant message first
221-
repaired = append(repaired, msgs[i])
222-
223-
// If there are missing tool_results, insert a synthetic user message immediately after
224-
if needsSyntheticMessage && len(toolUseIDs) > 0 {
225-
slog.Debug("Inserting synthetic user message for missing tool_results",
226-
"assistant_index", i,
227-
"missing_count", len(toolUseIDs))
228-
229-
blocks := make([]anthropic.BetaContentBlockParamUnion, 0, len(toolUseIDs))
230-
for id := range toolUseIDs {
231-
slog.Debug("Creating synthetic tool_result", "tool_use_id", id)
232-
blocks = append(blocks, anthropic.BetaContentBlockParamUnion{
233-
OfToolResult: &anthropic.BetaToolResultBlockParam{
234-
ToolUseID: id,
235-
Content: []anthropic.BetaToolResultBlockParamContentUnion{
236-
{OfText: &anthropic.BetaTextBlockParam{Text: "(tool execution failed)"}},
237-
},
238-
},
239-
})
240-
}
241-
repaired = append(repaired, anthropic.BetaMessageParam{
242-
Role: anthropic.BetaMessageParamRoleUser,
243-
Content: blocks,
244-
})
245-
}
246-
}
247-
return repaired
248-
}
249-
250-
// marshalToMapBeta is an alias for marshalToMap - shared with standard API.
251-
// Kept as separate function for clarity in Beta-specific code paths.
252-
var marshalToMapBeta = marshalToMap
253-
254-
// contentArrayBeta is an alias for contentArray - shared with standard API.
255-
var contentArrayBeta = contentArray
256-
257141
// countAnthropicTokensBeta calls Anthropic's Count Tokens API for the provided Beta API payload
258142
// and returns the number of input tokens.
259143
func countAnthropicTokensBeta(

‎pkg/model/provider/anthropic/beta_converter.go‎

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
// blocks from the same assistant message MUST be grouped into a single user message.
2323
func (c *Client) convertBetaMessages(ctx context.Context, messages []chat.Message) ([]anthropic.BetaMessageParam, error) {
2424
var betaMessages []anthropic.BetaMessageParam
25+
var pendingToolUseIDs map[string]struct{}
2526

2627
for i := 0; i < len(messages); i++ {
2728
msg := &messages[i]
@@ -75,11 +76,15 @@ func (c *Client) convertBetaMessages(ctx context.Context, messages []chat.Messag
7576

7677
// Add tool calls
7778
if len(msg.ToolCalls) > 0 {
79+
pendingToolUseIDs = make(map[string]struct{}, len(msg.ToolCalls))
7880
for _, toolCall := range msg.ToolCalls {
7981
var inpts map[string]any
8082
if err := json.Unmarshal([]byte(toolCall.Function.Arguments), &inpts); err != nil {
8183
inpts = map[string]any{}
8284
}
85+
if toolCall.ID != "" {
86+
pendingToolUseIDs[toolCall.ID] = struct{}{}
87+
}
8388
contentBlocks = append(contentBlocks, anthropic.BetaContentBlockParamUnion{
8489
OfToolUse: &anthropic.BetaToolUseBlockParam{
8590
ID: toolCall.ID,
@@ -88,6 +93,8 @@ func (c *Client) convertBetaMessages(ctx context.Context, messages []chat.Messag
8893
},
8994
})
9095
}
96+
} else {
97+
pendingToolUseIDs = nil
9198
}
9299

93100
if len(contentBlocks) > 0 {
@@ -102,38 +109,56 @@ func (c *Client) convertBetaMessages(ctx context.Context, messages []chat.Messag
102109
// Collect consecutive tool messages and merge them into a single user message
103110
// This is required by Anthropic API: all tool_result blocks for tool_use blocks
104111
// from the same assistant message must be in the same user message
105-
toolResultBlocks := []anthropic.BetaContentBlockParamUnion{
106-
{
107-
OfToolResult: &anthropic.BetaToolResultBlockParam{
108-
ToolUseID: msg.ToolCallID,
109-
Content: []anthropic.BetaToolResultBlockParamContentUnion{
110-
{OfText: &anthropic.BetaTextBlockParam{Text: strings.TrimSpace(msg.Content)}},
111-
},
112-
},
113-
},
112+
if pendingToolUseIDs == nil {
113+
// Orphan tool results (no preceding assistant tool_use in this window): drop them.
114+
j := i
115+
for j < len(messages) && messages[j].Role == chat.MessageRoleTool {
116+
j++
117+
}
118+
i = j - 1
119+
continue
114120
}
115121

116-
// Look ahead for consecutive tool messages and merge them
117-
j := i + 1
122+
toolResultBlocks := make([]anthropic.BetaContentBlockParamUnion, 0)
123+
hadToolMessages := false
124+
j := i
118125
for j < len(messages) && messages[j].Role == chat.MessageRoleTool {
119-
toolResultBlocks = append(toolResultBlocks, anthropic.BetaContentBlockParamUnion{
120-
OfToolResult: &anthropic.BetaToolResultBlockParam{
121-
ToolUseID: messages[j].ToolCallID,
122-
Content: []anthropic.BetaToolResultBlockParamContentUnion{
123-
{OfText: &anthropic.BetaTextBlockParam{Text: strings.TrimSpace(messages[j].Content)}},
124-
},
125-
},
126-
})
126+
hadToolMessages = true
127+
id := messages[j].ToolCallID
128+
if id != "" {
129+
if _, ok := pendingToolUseIDs[id]; ok {
130+
toolResultBlocks = append(toolResultBlocks, anthropic.BetaContentBlockParamUnion{
131+
OfToolResult: &anthropic.BetaToolResultBlockParam{
132+
ToolUseID: id,
133+
Content: []anthropic.BetaToolResultBlockParamContentUnion{
134+
{OfText: &anthropic.BetaTextBlockParam{Text: strings.TrimSpace(messages[j].Content)}},
135+
},
136+
},
137+
})
138+
delete(pendingToolUseIDs, id)
139+
} else {
140+
// Don't include mismatched tool results; they'd be invalid in Anthropic payloads.
141+
}
142+
}
127143
j++
128144
}
129145

130-
// Add the merged user message with all tool results
131-
betaMessages = append(betaMessages, anthropic.BetaMessageParam{
132-
Role: anthropic.BetaMessageParamRoleUser,
133-
Content: toolResultBlocks,
134-
})
146+
if hadToolMessages && len(toolResultBlocks) == 0 {
147+
return nil, fmt.Errorf("tool_result messages present but none match pending tool_use ids (beta converter)")
148+
}
149+
if len(pendingToolUseIDs) > 0 {
150+
for id := range pendingToolUseIDs {
151+
return nil, fmt.Errorf("missing tool_result for tool_use id %s (beta converter)", id)
152+
}
153+
}
135154

136-
// Skip the messages we've already processed
155+
if len(toolResultBlocks) > 0 {
156+
betaMessages = append(betaMessages, anthropic.BetaMessageParam{
157+
Role: anthropic.BetaMessageParamRoleUser,
158+
Content: toolResultBlocks,
159+
})
160+
}
161+
pendingToolUseIDs = nil
137162
i = j - 1
138163
continue
139164
}

‎pkg/model/provider/anthropic/beta_converter_test.go‎

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package anthropic
22

33
import (
4+
"encoding/json"
45
"testing"
56

67
"github.com/stretchr/testify/assert"
@@ -10,6 +11,39 @@ import (
1011
"github.com/docker/cagent/pkg/tools"
1112
)
1213

14+
func marshalToMapBeta(t *testing.T, v any) map[string]any {
15+
t.Helper()
16+
b, err := json.Marshal(v)
17+
require.NoError(t, err)
18+
var m map[string]any
19+
require.NoError(t, json.Unmarshal(b, &m))
20+
return m
21+
}
22+
23+
func contentArrayBeta(m map[string]any) []any {
24+
if a, ok := m["content"].([]any); ok {
25+
return a
26+
}
27+
return nil
28+
}
29+
30+
func collectToolResultIDsBeta(content []any) map[string]struct{} {
31+
ids := make(map[string]struct{})
32+
for _, c := range content {
33+
cb, ok := c.(map[string]any)
34+
if !ok {
35+
continue
36+
}
37+
if cb["type"] != "tool_result" {
38+
continue
39+
}
40+
if id, _ := cb["tool_use_id"].(string); id != "" {
41+
ids[id] = struct{}{}
42+
}
43+
}
44+
return ids
45+
}
46+
1347
func TestConvertBetaMessages_MergesConsecutiveToolMessages(t *testing.T) {
1448
// Simulates the roast battle scenario where:
1549
// - Assistant message has 2 tool_use blocks (transfer_task calls)
@@ -65,27 +99,22 @@ func TestConvertBetaMessages_MergesConsecutiveToolMessages(t *testing.T) {
6599

66100
require.Len(t, betaMessages, 4, "Should have 4 messages after conversion")
67101

68-
msg0Map, _ := marshalToMapBeta(betaMessages[0])
69-
msg1Map, _ := marshalToMapBeta(betaMessages[1])
70-
msg2Map, _ := marshalToMapBeta(betaMessages[2])
71-
msg3Map, _ := marshalToMapBeta(betaMessages[3])
102+
msg0Map := marshalToMapBeta(t, betaMessages[0])
103+
msg1Map := marshalToMapBeta(t, betaMessages[1])
104+
msg2Map := marshalToMapBeta(t, betaMessages[2])
105+
msg3Map := marshalToMapBeta(t, betaMessages[3])
72106
assert.Equal(t, "user", msg0Map["role"])
73107
assert.Equal(t, "assistant", msg1Map["role"])
74108
assert.Equal(t, "user", msg2Map["role"])
75109
assert.Equal(t, "assistant", msg3Map["role"])
76110

77-
userMsg2Map, ok := marshalToMapBeta(betaMessages[2])
78-
require.True(t, ok)
111+
userMsg2Map := marshalToMapBeta(t, betaMessages[2])
79112
content := contentArrayBeta(userMsg2Map)
80113
require.Len(t, content, 2, "User message should have 2 tool_result blocks")
81114

82-
toolResultIDs := collectToolResultIDs(content)
115+
toolResultIDs := collectToolResultIDsBeta(content)
83116
assert.Contains(t, toolResultIDs, "tool_call_1")
84117
assert.Contains(t, toolResultIDs, "tool_call_2")
85-
86-
// Most importantly: validate that the sequence is valid for Anthropic API
87-
err = validateAnthropicSequencingBeta(betaMessages)
88-
require.NoError(t, err, "Converted messages should pass Anthropic sequencing validation")
89118
}
90119

91120
func TestConvertBetaMessages_SingleToolMessage(t *testing.T) {
@@ -123,10 +152,6 @@ func TestConvertBetaMessages_SingleToolMessage(t *testing.T) {
123152
betaMessages, err := testClient().convertBetaMessages(t.Context(), messages)
124153
require.NoError(t, err)
125154
require.Len(t, betaMessages, 4)
126-
127-
// Validate sequence
128-
err = validateAnthropicSequencingBeta(betaMessages)
129-
require.NoError(t, err)
130155
}
131156

132157
func TestConvertBetaMessages_NonConsecutiveToolMessages(t *testing.T) {
@@ -181,10 +206,6 @@ func TestConvertBetaMessages_NonConsecutiveToolMessages(t *testing.T) {
181206
},
182207
}
183208

184-
betaMessages, err := testClient().convertBetaMessages(t.Context(), messages)
209+
_, err := testClient().convertBetaMessages(t.Context(), messages)
185210
require.NoError(t, err)
186-
187-
// Validate the entire sequence
188-
err = validateAnthropicSequencingBeta(betaMessages)
189-
require.NoError(t, err, "Messages with non-consecutive tool calls should still validate")
190211
}

0 commit comments

Comments
 (0)