Skip to content

Commit 876e910

Browse files
committed
fix(#2053): add reverse validation and merge-based repair for tool sequencing
validateSequencing only checked the forward direction: every assistant tool_use must have a matching tool_result in the next user message. It did not check the reverse: every tool_result must reference a tool_use in the immediately preceding assistant message. Orphan tool_results passed validation silently. Additionally, repairSequencing inserted a synthetic user message with missing tool_results *before* an existing user message that already had partial tool_results. This split tool_results across two user messages, causing the existing results to become orphaned: assistant(tool_use: A, B) synthetic_user(tool_result: B) <- repair inserted this user(tool_result: A) <- A now orphaned (previous is synthetic_user) Two fixes: 1. validateSequencing now also checks the reverse direction: for each user message containing tool_result blocks, verify the immediately preceding message is an assistant with corresponding tool_use IDs. 2. repairSequencing now merges missing tool_results into the existing next user message instead of inserting a separate synthetic message. This keeps all tool_results in a single user message adjacent to the assistant, satisfying both forward and reverse invariants. Assisted-By: docker-agent
1 parent e3c2819 commit 876e910

1 file changed

Lines changed: 100 additions & 27 deletions

File tree

pkg/model/provider/anthropic/client.go

Lines changed: 100 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -776,75 +776,148 @@ func contentArray(m map[string]any) []any {
776776
return nil
777777
}
778778

779-
// validateSequencing generically validates that every assistant message with tool_use blocks
780-
// is immediately followed by a user message with corresponding tool_result blocks.
781-
// It works on both standard (MessageParam) and Beta (BetaMessageParam) types by
782-
// marshaling to map[string]any for inspection.
779+
// validateSequencing generically validates that:
780+
// 1. Every assistant message with tool_use blocks is immediately followed by a user
781+
// message with corresponding tool_result blocks.
782+
// 2. Every user message with tool_result blocks is immediately preceded by an assistant
783+
// message that contains the corresponding tool_use blocks.
783784
func validateSequencing[T any](msgs []T) error {
784785
for i := range msgs {
785786
m, ok := marshalToMap(msgs[i])
786-
if !ok || m["role"] != "assistant" {
787+
if !ok {
787788
continue
788789
}
789790

790-
toolUseIDs := collectToolUseIDs(contentArray(m))
791-
if len(toolUseIDs) == 0 {
792-
continue
793-
}
791+
// Forward check: assistant tool_use → next user must have matching tool_results
792+
if m["role"] == "assistant" {
793+
toolUseIDs := collectToolUseIDs(contentArray(m))
794+
if len(toolUseIDs) == 0 {
795+
continue
796+
}
794797

795-
if i+1 >= len(msgs) {
796-
slog.Warn("Anthropic sequencing invalid: assistant tool_use present but no next user tool_result message", "assistant_index", i)
797-
return errors.New("assistant tool_use present but no subsequent user message with tool_result blocks")
798-
}
798+
if i+1 >= len(msgs) {
799+
slog.Warn("Anthropic sequencing invalid: assistant tool_use present but no next user tool_result message", "assistant_index", i)
800+
return errors.New("assistant tool_use present but no subsequent user message with tool_result blocks")
801+
}
802+
803+
next, ok := marshalToMap(msgs[i+1])
804+
if !ok || next["role"] != "user" {
805+
slog.Warn("Anthropic sequencing invalid: next message after assistant tool_use is not user", "assistant_index", i, "next_role", next["role"])
806+
return errors.New("assistant tool_use must be followed by a user message containing corresponding tool_result blocks")
807+
}
799808

800-
next, ok := marshalToMap(msgs[i+1])
801-
if !ok || next["role"] != "user" {
802-
slog.Warn("Anthropic sequencing invalid: next message after assistant tool_use is not user", "assistant_index", i, "next_role", next["role"])
803-
return errors.New("assistant tool_use must be followed by a user message containing corresponding tool_result blocks")
809+
toolResultIDs := collectToolResultIDs(contentArray(next))
810+
missing := differenceIDs(toolUseIDs, toolResultIDs)
811+
if len(missing) > 0 {
812+
slog.Warn("Anthropic 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))
813+
return fmt.Errorf("missing tool_result for tool_use id %s in the next user message", missing[0])
814+
}
804815
}
805816

806-
toolResultIDs := collectToolResultIDs(contentArray(next))
807-
missing := differenceIDs(toolUseIDs, toolResultIDs)
808-
if len(missing) > 0 {
809-
slog.Warn("Anthropic 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))
810-
return fmt.Errorf("missing tool_result for tool_use id %s in the next user message", missing[0])
817+
// Reverse check: user with tool_result → previous message must be assistant with matching tool_use
818+
if m["role"] == "user" {
819+
toolResultIDs := collectToolResultIDs(contentArray(m))
820+
if len(toolResultIDs) == 0 {
821+
continue
822+
}
823+
824+
if i == 0 {
825+
slog.Warn("Anthropic sequencing invalid: user tool_result with no preceding assistant message", "user_index", i)
826+
return errors.New("user tool_result blocks with no preceding assistant message")
827+
}
828+
829+
prev, ok := marshalToMap(msgs[i-1])
830+
if !ok || prev["role"] != "assistant" {
831+
slog.Warn("Anthropic sequencing invalid: user tool_result not preceded by assistant", "user_index", i)
832+
return errors.New("user tool_result blocks must be preceded by an assistant message with corresponding tool_use blocks")
833+
}
834+
835+
toolUseIDs := collectToolUseIDs(contentArray(prev))
836+
orphan := differenceIDs(toolResultIDs, toolUseIDs)
837+
if len(orphan) > 0 {
838+
slog.Warn("Anthropic sequencing invalid: orphan tool_result referencing non-existent tool_use", "user_index", i, "tool_use_id", orphan[0], "orphan_count", len(orphan))
839+
return fmt.Errorf("orphan tool_result for tool_use id %s: no matching tool_use in preceding assistant message", orphan[0])
840+
}
811841
}
812842
}
813843
return nil
814844
}
815845

816846
// repairSequencing generically inserts a synthetic user message after any assistant
817-
// tool_use message that is missing corresponding tool_result blocks. The makeSynthetic
847+
// tool_use message that is missing corresponding tool_result blocks. When the next
848+
// message is already a user message with partial tool_results, the missing results
849+
// are merged into that message to avoid splitting results across messages (which
850+
// would cause the API to reject orphan tool_result references). The makeSynthetic
818851
// callback builds the appropriate user message type for the remaining tool_use IDs.
819852
func repairSequencing[T any](msgs []T, makeSynthetic func(toolUseIDs map[string]struct{}) T) []T {
820853
if len(msgs) == 0 {
821854
return msgs
822855
}
823856
repaired := make([]T, 0, len(msgs)+2)
824857
for i := range msgs {
825-
repaired = append(repaired, msgs[i])
826-
827858
m, ok := marshalToMap(msgs[i])
828859
if !ok || m["role"] != "assistant" {
860+
repaired = append(repaired, msgs[i])
829861
continue
830862
}
831863

864+
repaired = append(repaired, msgs[i])
865+
832866
toolUseIDs := collectToolUseIDs(contentArray(m))
833867
if len(toolUseIDs) == 0 {
834868
continue
835869
}
836870

837-
// Remove any IDs that already have results in the next user message
871+
// Check if the next message is a user message with some tool_results
872+
hasNextUser := false
838873
if i+1 < len(msgs) {
839874
if next, ok := marshalToMap(msgs[i+1]); ok && next["role"] == "user" {
875+
hasNextUser = true
840876
toolResultIDs := collectToolResultIDs(contentArray(next))
841877
for id := range toolResultIDs {
842878
delete(toolUseIDs, id)
843879
}
844880
}
845881
}
846882

847-
if len(toolUseIDs) > 0 {
883+
if len(toolUseIDs) == 0 {
884+
continue
885+
}
886+
887+
if hasNextUser {
888+
// Merge the synthetic results into the existing next user message
889+
// by replacing it with a combined message (synthetic first, then original).
890+
// This avoids splitting tool_results across separate user messages.
891+
synthetic := makeSynthetic(toolUseIDs)
892+
synthMap, _ := marshalToMap(synthetic)
893+
nextMap, _ := marshalToMap(msgs[i+1])
894+
895+
synthContent := contentArray(synthMap)
896+
nextContent := contentArray(nextMap)
897+
898+
mergedContent := append(synthContent, nextContent...)
899+
nextMap["content"] = mergedContent
900+
901+
// Re-marshal the merged message back into the typed parameter
902+
mergedBytes, err := json.Marshal(nextMap)
903+
if err != nil {
904+
slog.Warn("Failed to marshal merged tool_result message, inserting synthetic instead", "error", err)
905+
repaired = append(repaired, synthetic)
906+
continue
907+
}
908+
var merged T
909+
if err := json.Unmarshal(mergedBytes, &merged); err != nil {
910+
slog.Warn("Failed to unmarshal merged tool_result message, inserting synthetic instead", "error", err)
911+
repaired = append(repaired, synthetic)
912+
continue
913+
}
914+
915+
slog.Debug("Merged synthetic tool_results into existing user message",
916+
"assistant_index", i,
917+
"missing_count", len(toolUseIDs))
918+
// Replace the next message in msgs so we skip the original
919+
msgs[i+1] = merged
920+
} else {
848921
slog.Debug("Inserting synthetic user message for missing tool_results",
849922
"assistant_index", i,
850923
"missing_count", len(toolUseIDs))

0 commit comments

Comments
 (0)