Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions model/openaimodel/internal/completions/media_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}}},
Expand Down
12 changes: 10 additions & 2 deletions model/openaimodel/internal/completions/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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")
}
}
Expand Down
67 changes: 67 additions & 0 deletions model/openaimodel/internal/completions/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions model/openaimodel/internal/responses/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
114 changes: 96 additions & 18 deletions model/openaimodel/internal/responses/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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",
Expand Down
38 changes: 37 additions & 1 deletion model/openaimodel/internal/shared/contents.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
}
}

Expand All @@ -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()
}
Loading
Loading