Skip to content

Commit 841e807

Browse files
fix(go-adk): extract system prompt from genai.GenerateContentConfig in Ollama and Bedrock model providers (#2118)
Closes #2114 Updated current unit tests, manually tested with Bedrock and Ollama models. --------- Signed-off-by: Jet Chiang <pokyuen.jetchiang-ext@solo.io>
1 parent 83c8ae9 commit 841e807

9 files changed

Lines changed: 191 additions & 40 deletions

File tree

go/adk/pkg/models/anthropic_adk.go

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -103,17 +103,7 @@ func applyAnthropicConfig(params *anthropic.MessageNewParams, cfg *AnthropicConf
103103
}
104104

105105
func genaiContentsToAnthropicMessages(contents []*genai.Content, config *genai.GenerateContentConfig) ([]anthropic.MessageParam, string) {
106-
// Extract system instruction
107-
var systemBuilder strings.Builder
108-
if config != nil && config.SystemInstruction != nil {
109-
for _, p := range config.SystemInstruction.Parts {
110-
if p != nil && p.Text != "" {
111-
systemBuilder.WriteString(p.Text)
112-
systemBuilder.WriteByte('\n')
113-
}
114-
}
115-
}
116-
systemPrompt := strings.TrimSpace(systemBuilder.String())
106+
systemPrompt := mergeSystemInstructionFromConfig("", config)
117107

118108
// Collect function responses for matching with function calls
119109
functionResponses := make(map[string]*genai.FunctionResponse)

go/adk/pkg/models/base.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,3 +166,23 @@ func lowercaseSchemaTypes(m map[string]any) {
166166
}
167167
}
168168
}
169+
170+
// mergeSystemInstructionFromConfig appends Config.SystemInstruction parts to any
171+
// system text extracted from conversation contents. The Google ADK delivers the
172+
// agent Instruction via Config.SystemInstruction, not as role-"system" Content.
173+
func mergeSystemInstructionFromConfig(existing string, config *genai.GenerateContentConfig) string {
174+
if config == nil || config.SystemInstruction == nil {
175+
return strings.TrimSpace(existing)
176+
}
177+
var b strings.Builder
178+
b.WriteString(existing)
179+
for _, p := range config.SystemInstruction.Parts {
180+
if p != nil && p.Text != "" {
181+
if b.Len() > 0 {
182+
b.WriteByte('\n')
183+
}
184+
b.WriteString(p.Text)
185+
}
186+
}
187+
return strings.TrimSpace(b.String())
188+
}

go/adk/pkg/models/base_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package models
2+
3+
import (
4+
"testing"
5+
6+
"google.golang.org/genai"
7+
)
8+
9+
func TestMergeSystemInstructionFromConfig(t *testing.T) {
10+
tests := []struct {
11+
name string
12+
existing string
13+
config *genai.GenerateContentConfig
14+
want string
15+
}{
16+
{
17+
name: "nil config returns trimmed existing",
18+
existing: " hello ",
19+
want: "hello",
20+
},
21+
{
22+
name: "config only",
23+
config: &genai.GenerateContentConfig{
24+
SystemInstruction: &genai.Content{
25+
Parts: []*genai.Part{
26+
{Text: "You are helpful."},
27+
{Text: "Be concise."},
28+
},
29+
},
30+
},
31+
want: "You are helpful.\nBe concise.",
32+
},
33+
{
34+
name: "skips empty text parts",
35+
config: &genai.GenerateContentConfig{
36+
SystemInstruction: &genai.Content{
37+
Parts: []*genai.Part{
38+
{Text: " one "},
39+
{Text: ""},
40+
{Text: "two"},
41+
},
42+
},
43+
},
44+
want: "one \ntwo",
45+
},
46+
{
47+
name: "merges existing with config",
48+
existing: "From contents",
49+
config: &genai.GenerateContentConfig{
50+
SystemInstruction: &genai.Content{
51+
Parts: []*genai.Part{{Text: "From config"}},
52+
},
53+
},
54+
want: "From contents\nFrom config",
55+
},
56+
}
57+
58+
for _, tt := range tests {
59+
t.Run(tt.name, func(t *testing.T) {
60+
got := mergeSystemInstructionFromConfig(tt.existing, tt.config)
61+
if got != tt.want {
62+
t.Errorf("mergeSystemInstructionFromConfig() = %q, want %q", got, tt.want)
63+
}
64+
})
65+
}
66+
}

