Skip to content

fix(tokenizers): parse Jinja tuple literals in grouping parens (#409) #415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jamesburton wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

fix(tokenizers): parse Jinja tuple literals in grouping parens (#409) #415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 33 additions & 2 deletions src/DotLLM.Tokenizers/ChatTemplates/JinjaParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,10 +591,41 @@ private IExpression ParsePrimary()

case JinjaTokenType.LeftParen:
{
// Grouping `(expr)` vs. tuple literal `(a, b, ...)` / `(a,)` / `()`.
//
// Jinja2/Python semantics: a comma inside the parens — including a lone
// trailing comma after a single item — makes this a tuple; a bare `(expr)`
// with no comma is plain grouping and evaluates to `expr` itself. Tuples are
// represented with the same <see cref="ListExpr"/> node used for `[...]`
// literals since the evaluator treats both as ordered sequences.
Advance();
var expr = ParseExpression();

if (CurrentIs(JinjaTokenType.RightParen))
{
// `()` — empty tuple.
Advance();
return new ListExpr([]);
}

var first = ParseExpression();

if (CurrentIs(JinjaTokenType.Comma))
{
var items = new List<IExpression> { first };
while (CurrentIs(JinjaTokenType.Comma))
{
Advance();
if (CurrentIs(JinjaTokenType.RightParen))
break; // trailing comma
items.Add(ParseExpression());
}

Expect(JinjaTokenType.RightParen);
return new ListExpr(items);
}

Expect(JinjaTokenType.RightParen);
return expr;
return first;
}

case JinjaTokenType.LeftBracket:
Expand Down
170 changes: 170 additions & 0 deletions tests/DotLLM.Tests.Unit/Tokenizers/ChatTemplates/Fixtures/qwen3.8-27b-chat-template.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
{%- set image_count = namespace(value=0) %}
{%- set video_count = namespace(value=0) %}
{%- macro render_content(content, do_vision_count, is_system_content=false) %}
{%- if content is string %}
{{- content }}
{%- elif content is iterable and content is not mapping %}
{%- for item in content %}
{%- if 'image' in item or 'image_url' in item or item.type == 'image' %}
{%- if is_system_content %}
{{- raise_exception('System message cannot contain images.') }}
{%- endif %}
{%- if do_vision_count %}
{%- set image_count.value = image_count.value + 1 %}
{%- endif %}
{%- if add_vision_id %}
{{- 'Picture ' ~ image_count.value ~ ': ' }}
{%- endif %}
{{- '<|vision_start|><|image_pad|><|vision_end|>' }}
{%- elif 'video' in item or item.type == 'video' %}
{%- if is_system_content %}
{{- raise_exception('System message cannot contain videos.') }}
{%- endif %}
{%- if do_vision_count %}
{%- set video_count.value = video_count.value + 1 %}
{%- endif %}
{%- if add_vision_id %}
{{- 'Video ' ~ video_count.value ~ ': ' }}
{%- endif %}
{{- '<|vision_start|><|video_pad|><|vision_end|>' }}
{%- elif 'text' in item %}
{{- item.text }}
{%- else %}
{{- raise_exception('Unexpected item type in content.') }}
{%- endif %}
{%- endfor %}
{%- elif content is none or content is undefined %}
{{- '' }}
{%- else %}
{{- raise_exception('Unexpected content type.') }}
{%- endif %}
{%- endmacro %}
{%- if not messages %}
{{- raise_exception('No messages provided.') }}
{%- endif %}
{%- set reasoning_instructions = '' %}
{%- if enable_thinking is undefined or enable_thinking is true %}
{%- set resolved_reasoning_effort = reasoning_effort|default('xhigh') %}
{%- if resolved_reasoning_effort not in ('xhigh', 'medium', 'low') %}
{{- raise_exception('Unexpected reasoning effort ' ~ reasoning_effort ~ '. Supported types are xhigh (default), medium, and low.') }}
{%- endif %}
{%- if resolved_reasoning_effort == 'xhigh' %}
{%- set reasoning_instructions = 'Reasoning effort is set to xhigh. Please think carefully through the task, validate key assumptions, consider plausible alternatives, and prioritize correctness, consistency, and clarity in the final answer.' %}
{%- elif resolved_reasoning_effort == 'low' %}
{%- set reasoning_instructions = 'Reasoning effort is set to low. Keep your thinking brief and focused, moving directly to the conclusion without unnecessary elaboration.' %}
{%- endif %}
{%- endif %}
{%- if tools and tools is iterable and tools is not mapping %}
{{- '<|im_start|>system\n' }}
{%- if reasoning_instructions %}
{{- reasoning_instructions + '\n\n' }}
{%- endif %}
{{- "# Tools\n\nYou have access to the following functions:\n\n<tools>" }}
{%- for tool in tools %}
{{- "\n" }}
{{- tool | tojson }}
{%- endfor %}
{{- "\n</tools>" }}
{{- '\n\nIf you choose to call a function ONLY reply in the following format with NO suffix:\n\n<tool_call>\n<function=example_function_name>\n<parameter=example_parameter_1>\nvalue_1\n</parameter>\n<parameter=example_parameter_2>\nThis is the value for the second parameter\nthat can span\nmultiple lines\n</parameter>\n</function>\n</tool_call>\n\n<IMPORTANT>\nReminder:\n- Function calls MUST follow the specified format: an inner <function=...></function> block must be nested within <tool_call></tool_call> XML tags\n- Required parameters MUST be specified\n- You may provide optional reasoning for your function call in natural language BEFORE the function call, but NOT after\n- If there is no function call available, answer the question like normal with your current knowledge and do not tell the user about function calls\n</IMPORTANT>' }}
{%- if messages[0].role == 'system' %}
{%- set content = render_content(messages[0].content, false, true)|trim %}
{%- if content %}
{{- '\n\n' + content }}
{%- endif %}
{%- endif %}
{{- '<|im_end|>\n' }}
{%- else %}
{%- if messages[0].role == 'system' %}
{%- set content = render_content(messages[0].content, false, true)|trim %}
{%- if content %}
{{- '<|im_start|>system\n' + (reasoning_instructions + '\n\n' if reasoning_instructions else '') + content + '<|im_end|>\n' }}
{%- elif reasoning_instructions %}
{{- '<|im_start|>system\n' + reasoning_instructions + '<|im_end|>\n' }}
{%- endif %}
{%- elif reasoning_instructions %}
{{- '<|im_start|>system\n' + reasoning_instructions + '<|im_end|>\n' }}
{%- endif %}
{%- endif %}
{%- set ns = namespace(multi_step_tool=true, last_query_index=messages|length - 1) %}
{%- for message in messages[::-1] %}
{%- set index = (messages|length - 1) - loop.index0 %}
{%- if ns.multi_step_tool and message.role == "user" %}
{%- set content = render_content(message.content, false)|trim %}
{%- if not(content.startswith('<tool_response>') and content.endswith('</tool_response>')) %}
{%- set ns.multi_step_tool = false %}
{%- set ns.last_query_index = index %}
{%- endif %}
{%- endif %}
{%- endfor %}
{%- if ns.multi_step_tool %}
{{- raise_exception('No user query found in messages.') }}
{%- endif %}
{%- for message in messages %}
{%- set content = render_content(message.content, true)|trim %}
{%- if message.role == "system" %}
{%- if not loop.first %}
{{- raise_exception('System message must be at the beginning.') }}
{%- endif %}
{%- elif message.role == "user" %}
{{- '<|im_start|>' + message.role + '\n' + content + '<|im_end|>' + '\n' }}
{%- elif message.role == "assistant" %}
{%- set reasoning_content = '' %}
{%- if message.reasoning_content is string %}
{%- set reasoning_content = message.reasoning_content %}
{%- endif %}
{%- set reasoning_content = reasoning_content|trim %}
{%- if preserve_thinking is undefined or preserve_thinking is true or loop.index0 > ns.last_query_index %}
{{- '<|im_start|>' + message.role + '\n<think>\n' + reasoning_content + '\n</think>\n\n' + content }}
{%- else %}
{{- '<|im_start|>' + message.role + '\n' + content }}
{%- endif %}
{%- if message.tool_calls and message.tool_calls is iterable and message.tool_calls is not mapping %}
{%- for tool_call in message.tool_calls %}
{%- if tool_call.function is defined %}
{%- set tool_call = tool_call.function %}
{%- endif %}
{%- if loop.first %}
{%- if content|trim %}
{{- '\n\n<tool_call>\n<function=' + tool_call.name + '>\n' }}
{%- else %}
{{- '<tool_call>\n<function=' + tool_call.name + '>\n' }}
{%- endif %}
{%- else %}
{{- '\n<tool_call>\n<function=' + tool_call.name + '>\n' }}
{%- endif %}
{%- if tool_call.arguments is defined and tool_call.arguments != '' %}
{%- for args_name, args_value in tool_call.arguments|items %}
{{- '<parameter=' + args_name + '>\n' }}
{%- set args_value = args_value | string if args_value is string else args_value | tojson | safe %}
{{- args_value }}
{{- '\n</parameter>\n' }}
{%- endfor %}
{%- endif %}
{{- '</function>\n</tool_call>' }}
{%- endfor %}
{%- endif %}
{{- '<|im_end|>\n' }}
{%- elif message.role == "tool" %}
{%- if loop.previtem and loop.previtem.role != "tool" %}
{{- '<|im_start|>user' }}
{%- endif %}
{{- '\n<tool_response>\n' }}
{{- content }}
{{- '\n</tool_response>' }}
{%- if not loop.last and loop.nextitem.role != "tool" %}
{{- '<|im_end|>\n' }}
{%- elif loop.last %}
{{- '<|im_end|>\n' }}
{%- endif %}
{%- else %}
{{- raise_exception('Unexpected message role.') }}
{%- endif %}
{%- endfor %}
{%- if add_generation_prompt %}
{{- '<|im_start|>assistant\n' }}
{%- if enable_thinking is defined and enable_thinking is false %}
{{- '<think>\n\n</think>\n\n' }}
{%- else %}
{{- '<think>\n' }}
{%- endif %}
{%- endif %}
106 changes: 106 additions & 0 deletions tests/DotLLM.Tests.Unit/Tokenizers/ChatTemplates/JinjaParserTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,4 +321,110 @@ public void FilterWithArgs()
Assert.Equal("default", filter.FilterName);
Assert.Single(filter.Args);
}

// ── Tuple literals (#409) ──
//
// Jinja2/Python semantics: parentheses containing a comma-separated expression list
// form a tuple; a lone comma after a single item ("(a,)") is significant and still makes
// a tuple, while a comma-free "(a)" is plain grouping and must NOT become a tuple.

[Fact]
public void ParenGrouping_SingleExpression_NoTrailingComma_IsNotATuple()
{
// "(a)" must parse identically to "a" — no ListExpr wrapper. This is the case a
// naive "any parens with a comma-splittable body" implementation gets wrong.
var grouped = Parse("{{ (x) }}");
var groupedOutput = Assert.IsType<ExpressionOutputNode>(grouped.Nodes[0]);
Assert.IsType<IdentifierExpr>(groupedOutput.Expression);

var plain = Parse("{{ x }}");
var plainOutput = Assert.IsType<ExpressionOutputNode>(plain.Nodes[0]);
Assert.Equal(
((IdentifierExpr)plainOutput.Expression).Name,
((IdentifierExpr)groupedOutput.Expression).Name);
}

[Fact]
public void ParenGrouping_ArithmeticPrecedence_StillWorks()
{
// "(a)" grouping must still compose normally with surrounding operators.
var ast = Parse("{{ (1 + 2) * 3 }}");
var output = Assert.IsType<ExpressionOutputNode>(ast.Nodes[0]);
var binary = Assert.IsType<BinaryExpr>(output.Expression);
Assert.Equal(BinaryOp.Multiply, binary.Op);
Assert.IsType<BinaryExpr>(binary.Left); // the "(1 + 2)" grouping, unwrapped
}

[Fact]
public void TupleLiteral_OneElementWithTrailingComma_IsAOneElementTuple()
{
// "(a,)" — the trailing comma is significant: distinct from plain "(a)" grouping.
var ast = Parse("{{ (x,) }}");
var output = Assert.IsType<ExpressionOutputNode>(ast.Nodes[0]);
var tuple = Assert.IsType<ListExpr>(output.Expression);
Assert.Single(tuple.Items);
Assert.IsType<IdentifierExpr>(tuple.Items[0]);
}

[Fact]
public void TupleLiteral_MultipleElements()
{
var ast = Parse("{{ ('xhigh', 'medium', 'low') }}");
var output = Assert.IsType<ExpressionOutputNode>(ast.Nodes[0]);
var tuple = Assert.IsType<ListExpr>(output.Expression);
Assert.Equal(3, tuple.Items.Count);
Assert.Equal("xhigh", ((LiteralExpr)tuple.Items[0]).Value);
Assert.Equal("medium", ((LiteralExpr)tuple.Items[1]).Value);
Assert.Equal("low", ((LiteralExpr)tuple.Items[2]).Value);
}

[Fact]
public void TupleLiteral_TrailingCommaAfterMultipleElements_IsLegal()
{
var ast = Parse("{{ (1, 2, 3,) }}");
var output = Assert.IsType<ExpressionOutputNode>(ast.Nodes[0]);
var tuple = Assert.IsType<ListExpr>(output.Expression);
Assert.Equal(3, tuple.Items.Count);
}

[Fact]
public void TupleLiteral_Empty()
{
var ast = Parse("{{ () }}");
var output = Assert.IsType<ExpressionOutputNode>(ast.Nodes[0]);
var tuple = Assert.IsType<ListExpr>(output.Expression);
Assert.Empty(tuple.Items);
}

[Fact]
public void TupleLiteral_NotIn_TrueBranch()
{
// The literal construct from Qwen3.8-27B's chat_template.jinja line 48.
var ast = Parse(
"{%- if resolved_reasoning_effort not in ('xhigh', 'medium', 'low') -%}yes{%- endif -%}");
var ifNode = Assert.IsType<IfNode>(ast.Nodes[0]);
var condition = ifNode.Branches[0].Condition;
var unary = Assert.IsType<UnaryExpr>(condition);
Assert.Equal(UnaryOp.Not, unary.Op);
var binary = Assert.IsType<BinaryExpr>(unary.Operand);
Assert.Equal(BinaryOp.In, binary.Op);
var tuple = Assert.IsType<ListExpr>(binary.Right);
Assert.Equal(3, tuple.Items.Count);
}

[Fact]
public void TupleLiteral_In_EvaluatesTrueWhenMember()
{
var ast = Parse("{%- if effort in ('xhigh', 'medium', 'low') -%}matched{%- else -%}unmatched{%- endif -%}");
var evaluator = new JinjaEvaluator(new Dictionary<string, object?> { ["effort"] = "medium" });
Assert.Equal("matched", evaluator.Evaluate(ast));
}

[Fact]
public void TupleLiteral_NotIn_EvaluatesTrueWhenNotMember()
{
var ast = Parse("{%- if effort not in ('xhigh', 'medium', 'low') -%}matched{%- else -%}unmatched{%- endif -%}");
var evaluator = new JinjaEvaluator(new Dictionary<string, object?> { ["effort"] = "auto" });
Assert.Equal("matched", evaluator.Evaluate(ast));
}
}
Loading
Loading