Skip to content

chat_template_kwargs: null array elements dropped, reserved keys overwritten, top-level enable_thinking does not override a non-boolean kwarg #264

Open

Description

@solderzzc

Three low-severity follow-ups from the review of #260 (merged as 7376b6b). None is a regression, and none blocks #259.

  1. Null elements in arrays are dropped. sanitizeForJinja uses compactMap on arrays, so chat_template_kwargs: {"ids": [1, null, 2]} reaches the template as [1, 2]. The PR text says nulls are stripped, which is true for dict entries and also for arrays. Index-based templates would read shifted values. The same path already applies to tool schemas. Decide whether arrays should keep their positions (for example, replace null with NSNull) or whether the current behaviour is the contract, then document it.

  2. Reserved names overwrite tokenizer variables. Kwargs are merged after messages, tools and add_generation_prompt in the Tokenizers context, so a client that sends one of those names overwrites the value the tokenizer set. Only the requester's own request is affected. Reject or namespace these keys.

  3. A non-boolean chat_template_kwargs.enable_thinking gets 400 even when the top-level enable_thinking is set. The validation runs before the precedence, so {"enable_thinking": false, "chat_template_kwargs": {"enable_thinking": "yes"}} returns 400. This is deliberate and covered by test 17b. The README wording ("overridden by the top-level field") reads as if the kwarg is ignored. Either document the validation or let a top-level value skip it.

Severity: low for all three.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions