Skip to content

fix: minja upper/lower filter fails on array type values#21342

Closed
gerstnr wants to merge 1 commit intoggml-org:masterfrom
gerstnr:fix-lower-upper-filter
Closed

fix: minja upper/lower filter fails on array type values#21342
gerstnr wants to merge 1 commit intoggml-org:masterfrom
gerstnr:fix-lower-upper-filter

Conversation

@gerstnr
Copy link
Copy Markdown

@gerstnr gerstnr commented Apr 3, 2026

Overview

When using Gemma 4 models with tool calling (tested Gemma-4-31B-It, unsloth gguf), requests to /v1/chat/completions or /v1/responses fail with a 500 error:

While executing FilterExpression at line 18, column 34 in source:
...if -%}
{%- if value['type'] | upper == 'STRING' -%}
^
Error: Unknown (built-in) filter 'upper' for type Array

This happens because Gemma 4's chat template (both the GGUF-embedded template and the new gemma4.jinja template from #21326) uses value['type'] | upper to check JSON Schema parameter types. Minja's value_array_t had no upper (or lower) method, causing the exception.

Reported in a comment on #21326: #21326 (comment)

Fix

Add upper and lower to value_array_t::get_builtins() in common/jinja/value.cpp, returning an empty string — the same fallback behavior already used by value_undefined_t.

Additional information

ch10299342 proposed fixing the template itself in #21326.

Valid, would preserve type-specific formatting for nullable params. The runtime fix is simpler and more defensive.
The two approaches are not mutually exclusive — a template fix could be applied on top.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: yes, root cause analysis and search for existing PRs

@gerstnr gerstnr requested a review from CISC as a code owner April 3, 2026 00:13
@ggml-gh-bot

This comment was marked as off-topic.

@github-actions github-actions Bot added the jinja parser Issues related to the jinja parser label Apr 3, 2026
@ch10299342
Copy link
Copy Markdown

I'm going to close my PR this is better.

@CISC
Copy link
Copy Markdown
Member

CISC commented Apr 3, 2026

This is unfortunately still not quite right, I will make a PR with the proper fix.

BTW, the template does not seem to correctly handle when value['type'] is an array anyway, so it might need fixing. cc/ @ngxson @osanseviero

@CISC
Copy link
Copy Markdown
Member

CISC commented Apr 3, 2026

Proper jinja fix in #21370, at least from a compatibility viewpoint, the template still requires fixing IMO.

@gerstnr
Copy link
Copy Markdown
Author

gerstnr commented Apr 3, 2026

Thanks @CISC!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jinja parser Issues related to the jinja parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants