diff --git a/model/openaimodel/internal/completions/media_test.go b/model/openaimodel/internal/completions/media_test.go index 4523e37c80..4ae0d787bc 100644 --- a/model/openaimodel/internal/completions/media_test.go +++ b/model/openaimodel/internal/completions/media_test.go @@ -78,6 +78,14 @@ func TestBuildParams_MediaOnTheWire(t *testing.T) { parts: []*genai.Part{{Text: "look", InlineData: &genai.Blob{MIMEType: "image/png", Data: png}}}, want: `[{"text":"look","type":"text"},` + image + `]`, }, + { + name: "executable code before an inline image keeps its order", + parts: []*genai.Part{ + {ExecutableCode: &genai.ExecutableCode{Code: "print(1)"}}, + {InlineData: &genai.Blob{MIMEType: "image/png", Data: png}}, + }, + want: "[{\"text\":\"Code:```python\\nprint(1)\\n```\",\"type\":\"text\"}," + image + "]", + }, { name: "image by url", parts: []*genai.Part{{FileData: &genai.FileData{MIMEType: "image/jpeg", FileURI: "https://example.com/a.jpg"}}}, diff --git a/model/openaimodel/internal/completions/request.go b/model/openaimodel/internal/completions/request.go index 7de0ea692f..ac2fd37390 100644 --- a/model/openaimodel/internal/completions/request.go +++ b/model/openaimodel/internal/completions/request.go @@ -139,9 +139,13 @@ func convertContent(content *genai.Content, tracker *shared.CallTracker) ([]open // Checked first, so a field this package cannot send is named even when // text or a call on the same part would otherwise carry it out unnoticed. // Media is excluded from the report because it is emitted below. - if field := shared.UnsupportedPayload(part, shared.PartFieldMedia); field != "" { + if field := shared.UnsupportedPayload(part, shared.PartFieldMedia, shared.PartFieldCodeExecution); field != "" { return nil, fmt.Errorf("openai: unsupported content part: %s", field) } + // Code execution has no wire field here. It goes out as the same + // fenced text the Responses path sends, so a Gemini turn replayed + // into this conversation is not dropped. + codeText := shared.CodeExecutionText(part) // Classified before any of the part is emitted, so media that cannot be // labeled fails the request instead of leaving with the text beside it. media, err := shared.ClassifyMedia(part) @@ -161,6 +165,10 @@ func convertContent(content *genai.Content, tracker *shared.CallTracker) ([]open texts = append(texts, part.Text) pieces = append(pieces, contentPiece{text: part.Text}) } + if codeText != "" { + texts = append(texts, codeText) + pieces = append(pieces, contentPiece{text: codeText}) + } for i := range media { pieces = append(pieces, contentPiece{media: &media[i]}) hasMedia = true @@ -178,7 +186,7 @@ func convertContent(content *genai.Content, tracker *shared.CallTracker) ([]open return nil, err } messages = append(messages, *msg) - case !sendText && len(media) == 0 && !shared.ReplayedReasoning(part): + case !sendText && codeText == "" && len(media) == 0 && !shared.ReplayedReasoning(part): return nil, errors.New("openai: unsupported content part: carries nothing to send") } } diff --git a/model/openaimodel/internal/completions/request_test.go b/model/openaimodel/internal/completions/request_test.go index 983f56d4dc..a0b409f26f 100644 --- a/model/openaimodel/internal/completions/request_test.go +++ b/model/openaimodel/internal/completions/request_test.go @@ -157,6 +157,73 @@ func TestBuildParams_MultiTurnHistory(t *testing.T) { } } +// TestBuildParams_CodeExecutionAsText pins the regression for a Gemini +// code-execution turn replayed into a Chat Completions request. The parts +// used to fail the request; they now go out as the same fenced text the +// Responses path sends, rather than being dropped. +func TestBuildParams_CodeExecutionAsText(t *testing.T) { + code := "Code:```python\nprint(1)\n```" + result := "Execution Result:```code_output\n1\n```" + tests := []struct { + name string + req *model.LLMRequest + want string + wantRole string + }{ + { + name: "user code", + req: userReq(&genai.Part{Text: "For context:"}, &genai.Part{ExecutableCode: &genai.ExecutableCode{Code: "print(1)"}}), + want: "For context:\n" + code, + wantRole: "user", + }, + { + name: "model code and result", + req: &model.LLMRequest{Contents: []*genai.Content{{ + Role: "model", + Parts: []*genai.Part{ + {ExecutableCode: &genai.ExecutableCode{Code: "print(1)", Language: genai.LanguagePython}}, + {CodeExecutionResult: &genai.CodeExecutionResult{Outcome: genai.OutcomeOK, Output: "1"}}, + }, + }}}, + want: code + "\n" + result, + wantRole: "assistant", + }, + { + name: "thought text is dropped and the code remains", + req: &model.LLMRequest{Contents: []*genai.Content{{ + Role: "model", + Parts: []*genai.Part{{ + Thought: true, + Text: "scratch", + ExecutableCode: &genai.ExecutableCode{Code: "print(1)"}, + }}, + }}}, + want: code, + wantRole: "assistant", + }, + { + name: "empty code is still sent", + req: userReq(&genai.Part{ExecutableCode: &genai.ExecutableCode{}}), + want: "Code:```python\n\n```", + wantRole: "user", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgs := wireMessages(t, requestWire(t, tt.req)) + if len(msgs) != 1 { + t.Fatalf("messages = %d, want 1: %#v", len(msgs), msgs) + } + if got := msgs[0]["role"]; got != tt.wantRole { + t.Errorf("role = %v, want %v", got, tt.wantRole) + } + if got := msgs[0]["content"]; got != tt.want { + t.Errorf("content = %q, want %q", got, tt.want) + } + }) + } +} + func TestBuildParams_TextPartsJoin(t *testing.T) { wire := requestWire(t, userReq(&genai.Part{Text: "one"}, &genai.Part{Text: " "}, &genai.Part{Text: "two"})) msgs := wireMessages(t, wire) diff --git a/model/openaimodel/internal/responses/request.go b/model/openaimodel/internal/responses/request.go index 1786f13ef8..29d8b6bbe4 100644 --- a/model/openaimodel/internal/responses/request.go +++ b/model/openaimodel/internal/responses/request.go @@ -152,9 +152,13 @@ func convertContents(contents []*genai.Content) (oairesponses.ResponseInputParam // the same part and would otherwise have carried it out unnoticed. // Media is excluded from the report because this endpoint emits it // below, as Chat Completions does. - if field := shared.UnsupportedPayload(part, shared.PartFieldMedia); field != "" { + if field := shared.UnsupportedPayload(part, shared.PartFieldMedia, shared.PartFieldCodeExecution); field != "" { return nil, false, fmt.Errorf("openai: unsupported content part: %s", field) } + // Same fenced text adk-python's Responses converter sends. Chat + // Completions uses it too: dropping the parts there would hide + // the code and its output from the model. + codeText := shared.CodeExecutionText(part) // Classified before any of it is emitted, for the same reason the // check above runs first: media this endpoint cannot label must // fail the request rather than leave with the text beside it. @@ -181,6 +185,9 @@ func convertContents(contents []*genai.Content) (oairesponses.ResponseInputParam return nil, false, err } } + if codeText != "" { + buffered = append(buffered, bufferedContent{text: codeText}) + } // After the part's own text, so that "describe this:" keeps its // place ahead of the image it introduces, and before the call // switch below, whose flush has to carry the media out with it. @@ -208,7 +215,7 @@ func convertContents(contents []*genai.Content) (oairesponses.ResponseInputParam return nil, false, err } items = append(items, oairesponses.ResponseInputItemUnionParam{OfFunctionCallOutput: respParam}) - case !sendText && len(media) == 0 && !shared.ReplayedReasoning(part): + case !sendText && codeText == "" && len(media) == 0 && !shared.ReplayedReasoning(part): // Nothing in the part reaches the request. It keeps the // unsupported-content-part prefix the single message used // before, so a caller matching on that still matches here. diff --git a/model/openaimodel/internal/responses/request_test.go b/model/openaimodel/internal/responses/request_test.go index afb6165f66..bde90ec176 100644 --- a/model/openaimodel/internal/responses/request_test.go +++ b/model/openaimodel/internal/responses/request_test.go @@ -380,28 +380,100 @@ func TestBuildParams_UnsupportedPart(t *testing.T) { // unsupported part leaves the request empty, so a build that skipped it // silently would still fail with shared.ErrNoContents and look like a rejection. // - // The part carries executable code rather than media: media has a wire - // field on this endpoint now, and the walk below covers what is left. + // The part carries a server-side tool call rather than media or code: + // both of those have a way out of this endpoint now, and the walk below + // covers what is left. req := &model.LLMRequest{ Contents: []*genai.Content{ genai.NewContentFromText("q", genai.RoleUser), { Role: string(genai.RoleUser), Parts: []*genai.Part{ - {ExecutableCode: &genai.ExecutableCode{Code: "print(1)"}}, + {ToolCall: &genai.ToolCall{ID: "t1"}}, }, }, }, } _, err := buildParams("fallback", req) if err == nil { - t.Fatalf("expected error for an executable code part") + t.Fatalf("expected error for a tool call part") } if errors.Is(err, shared.ErrNoContents) || !strings.Contains(err.Error(), "unsupported content part") { t.Errorf("buildParams() err = %v, want an unsupported-content-part error", err) } } +// TestBuildParams_CodeExecutionAsText pins the regression for a Gemini +// code-execution turn replayed into an OpenAI request: the parts used to +// fail the whole request, and now go out as the fenced text adk-python sends. +func TestBuildParams_CodeExecutionAsText(t *testing.T) { + tests := []struct { + name string + contents []*genai.Content + want []string + }{ + { + name: "code_and_result_on_a_model_turn", + contents: []*genai.Content{ + modelContent(&genai.Part{ExecutableCode: &genai.ExecutableCode{Code: "print(1)", Language: genai.LanguagePython}}), + modelContent(&genai.Part{CodeExecutionResult: &genai.CodeExecutionResult{Outcome: genai.OutcomeOK, Output: "1"}}), + }, + want: []string{ + "out/assistant:Code:```python\nprint(1)\n```", + "out/assistant:Execution Result:```code_output\n1\n```", + }, + }, + { + name: "text_keeps_its_place_ahead_of_the_code", + contents: []*genai.Content{ + userContent( + &genai.Part{Text: "For context:"}, + &genai.Part{ExecutableCode: &genai.ExecutableCode{Code: "print(1)"}}, + ), + }, + want: []string{"in/user:For context:|Code:```python\nprint(1)\n```"}, + }, + { + name: "code_keeps_its_place_on_a_media_message", + contents: []*genai.Content{ + userContent( + &genai.Part{ExecutableCode: &genai.ExecutableCode{Code: "print(1)"}}, + &genai.Part{InlineData: &genai.Blob{MIMEType: "image/png", Data: []byte{1}}}, + ), + }, + want: []string{"in/user:Code:```python\nprint(1)\n```|img/auto:image_url=data:image/png;base64,AQ=="}, + }, + { + name: "both_fields_on_one_part_render_code_then_result", + contents: []*genai.Content{ + userContent(&genai.Part{ + ExecutableCode: &genai.ExecutableCode{Code: "print(1)"}, + CodeExecutionResult: &genai.CodeExecutionResult{Output: "1"}, + }), + }, + want: []string{"in/user:Code:```python\nprint(1)\n```\nExecution Result:```code_output\n1\n```"}, + }, + { + name: "empty_code_is_still_sent", + contents: []*genai.Content{ + userContent(&genai.Part{ExecutableCode: &genai.ExecutableCode{}}), + }, + want: []string{"in/user:Code:```python\n\n```"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params, err := buildParams("fallback", &model.LLMRequest{Contents: tt.contents}) + if err != nil { + t.Fatalf("buildParams() err = %v", err) + } + if got := describeInput(params.Input.OfInputItemList); !reflect.DeepEqual(got, tt.want) { + t.Errorf("input items = %q, want %q", got, tt.want) + } + }) + } +} + // describeInput renders converted input items compactly, so a table case can // state the entire request body: "in/user:hi", "out/assistant:A|B" for an // assistant output message carrying two text items, "call:name/id", @@ -669,7 +741,9 @@ func TestBuildParams_DropsReplayedThoughts(t *testing.T) { want: []string{"in/user:q", "in/user:img/auto:image_url=data:image/png;base64,AQ=="}, }, { - name: "thought_text_riding_on_code_is_still_rejected", + // Marking the part as a thought drops the scratchpad and leaves + // the code, the same split media already gets. + name: "thought_text_riding_on_code_drops_only_the_text", contents: []*genai.Content{ genai.NewContentFromText("q", genai.RoleUser), modelTurn(&genai.Part{ @@ -678,7 +752,7 @@ func TestBuildParams_DropsReplayedThoughts(t *testing.T) { ExecutableCode: &genai.ExecutableCode{Code: "print(1)"}, }), }, - wantErrText: "unsupported content part", + want: []string{"in/user:q", "out/assistant:Code:```python\nprint(1)\n```"}, }, { // Dropping the reasoning must not swallow the role error the same @@ -1341,23 +1415,27 @@ func TestReplayedReasoning(t *testing.T) { // // Each endpoint opts into shared.PartFieldMedia when it emits media. Both // Responses and Chat Completions now do; the shared package separately tests -// the default rejection when a converter does not opt in. +// the default rejection when a converter does not opt in. Both endpoints also +// pass shared.PartFieldCodeExecution, so executable code and its result are +// accounted for here. // // The list is deliberately restated here instead of derived from the // production code, so a field genai adds later is absent from it by // construction and the walk below demands it be reported instead of silently // accepted — the failure a denylist would not produce. var accountedForFields = map[string]string{ - "Text": "sent, or dropped when it is reasoning", - "Thought": "the marker deciding which", - "ThoughtSignature": "no request field on either endpoint carries one", - "FunctionCall": "sent as a call", - "FunctionResponse": "sent as the call's result", - "VideoMetadata": "qualifies media carried in another field", - "MediaResolution": "qualifies media carried in another field", - "PartMetadata": "caller bookkeeping, never content", - "InlineData": "sent as an input_image or input_file carrying the bytes", - "FileData": "sent as an input_image or input_file naming an upload or a location", + "Text": "sent, or dropped when it is reasoning", + "Thought": "the marker deciding which", + "ThoughtSignature": "no request field on either endpoint carries one", + "FunctionCall": "sent as a call", + "FunctionResponse": "sent as the call's result", + "VideoMetadata": "qualifies media carried in another field", + "MediaResolution": "qualifies media carried in another field", + "PartMetadata": "caller bookkeeping, never content", + "InlineData": "sent as an input_image or input_file carrying the bytes", + "FileData": "sent as an input_image or input_file naming an upload or a location", + "ExecutableCode": "sent as a fenced python block", + "CodeExecutionResult": "sent as a fenced code_output block", } func TestUnsupportedPayload_WalksEveryPartField(t *testing.T) { @@ -1372,7 +1450,7 @@ func TestUnsupportedPayload_WalksEveryPartField(t *testing.T) { part := &genai.Part{} reflect.ValueOf(part).Elem().Field(i).Set(nonZero(t, field.Type)) - got := shared.UnsupportedPayload(part, shared.PartFieldMedia) + got := shared.UnsupportedPayload(part, shared.PartFieldMedia, shared.PartFieldCodeExecution) if why, accounted := accountedForFields[field.Name]; accounted { if got != "" { t.Errorf("UnsupportedPayload() = %q for a part carrying only %s, want %q: %s", diff --git a/model/openaimodel/internal/shared/contents.go b/model/openaimodel/internal/shared/contents.go index 5a2d2e3a2a..5041ca0452 100644 --- a/model/openaimodel/internal/shared/contents.go +++ b/model/openaimodel/internal/shared/contents.go @@ -104,6 +104,9 @@ const ( // one is able to send the other: they differ in where the bytes are, not // in what the wire field is. PartFieldMedia PartField = iota + // PartFieldCodeExecution is ExecutableCode and CodeExecutionResult, which + // both endpoints send as text. See CodeExecutionText. + PartFieldCodeExecution ) // UnsupportedPayload names the first field on part that the calling endpoint @@ -130,7 +133,8 @@ func UnsupportedPayload(part *genai.Part, emitted ...PartField) string { rest.PartMetadata = nil // caller bookkeeping, never content for _, field := range emitted { - if field == PartFieldMedia { + switch field { + case PartFieldMedia: rest.InlineData = nil // the caller has a wire field for the bytes rest.FileData = nil // and for the id or location naming them // A qualifier is only harmless while nothing is sent for it to @@ -145,6 +149,9 @@ func UnsupportedPayload(part *genai.Part, emitted ...PartField) string { rest.MediaResolution = part.MediaResolution rest.VideoMetadata = part.VideoMetadata } + case PartFieldCodeExecution: + rest.ExecutableCode = nil + rest.CodeExecutionResult = nil } } @@ -156,3 +163,32 @@ func UnsupportedPayload(part *genai.Part, emitted ...PartField) string { } return "" } + +// CodeExecutionText renders Gemini code-execution parts as the fenced text +// adk-python's Responses converter sends, so an OpenAI model sharing the +// session can read the code and its output. The fence language is always +// python, matching that converter, whatever ExecutableCode.Language says. +// +// An empty code or output is still rendered: the part is present, and +// dropping it would hide that a run happened. A part carrying both fields +// renders the code first, then the result. +func CodeExecutionText(part *genai.Part) string { + if part == nil { + return "" + } + var b strings.Builder + if part.ExecutableCode != nil { + b.WriteString("Code:```python\n") + b.WriteString(part.ExecutableCode.Code) + b.WriteString("\n```") + } + if part.CodeExecutionResult != nil { + if b.Len() > 0 { + b.WriteByte('\n') + } + b.WriteString("Execution Result:```code_output\n") + b.WriteString(part.CodeExecutionResult.Output) + b.WriteString("\n```") + } + return b.String() +} diff --git a/model/openaimodel/internal/shared/contents_test.go b/model/openaimodel/internal/shared/contents_test.go index 8e1cb63404..4f3340939e 100644 --- a/model/openaimodel/internal/shared/contents_test.go +++ b/model/openaimodel/internal/shared/contents_test.go @@ -217,6 +217,50 @@ func TestUnsupportedPayload_EmittingMediaDoesNotHideAnythingElse(t *testing.T) { if got := UnsupportedPayload(part, PartFieldMedia); got != "ExecutableCode" { t.Errorf("UnsupportedPayload(PartFieldMedia) = %q, want %q", got, "ExecutableCode") } + if got := UnsupportedPayload(part, PartFieldMedia, PartFieldCodeExecution); got != "" { + t.Errorf("UnsupportedPayload(PartFieldMedia, PartFieldCodeExecution) = %q, want %q", got, "") + } +} + +func TestCodeExecutionText(t *testing.T) { + tests := []struct { + name string + part *genai.Part + want string + }{ + {name: "nil", part: nil, want: ""}, + {name: "neither field", part: &genai.Part{Text: "hi"}, want: ""}, + { + name: "code", + part: &genai.Part{ExecutableCode: &genai.ExecutableCode{Code: "print(1)", Language: genai.LanguagePython}}, + want: "Code:```python\nprint(1)\n```", + }, + { + name: "result", + part: &genai.Part{CodeExecutionResult: &genai.CodeExecutionResult{Outcome: genai.OutcomeOK, Output: "1"}}, + want: "Execution Result:```code_output\n1\n```", + }, + { + name: "both, code first", + part: &genai.Part{ + ExecutableCode: &genai.ExecutableCode{Code: "print(1)"}, + CodeExecutionResult: &genai.CodeExecutionResult{Output: "1"}, + }, + want: "Code:```python\nprint(1)\n```\nExecution Result:```code_output\n1\n```", + }, + { + name: "empty code is still labeled", + part: &genai.Part{ExecutableCode: &genai.ExecutableCode{}}, + want: "Code:```python\n\n```", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := CodeExecutionText(tt.part); got != tt.want { + t.Errorf("CodeExecutionText() = %q, want %q", got, tt.want) + } + }) + } } // TestReplayedReasoning_MediaStillDisqualifies states that the drop was not