go/adk/pkg/models/bedrock.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func (m *BedrockModel) GenerateContent(ctx context.Context, req *model.LLMReques
196196
// Convert content to Bedrock messages.
197197
// nameMap is passed so that any tool call recorded in conversation history
198198
// is written with the sanitized name Bedrock already knows about.
199-
messages, systemInstruction := convertGenaiContentsToBedrockMessages(req.Contents, nameMap)
199+
messages, systemInstruction := convertGenaiContentsToBedrockMessages(req.Contents, nameMap, req.Config)
200200

201201
// temperature/top_p must not be sent when thinking is active. https://docs.aws.amazon.com/bedrock/latest/userguide/claude-messages-extended-thinking.html
202202
_, thinkingEnabled := m.Config.AdditionalModelRequestFields["thinking"]
@@ -559,7 +559,7 @@ func truncateToolResult(s string, maxLen int) string {
559559
// nameMap is the original->sanitized tool name map produced by convertGenaiToolsToBedrock.
560560
// Any FunctionCall found in the conversation history is written with the sanitized name so
561561
// that Bedrock can correlate it with the tool spec it already received. A nil nameMap is safe.
562-
func convertGenaiContentsToBedrockMessages(contents []*genai.Content, nameMap map[string]string) ([]types.Message, string) {
562+
func convertGenaiContentsToBedrockMessages(contents []*genai.Content, nameMap map[string]string, config *genai.GenerateContentConfig) ([]types.Message, string) {
563563
var messages []types.Message
564564
var systemInstruction string
565565

@@ -684,7 +684,7 @@ func convertGenaiContentsToBedrockMessages(contents []*genai.Content, nameMap ma
684684
}
685685
}
686686

687-
return messages, systemInstruction
687+
return messages, mergeSystemInstructionFromConfig(systemInstruction, config)
688688
}
689689

690690
// convertGenaiToolsToBedrock converts genai.Tool to Bedrock Tool format.

go/adk/pkg/models/bedrock_test.go

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ func TestConvertGenaiContentsToBedrockMessages(t *testing.T) {
3737
tests := []struct {
3838
name string
3939
contents []*genai.Content
40+
config *genai.GenerateContentConfig
4041
wantMsgCount int
4142
wantSystemText string
4243
checkMsg func(t *testing.T, msgs []types.Message)
@@ -57,6 +58,36 @@ func TestConvertGenaiContentsToBedrockMessages(t *testing.T) {
5758
wantMsgCount: 1,
5859
wantSystemText: "You are a helpful assistant",
5960
},
61+
{
62+
name: "system instruction from config",
63+
contents: []*genai.Content{
64+
{Role: "user", Parts: []*genai.Part{{Text: "Hello"}}},
65+
},
66+
config: &genai.GenerateContentConfig{
67+
SystemInstruction: &genai.Content{
68+
Parts: []*genai.Part{
69+
{Text: "You are a test agent."},
70+
{Text: "Begin EVERY reply with ZEBRA9."},
71+
},
72+
},
73+
},
74+
wantMsgCount: 1,
75+
wantSystemText: "You are a test agent.\nBegin EVERY reply with ZEBRA9.",
76+
},
77+
{
78+
name: "system instruction from config merges with content role system",
79+
contents: []*genai.Content{
80+
{Role: "system", Parts: []*genai.Part{{Text: "From contents"}}},
81+
{Role: "user", Parts: []*genai.Part{{Text: "Hello"}}},
82+
},
83+
config: &genai.GenerateContentConfig{
84+
SystemInstruction: &genai.Content{
85+
Parts: []*genai.Part{{Text: "From config"}},
86+
},
87+
},
88+
wantMsgCount: 1,
89+
wantSystemText: "From contents\nFrom config",
90+
},
6091
{
6192
name: "user and model conversation",
6293
contents: []*genai.Content{
@@ -142,7 +173,7 @@ func TestConvertGenaiContentsToBedrockMessages(t *testing.T) {
142173

143174
for _, tt := range tests {
144175
t.Run(tt.name, func(t *testing.T) {
145-
msgs, systemText := convertGenaiContentsToBedrockMessages(tt.contents, nil)
176+
msgs, systemText := convertGenaiContentsToBedrockMessages(tt.contents, nil, tt.config)
146177
if len(msgs) != tt.wantMsgCount {
147178
t.Errorf("expected %d messages, got %d", tt.wantMsgCount, len(msgs))
148179
}
@@ -487,7 +518,7 @@ func TestThinkingOnlyInLastAssistantTurn(t *testing.T) {
487518
},
488519
}
489520

490-
msgs, _ := convertGenaiContentsToBedrockMessages(contents, nil)
521+
msgs, _ := convertGenaiContentsToBedrockMessages(contents, nil, nil)
491522
if len(msgs) != 4 {
492523
t.Fatalf("want 4 messages, got %d", len(msgs))
493524
}
@@ -524,7 +555,7 @@ func TestHistoricalToolResultTruncation(t *testing.T) {
524555
},
525556
}
526557

527-
msgs, _ := convertGenaiContentsToBedrockMessages(contents, nil)
558+
msgs, _ := convertGenaiContentsToBedrockMessages(contents, nil, nil)
528559
if len(msgs) != 2 {
529560
t.Fatalf("want 2 messages, got %d", len(msgs))
530561
}

go/adk/pkg/models/ollama_adk.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func (m *OllamaModel) GenerateContent(ctx context.Context, req *model.LLMRequest
3030
}
3131

3232
// Convert content to Ollama messages
33-
messages, systemInstruction := convertGenaiContentsToOllamaMessages(req.Contents)
33+
messages, systemInstruction := convertGenaiContentsToOllamaMessages(req.Contents, req.Config)
3434

3535
// Add system instruction as first message if present
3636
if systemInstruction != "" {
@@ -233,7 +233,7 @@ func (m *OllamaModel) generateNonStreaming(ctx context.Context, modelName string
233233

234234
// convertGenaiContentsToOllamaMessages converts genai.Content to Ollama message format.
235235
// Returns messages and system instruction (extracted from system role content).
236-
func convertGenaiContentsToOllamaMessages(contents []*genai.Content) ([]api.Message, string) {
236+
func convertGenaiContentsToOllamaMessages(contents []*genai.Content, config *genai.GenerateContentConfig) ([]api.Message, string) {
237237
var messages []api.Message
238238
var systemInstruction string
239239

@@ -328,7 +328,7 @@ func convertGenaiContentsToOllamaMessages(contents []*genai.Content) ([]api.Mess
328328
}
329329
}
330330

331-
return messages, systemInstruction
331+
return messages, mergeSystemInstructionFromConfig(systemInstruction, config)
332332
}
333333

334334
// convertGenaiToolsToOllama converts genai.Tool to Ollama tool format.

go/adk/pkg/models/ollama_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package models
33
import (
44
"reflect"
55
"testing"
6+
7+
"google.golang.org/genai"
68
)
79

810
func TestConvertOllamaOptions(t *testing.T) {
@@ -152,3 +154,63 @@ func TestOllamaConfigDefaults(t *testing.T) {
152154
t.Errorf("expected temperature 0.8, got %v", converted["temperature"])
153155
}
154156
}
157+
158+
func TestConvertGenaiContentsToOllamaMessages(t *testing.T) {
159+
tests := []struct {
160+
name string
161+
contents []*genai.Content
162+
config *genai.GenerateContentConfig
163+
wantMsgCount int
164+
wantSystemText string
165+
}{
166+
{
167+
name: "simple user message",
168+
contents: []*genai.Content{
169+
{Role: "user", Parts: []*genai.Part{{Text: "Hello"}}},
170+
},
171+
wantMsgCount: 1,
172+
},
173+
{
174+
name: "system instruction from config",
175+
contents: []*genai.Content{
176+
{Role: "user", Parts: []*genai.Part{{Text: "Hello"}}},
177+
},
178+
config: &genai.GenerateContentConfig{
179+
SystemInstruction: &genai.Content{
180+
Parts: []*genai.Part{
181+
{Text: "You are a test agent."},
182+
{Text: "Begin EVERY reply with ZEBRA9."},
183+
},
184+
},
185+
},
186+
wantMsgCount: 1,
187+
wantSystemText: "You are a test agent.\nBegin EVERY reply with ZEBRA9.",
188+
},
189+
{
190+
name: "system instruction from config merges with content role system",
191+
contents: []*genai.Content{
192+
{Role: "system", Parts: []*genai.Part{{Text: "From contents"}}},
193+
{Role: "user", Parts: []*genai.Part{{Text: "Hello"}}},
194+
},
195+
config: &genai.GenerateContentConfig{
196+
SystemInstruction: &genai.Content{
197+
Parts: []*genai.Part{{Text: "From config"}},
198+
},
199+
},
200+
wantMsgCount: 1,
201+
wantSystemText: "From contents\nFrom config",
202+
},
203+
}
204+
205+
for _, tt := range tests {
206+
t.Run(tt.name, func(t *testing.T) {
207+
msgs, systemText := convertGenaiContentsToOllamaMessages(tt.contents, tt.config)
208+
if len(msgs) != tt.wantMsgCount {
209+
t.Errorf("expected %d messages, got %d", tt.wantMsgCount, len(msgs))
210+
}
211+
if systemText != tt.wantSystemText {
212+
t.Errorf("expected system text %q, got %q", tt.wantSystemText, systemText)
213+
}
214+
})
215+
}
216+
}

go/adk/pkg/models/openai_adk.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -195,16 +195,7 @@ func applyOpenAIConfig(params *openai.ChatCompletionNewParams, cfg *OpenAIConfig
195195
}
196196

197197
func genaiContentsToOpenAIMessages(contents []*genai.Content, config *genai.GenerateContentConfig) ([]openai.ChatCompletionMessageParamUnion, string) {
198-
var systemBuilder strings.Builder
199-
if config != nil && config.SystemInstruction != nil {
200-
for _, p := range config.SystemInstruction.Parts {
201-
if p != nil && p.Text != "" {
202-
systemBuilder.WriteString(p.Text)
203-
systemBuilder.WriteByte('\n')
204-
}
205-
}
206-
}
207-
systemInstruction := strings.TrimSpace(systemBuilder.String())
198+
systemInstruction := mergeSystemInstructionFromConfig("", config)
208199

209200
functionResponses := make(map[string]*genai.FunctionResponse)
210201
thoughtSignatures := thoughtSignaturesByToolCallID(contents)

go/adk/pkg/models/sapaicore_adk.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -169,16 +169,7 @@ func (m *SAPAICoreModel) buildOrchestrationBody(req *model.LLMRequest, stream bo
169169
}
170170

171171
func genaiContentsToOrchTemplate(contents []*genai.Content, config *genai.GenerateContentConfig) ([]map[string]any, string) {
172-
var systemBuilder strings.Builder
173-
if config != nil && config.SystemInstruction != nil {
174-
for _, p := range config.SystemInstruction.Parts {
175-
if p != nil && p.Text != "" {
176-
systemBuilder.WriteString(p.Text)
177-
systemBuilder.WriteByte('\n')
178-
}
179-
}
180-
}
181-
systemInstruction := strings.TrimSpace(systemBuilder.String())
172+
systemInstruction := mergeSystemInstructionFromConfig("", config)
182173

183174
functionResponses := make(map[string]*genai.FunctionResponse)
184175
for _, c := range contents {

0 commit comments

Comments
 (0)