From c7c2837c96cd6fcc0d0424e0a096ed24516d3d59 Mon Sep 17 00:00:00 2001 From: Parth Sareen Date: Wed, 29 Apr 2026 18:40:23 -0700 Subject: [PATCH] renderers: update gemma4 renderer (#15886) --- model/renderers/gemma4.go | 7 ++- model/renderers/gemma4_reference_test.go | 48 ++++++++++++++++++- .../testdata/gemma4_31b_chat_template.jinja2 | 27 +++++++---- .../testdata/gemma4_e2b_chat_template.jinja2 | 27 +++++++---- 4 files changed, 86 insertions(+), 23 deletions(-) diff --git a/model/renderers/gemma4.go b/model/renderers/gemma4.go index 4d8baa53..bbf717f3 100644 --- a/model/renderers/gemma4.go +++ b/model/renderers/gemma4.go @@ -71,7 +71,6 @@ func (r *Gemma4Renderer) Render(messages []api.Message, tools []api.Tool, thinkV continue } - messageHadContent := r.messageHasContent(message) prevMessageType = "" role := message.Role if role == "assistant" { @@ -105,14 +104,18 @@ func (r *Gemma4Renderer) Render(messages []api.Message, tools []api.Tool, thinkV } } + messageHadContent := false switch role { case "model": if message.Content != "" || len(message.Images) > 0 { message.Content = stripThinking(message.Content) r.renderContent(&sb, message, &imageOffset, false) + messageHadContent = r.messageHasContent(message) } default: r.renderContent(&sb, message, &imageOffset, true) + message.Content = strings.TrimSpace(message.Content) + messageHadContent = r.messageHasContent(message) } if prevMessageType == "tool_call" && !toolResponsesEmitted { @@ -180,7 +183,7 @@ func (r *Gemma4Renderer) previousNonToolRole(messages []api.Message, idx int) st } func (r *Gemma4Renderer) messageHasContent(message api.Message) bool { - return message.Content != "" || len(message.Images) > 0 + return strings.TrimSpace(message.Content) != "" || len(message.Images) > 0 } func (r *Gemma4Renderer) toolResponseName(message api.Message, toolCalls []api.ToolCall) string { diff --git a/model/renderers/gemma4_reference_test.go b/model/renderers/gemma4_reference_test.go index cb5a4696..ed91bd41 100644 --- a/model/renderers/gemma4_reference_test.go +++ b/model/renderers/gemma4_reference_test.go @@ -739,6 +739,18 @@ func TestGemma4RendererMatchesReference(t *testing.T) { think: thinkTrue(), expected: "<|turn>system\n<|think|>\nYou are helpful." + bashDeclRef + "\n<|turn>user\nHi\n<|turn>model\n", }, + { + name: "developer_tools_thinking_trimmed", + messages: []api.Message{ + {Role: "developer", Content: " Prefer terse answers.\nUse tools when needed. "}, + {Role: "user", Content: "Hi"}, + }, + tools: bashAndReadRefTools(), + think: thinkTrue(), + expected: "<|turn>system\n<|think|>\nPrefer terse answers.\nUse tools when needed." + + bashDeclRef + readDeclRef + "\n" + + "<|turn>user\nHi\n<|turn>model\n", + }, { name: "thinking_explicitly_disabled", messages: []api.Message{{Role: "user", Content: "Hi"}}, @@ -845,6 +857,40 @@ func TestGemma4RendererMatchesReference(t *testing.T) { "<|tool_response>response:read{value:" + q + "module example.com/foo" + q + "}", skipJinja2: true, }, + { + // Multiple tool calls in one assistant round, with explicit thinking, + // tool_call_id resolution, out-of-order tool responses, and a final + // assistant continuation in the same model turn. + name: "multiple_tool_calls_with_thinking_ids_and_continuation", + messages: []api.Message{ + {Role: "system", Content: "You are a coding assistant."}, + {Role: "user", Content: "List files, then read config."}, + {Role: "assistant", Thinking: "Need the directory listing before reading the config.", ToolCalls: []api.ToolCall{ + { + ID: "call_bash", + Function: api.ToolCallFunction{Name: "bash", Arguments: testArgs(map[string]any{"command": "ls -la"})}, + }, + { + ID: "call_read", + Function: api.ToolCallFunction{Name: "read", Arguments: testArgs(map[string]any{"path": "config.json"})}, + }, + }}, + {Role: "tool", ToolCallID: "call_read", Content: `{"debug": true}`}, + {Role: "tool", ToolCallID: "call_bash", Content: "config.json\nmain.go"}, + {Role: "assistant", Content: "Config loaded."}, + }, + tools: bashAndReadRefTools(), + think: thinkTrue(), + expected: "<|turn>system\n<|think|>\nYou are a coding assistant." + bashDeclRef + readDeclRef + "\n" + + "<|turn>user\nList files, then read config.\n" + + "<|turn>model\n<|channel>thought\nNeed the directory listing before reading the config.\n" + + "<|tool_call>call:bash{command:" + q + "ls -la" + q + "}" + + "<|tool_call>call:read{path:" + q + "config.json" + q + "}" + + "<|tool_response>response:read{value:" + q + `{"debug": true}` + q + "}" + + "<|tool_response>response:bash{value:" + q + "config.json\nmain.go" + q + "}" + + "Config loaded.\n" + + "<|turn>model\n", + }, { // Thinking content in assistant history should be stripped name: "strip_thinking_history", @@ -1338,7 +1384,7 @@ Hi "<|turn>user\nList files\n" + "<|turn>model\n<|tool_call>call:bash{command:" + q + "ls" + q + "}" + "<|tool_response>response:bash{value:" + q + "file1.txt" + q + "}" + - "\nHere are the files.\n" + + "Here are the files.\n" + "<|turn>user\nThanks\n" + "<|turn>model\n", }, diff --git a/model/renderers/testdata/gemma4_31b_chat_template.jinja2 b/model/renderers/testdata/gemma4_31b_chat_template.jinja2 index 98da08eb..f62ca843 100644 --- a/model/renderers/testdata/gemma4_31b_chat_template.jinja2 +++ b/model/renderers/testdata/gemma4_31b_chat_template.jinja2 @@ -1,9 +1,9 @@ -{%- macro format_parameters(properties, required) -%} +{%- macro format_parameters(properties, required, filter_keys=false) -%} {%- set standard_keys = ['description', 'type', 'properties', 'required', 'nullable'] -%} {%- set ns = namespace(found_first=false) -%} {%- for key, value in properties | dictsort -%} {%- set add_comma = false -%} - {%- if key not in standard_keys -%} + {%- if not filter_keys or key not in standard_keys -%} {%- if ns.found_first %},{% endif -%} {%- set ns.found_first = true -%} {{ key }}:{ @@ -65,7 +65,7 @@ {%- elif value is mapping -%} {%- if add_comma %},{%- else -%} {%- set add_comma = true -%} {% endif -%} properties:{ - {{- format_parameters(value, value['required'] | default([])) -}} + {{- format_parameters(value, value['required'] | default([]), filter_keys=true) -}} } {%- endif -%} {%- if value['required'] -%} @@ -178,18 +178,21 @@ {#- Handle System/Tool Definitions Block -#} {%- if (enable_thinking is defined and enable_thinking) or tools or messages[0]['role'] in ['system', 'developer'] -%} {{- '<|turn>system\n' -}} - {#- Inject Thinking token at the very top of the FIRST system turn -#} {%- if enable_thinking is defined and enable_thinking -%} {{- '<|think|>\n' -}} {%- set ns.prev_message_type = 'think' -%} {%- endif -%} - {%- if messages[0]['role'] in ['system', 'developer'] -%} - {{- messages[0]['content'] | trim -}} + {%- if messages[0]['content'] is string -%} + {{- messages[0]['content'] | trim -}} + {%- elif messages[0]['content'] is sequence -%} + {%- for item in messages[0]['content'] -%} + {{- item['text'] | trim + ' '-}} + {%- endfor -%} + {%- endif -%} {%- set loop_messages = messages[1:] -%} {%- endif -%} - {%- if tools -%} {%- for tool in tools %} {{- '<|tool>' -}} @@ -198,7 +201,6 @@ {%- endfor %} {%- set ns.prev_message_type = 'tool' -%} {%- endif -%} - {{- '\n' -}} {%- endif %} @@ -302,6 +304,7 @@ {%- endfor -%} {%- endif -%} + {%- set captured_content -%} {%- if message['content'] is string -%} {%- if role == 'model' -%} {{- strip_thinking(message['content']) -}} @@ -328,10 +331,14 @@ {%- endif -%} {%- endfor -%} {%- endif -%} + {%- endset -%} + + {{- captured_content -}} + {%- set has_content = captured_content | trim | length > 0 -%} {%- if ns.prev_message_type == 'tool_call' and not ns_tr_out.flag -%} {{- '<|tool_response>' -}} - {%- elif not (ns_tr_out.flag and not message.get('content')) -%} + {%- elif not (ns_tr_out.flag and not has_content) -%} {{- '\n' -}} {%- endif -%} {%- endif -%} @@ -344,4 +351,4 @@ {{- '<|channel>thought\n' -}} {%- endif -%} {%- endif -%} -{%- endif -%} +{%- endif -%} \ No newline at end of file diff --git a/model/renderers/testdata/gemma4_e2b_chat_template.jinja2 b/model/renderers/testdata/gemma4_e2b_chat_template.jinja2 index 6ec327b0..dc032e46 100644 --- a/model/renderers/testdata/gemma4_e2b_chat_template.jinja2 +++ b/model/renderers/testdata/gemma4_e2b_chat_template.jinja2 @@ -1,9 +1,9 @@ -{%- macro format_parameters(properties, required) -%} +{%- macro format_parameters(properties, required, filter_keys=false) -%} {%- set standard_keys = ['description', 'type', 'properties', 'required', 'nullable'] -%} {%- set ns = namespace(found_first=false) -%} {%- for key, value in properties | dictsort -%} {%- set add_comma = false -%} - {%- if key not in standard_keys -%} + {%- if not filter_keys or key not in standard_keys -%} {%- if ns.found_first %},{% endif -%} {%- set ns.found_first = true -%} {{ key }}:{ @@ -65,7 +65,7 @@ {%- elif value is mapping -%} {%- if add_comma %},{%- else -%} {%- set add_comma = true -%} {% endif -%} properties:{ - {{- format_parameters(value, value['required'] | default([])) -}} + {{- format_parameters(value, value['required'] | default([]), filter_keys=true) -}} } {%- endif -%} {%- if value['required'] -%} @@ -178,18 +178,21 @@ {#- Handle System/Tool Definitions Block -#} {%- if (enable_thinking is defined and enable_thinking) or tools or messages[0]['role'] in ['system', 'developer'] -%} {{- '<|turn>system\n' -}} - {#- Inject Thinking token at the very top of the FIRST system turn -#} {%- if enable_thinking is defined and enable_thinking -%} {{- '<|think|>\n' -}} {%- set ns.prev_message_type = 'think' -%} {%- endif -%} - {%- if messages[0]['role'] in ['system', 'developer'] -%} - {{- messages[0]['content'] | trim -}} + {%- if messages[0]['content'] is string -%} + {{- messages[0]['content'] | trim -}} + {%- elif messages[0]['content'] is sequence -%} + {%- for item in messages[0]['content'] -%} + {{- item['text'] | trim + ' '-}} + {%- endfor -%} + {%- endif -%} {%- set loop_messages = messages[1:] -%} {%- endif -%} - {%- if tools -%} {%- for tool in tools %} {{- '<|tool>' -}} @@ -198,7 +201,6 @@ {%- endfor %} {%- set ns.prev_message_type = 'tool' -%} {%- endif -%} - {{- '\n' -}} {%- endif %} @@ -302,6 +304,7 @@ {%- endfor -%} {%- endif -%} + {%- set captured_content -%} {%- if message['content'] is string -%} {%- if role == 'model' -%} {{- strip_thinking(message['content']) -}} @@ -328,10 +331,14 @@ {%- endif -%} {%- endfor -%} {%- endif -%} + {%- endset -%} + + {{- captured_content -}} + {%- set has_content = captured_content | trim | length > 0 -%} {%- if ns.prev_message_type == 'tool_call' and not ns_tr_out.flag -%} {{- '<|tool_response>' -}} - {%- elif not (ns_tr_out.flag and not message.get('content')) -%} + {%- elif not (ns_tr_out.flag and not has_content) -%} {{- '\n' -}} {%- endif -%} {%- endif -%} @@ -341,4 +348,4 @@ {%- if ns.prev_message_type != 'tool_response' and ns.prev_message_type != 'tool_call' -%} {{- '<|turn>model\n' -}} {%- endif -%} -{%- endif -%} +{%- endif -%} \ No newline at end of file