diff --git a/internal/provider/openai.go b/internal/provider/openai.go index 4a4fc9dce..96d7c3548 100644 --- a/internal/provider/openai.go +++ b/internal/provider/openai.go @@ -7,7 +7,9 @@ import ( "errors" "fmt" "io" + "maps" "net/http" + "slices" "strings" "sync" @@ -901,7 +903,12 @@ func (p *OpenAIProvider) ChatStream(ctx context.Context, messages []Message, too debug.Log("openai", "tool_call id=%s name=%s args=%s", tc.ID, tc.Name, string(tc.Arguments)) } } - for idx, tc := range toolCalls { + // #4244: Go map iteration is randomized - emitting the parallel + // tool calls in map order lost the model's idx-expressed intent + // (mkdir→write, add→commit pairs executed in random order, and + // replay transcripts differed run-to-run). Emit sorted by idx. + for _, idx := range slices.Sorted(maps.Keys(toolCalls)) { + tc := toolCalls[idx] // Attempt JSON repair for truncated/malformed args. // This is especially important when finish_reason is // "length" (max_tokens truncation) - the args may be diff --git a/internal/provider/openai_stream_flush.go b/internal/provider/openai_stream_flush.go index 6f563d4f2..cc949385b 100644 --- a/internal/provider/openai_stream_flush.go +++ b/internal/provider/openai_stream_flush.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "fmt" + "maps" + "slices" "github.com/topcheer/ggcode/internal/debug" ) @@ -20,7 +22,10 @@ func (p *OpenAIProvider) flushResidualToolCalls(ctx context.Context, ch chan<- S if !shouldFlush { return 0, false, true } - for idx, tc := range toolCalls { + // #4244: sorted emit (same randomized-map fix as the finish_reason + // loop in openai.go) - the flush path must not reorder either. + for _, idx := range slices.Sorted(maps.Keys(toolCalls)) { + tc := toolCalls[idx] if tc.Name == "" || tc.ID == "" { continue } diff --git a/internal/provider/zz_issue4244_test.go b/internal/provider/zz_issue4244_test.go new file mode 100644 index 000000000..fbcf818c8 --- /dev/null +++ b/internal/provider/zz_issue4244_test.go @@ -0,0 +1,28 @@ +package provider + +import ( + "maps" + "slices" + "testing" +) + +// #4244: parallel tool calls must be emitted in model-idx order, not Go map +// random order. The fixed emit sequence is slices.Sorted(maps.Keys(acc)) in +// both the finish_reason loop (openai.go) and the residual flush +// (openai_stream_flush.go) - prove it is sorted and complete for a +// non-trivial key set, over many reshuffles of the map. +func TestIssue4244_SortedEmitOrder(t *testing.T) { + acc := map[int]*ToolCallDelta{ + 7: {ID: "c7", Name: "git_commit"}, + 0: {ID: "c0", Name: "git_add"}, + 3: {ID: "c3", Name: "mkdir"}, + 1: {ID: "c1", Name: "write_file"}, + } + want := []int{0, 1, 3, 7} + for i := 0; i < 50; i++ { // re-hash guard: order must hold every run + got := slices.Sorted(maps.Keys(acc)) + if !slices.Equal(got, want) { + t.Fatalf("emit order must be the sorted key set %v, got %v", want, got) + } + } +}