From 635d3516a39ca4c5b2a1dc838ee9fb267ac7253a Mon Sep 17 00:00:00 2001 From: hallerite Date: Wed, 12 Aug 2026 00:19:01 +0200 Subject: [PATCH] Enforce explicit chat-template kwarg allowlists --- README.md | 2 +- docs/renderer-config.md | 17 +++++-- renderers/base.py | 23 ++++++++- renderers/configs.py | 92 ++++++++++++++++++++++++++--------- tests/test_renderer_config.py | 51 ++++++++++++++++++- 5 files changed, 153 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 522fb6a..3609527 100644 --- a/README.md +++ b/README.md @@ -143,7 +143,7 @@ renderer = create_renderer( Discriminated union: every per-renderer config is a variant of `RendererConfig`, dispatched on the `name` field. Bogus combinations (e.g. `add_vision_id` under `name="qwen3"`) error at construction with a `pydantic.ValidationError`. Downstream pydantic configs (prime-rl orchestrator, verifiers `ClientConfig`) hold a single field typed as `RendererConfig` and inherit the same strict-per-variant validation. -When `chat_template_kwargs` is passed with `config=None` / `AutoRendererConfig`, renderers first resolves the concrete renderer from the model name, then validates those kwargs against that renderer's config. `Auto + unknown model + chat_template_kwargs` fails loudly; use an explicit typed config or explicit `DefaultRendererConfig` for opaque fallback templates. +When `chat_template_kwargs` is passed with `config=None` / `AutoRendererConfig`, renderers first resolves the concrete renderer from the model name, then validates each key against that renderer's explicit template-kwarg allowlist. Renderer-only fields such as `image_cache_max` must be passed through the typed config instead. `Auto + unknown model + chat_template_kwargs` fails loudly; use an explicit typed config or explicit `DefaultRendererConfig` for opaque fallback templates. One shared behaviour flag lives on typed renderer configs: `thinking_retention`, an optional bridge-policy override. Leave it unset to derive bridge behaviour from the chat template and its renderer-exposed kwargs. diff --git a/docs/renderer-config.md b/docs/renderer-config.md index 85c50da..6516e00 100644 --- a/docs/renderer-config.md +++ b/docs/renderer-config.md @@ -19,8 +19,10 @@ construction. ## Per-renderer configs -Use `type(config).template_field_names()` to inspect the fields that mirror -chat-template kwargs. Those fields are covered by parity tests against +Use `type(config).template_field_names()` to inspect the explicit allowlist of +fields accepted through `chat_template_kwargs`. Every renderer-specific field +is classified as either a template field or a renderer-only field at class +definition time. Template fields are covered by parity tests against `apply_chat_template` in `tests/test_renderer_config_parity.py`. | Renderer | Config class | Template fields | Renderer-only fields | @@ -81,8 +83,10 @@ pool = create_renderer_pool( ``` Renderers resolves auto configs before applying `chat_template_kwargs`, so the -kwargs validate against the concrete renderer config. Unknown kwargs, or kwargs -that conflict with an explicit `thinking_retention`, fail at construction. +kwargs validate against the concrete renderer's template-field allowlist. +Unknown kwargs, renderer-only fields, or kwargs that conflict with an explicit +`thinking_retention` fail at construction. Renderer-only options remain valid +when supplied through the typed config itself. Auto-resolution fails loudly for VLMs without an exact registered renderer. Text-only unknown models fall back to `DefaultRenderer`, unless @@ -91,7 +95,10 @@ cannot implement selective bridge retention, so that combination raises. `AutoRendererConfig` with `chat_template_kwargs` also raises for unknown models, because renderers cannot validate those kwargs without a concrete renderer. Use an explicit model-specific config, or `DefaultRendererConfig(...)` when you -intentionally want opaque `apply_chat_template` kwargs. +intentionally want opaque `apply_chat_template` kwargs. Even for the default +renderer, typed fields such as `tool_parser`, `reasoning_parser`, and +`thinking_retention` must be passed through the config rather than through the +opaque kwargs mapping. ## `thinking_retention` diff --git a/renderers/base.py b/renderers/base.py index ccb59b7..074f016 100644 --- a/renderers/base.py +++ b/renderers/base.py @@ -1491,12 +1491,31 @@ def _merge_chat_template_kwargs( return config if not isinstance(chat_template_kwargs, Mapping): raise TypeError("chat_template_kwargs must be a mapping.") + kwargs = dict(chat_template_kwargs) + config_cls = type(config) + allowed = config_cls.template_field_names() + if config_cls._allow_opaque_template_kwargs: + reserved = frozenset(config_cls.model_fields) - allowed - {"name"} + unsupported = frozenset(kwargs) & reserved + else: + unsupported = frozenset(kwargs) - allowed + if unsupported: + allowed_text = ( + "opaque Jinja kwargs" + if config_cls._allow_opaque_template_kwargs + else ", ".join(sorted(allowed)) or "(none)" + ) + raise ValueError( + f"Unsupported chat_template_kwargs for {config.name!r}: " + f"{sorted(unsupported)}. Allowed: {allowed_text}. Pass " + "renderer-internal options through the typed config instead." + ) data: dict[str, Any] = {"name": config.name} for field_name in config.__pydantic_fields_set__: data[field_name] = getattr(config, field_name) data.update(getattr(config, "model_extra", None) or {}) - data.update(dict(chat_template_kwargs)) - return type(config).model_validate(data) + data.update(kwargs) + return config_cls.model_validate(data) def _resolve_renderer_config( diff --git a/renderers/configs.py b/renderers/configs.py index ba94e6a..f792cdc 100644 --- a/renderers/configs.py +++ b/renderers/configs.py @@ -1,10 +1,11 @@ """Typed renderer configs — one pydantic model per renderer, unified by a discriminated union (``RendererConfig``). -Each renderer accepts its own typed config; bad combinations (e.g. -``add_vision_id`` under ``name="qwen3"``) fail at config-load time with a -pydantic ``ValidationError`` rather than at runtime via an allowlist -check. The shared ``thinking_retention`` flag is optional: ``None`` means +Each renderer accepts its own typed config and declares an explicit +``_template_fields`` allowlist for fields that may arrive through +``chat_template_kwargs``. Bad combinations (e.g. ``add_vision_id`` under +``name="qwen3"``) fail before renderer construction. The shared +``thinking_retention`` flag is optional: ``None`` means "derive bridge policy from this renderer's chat-template knobs"; an explicit value is a bridge-policy override. @@ -87,31 +88,36 @@ class BaseRendererConfig(BaseConfig): to the Python chat-template implementation and its explicit template kwargs.""" - # Fields that are renderer-internal — not forwarded to (or mirrored - # by) ``apply_chat_template``. Override in subclasses that hold - # non-template config (e.g. ``image_cache_max``, GptOss's - # ``use_system_prompt`` / ``knowledge_cutoff`` / ``model_identity``, - # or fields that exist as renderer conventions without a Jinja - # analogue like DeepSeek V3 / Kimi K2 ``enable_thinking``). - # - # Used by parity tests to compute the field subset that, when - # changed, must produce token streams matching - # ``apply_chat_template`` — see :meth:`template_field_names`. The - # renderer is the only end-to-end consumer of these fields, so this - # is a renderer-side bookkeeping concern rather than a public API. + # Every renderer-specific field must be classified exactly once: either + # as a chat-template kwarg or as renderer-internal configuration. + _template_fields: ClassVar[frozenset[str]] = frozenset() _internal_fields: ClassVar[frozenset[str]] = frozenset() + _allow_opaque_template_kwargs: ClassVar[bool] = False + + @classmethod + def __pydantic_init_subclass__(cls, **kwargs) -> None: + super().__pydantic_init_subclass__(**kwargs) + base_fields = frozenset(BaseRendererConfig.model_fields) + renderer_fields = frozenset(cls.model_fields) - base_fields - {"name"} + overlap = cls._template_fields & cls._internal_fields + missing = renderer_fields - cls._template_fields - cls._internal_fields + unknown = (cls._template_fields | cls._internal_fields) - renderer_fields + if overlap or missing or unknown: + raise TypeError( + f"{cls.__name__} has an invalid renderer-field classification: " + f"overlap={sorted(overlap)}, missing={sorted(missing)}, " + f"unknown={sorted(unknown)}" + ) @classmethod def template_field_names(cls) -> frozenset[str]: """Subset of fields that mirror Jinja chat-template kwargs. - Default: every non-base field except ``name`` and any field - listed in ``_internal_fields``. Used by the parity test matrix - (``tests/test_renderer_config_parity.py``) to discover the - cells that must agree with ``apply_chat_template``. + Used both as the runtime allowlist for ``chat_template_kwargs`` and + by parity tests to discover the cells that must agree with + ``apply_chat_template``. """ - base = frozenset(BaseRendererConfig.model_fields) - return frozenset(cls.model_fields) - base - {"name"} - cls._internal_fields + return cls._template_fields class AutoRendererConfig(BaseRendererConfig): @@ -122,6 +128,7 @@ class AutoRendererConfig(BaseRendererConfig): at the call site.""" name: Literal["auto"] = "auto" + _template_fields = frozenset() class DefaultRendererConfig(BaseRendererConfig): @@ -148,6 +155,8 @@ class DefaultRendererConfig(BaseRendererConfig): # DefaultRenderer's parsing pipeline, not the underlying Jinja # template. Jinja kwargs live in ``model_extra`` (extra="allow"). _internal_fields = frozenset({"tool_parser", "reasoning_parser"}) + _template_fields = frozenset() + _allow_opaque_template_kwargs = True @model_validator(mode="after") def _reject_legacy_preserve_flags(self): @@ -174,6 +183,7 @@ class Qwen3RendererConfig(BaseRendererConfig): """Qwen3 (text-only) renderer config.""" name: Literal["qwen3"] = "qwen3" + _template_fields = frozenset({"enable_thinking"}) enable_thinking: bool = True """When ``True``, the generation prompt includes ```` so the @@ -191,12 +201,14 @@ class PrimeQwen3RendererConfig(BaseRendererConfig): """PrimeIntellect Qwen3 renderer config.""" name: Literal["prime-qwen3"] = "prime-qwen3" + _template_fields = frozenset() class Qwen35RendererConfig(BaseRendererConfig): """Qwen3.5 renderer config.""" name: Literal["qwen3.5"] = "qwen3.5" + _template_fields = frozenset({"enable_thinking", "add_vision_id"}) enable_thinking: bool | None = None """When ``True``, the generation prompt includes ````. ``None`` @@ -228,6 +240,9 @@ class Qwen36RendererConfig(BaseRendererConfig): """Qwen3.6 renderer config. Inherits Qwen3.5's template surface.""" name: Literal["qwen3.6"] = "qwen3.6" + _template_fields = frozenset( + {"enable_thinking", "add_vision_id", "preserve_thinking"} + ) enable_thinking: bool | None = None """See :class:`Qwen35RendererConfig.enable_thinking`.""" @@ -260,6 +275,7 @@ class Qwen3VLRendererConfig(BaseRendererConfig): """Qwen3-VL renderer config.""" name: Literal["qwen3-vl"] = "qwen3-vl" + _template_fields = frozenset({"add_vision_id"}) add_vision_id: bool = False """See :class:`Qwen35RendererConfig.add_vision_id`.""" @@ -274,6 +290,7 @@ class Gemma4RendererConfig(BaseRendererConfig): """Gemma 4 renderer config.""" name: Literal["gemma4"] = "gemma4" + _template_fields = frozenset({"enable_thinking", "preserve_thinking"}) enable_thinking: bool = False """Enable Gemma 4's thinking mode. Mirrors the canonical template kwarg.""" @@ -301,6 +318,7 @@ class GLM5RendererConfig(BaseRendererConfig): """GLM-5 renderer config.""" name: Literal["glm-5"] = "glm-5" + _template_fields = frozenset({"enable_thinking", "clear_thinking"}) enable_thinking: bool = True """When ``True``, the generation prompt includes ````. Mirrors @@ -328,6 +346,7 @@ class GLM51RendererConfig(BaseRendererConfig): discriminator so the registry can route to ``GLM51Renderer``.""" name: Literal["glm-5.1"] = "glm-5.1" + _template_fields = frozenset({"enable_thinking", "clear_thinking"}) enable_thinking: bool = True """See :class:`GLM5RendererConfig.enable_thinking`.""" @@ -350,6 +369,7 @@ class GLM45RendererConfig(BaseRendererConfig): """GLM-4.5 Air renderer config.""" name: Literal["glm-4.5"] = "glm-4.5" + _template_fields = frozenset({"enable_thinking"}) enable_thinking: bool = True """When ``True``, the generation prompt includes ````. Mirrors @@ -377,6 +397,15 @@ class Hy3RendererConfig(BaseRendererConfig): """ name: Literal["hy3"] = "hy3" + _template_fields = frozenset( + { + "reasoning_effort", + "preserved_thinking", + "is_training", + "raw_last_assistant", + "fallback_strategy", + } + ) reasoning_effort: Literal["no_think", "low", "high"] = "no_think" """Reasoning gate. Mirrors the chat template's ``reasoning_effort`` kwarg. @@ -449,6 +478,7 @@ class InklingRendererConfig(BaseRendererConfig): """ name: Literal["inkling"] = "inkling" + _template_fields = frozenset({"reasoning_effort"}) reasoning_effort: str | float = 0.9 """Reasoning-effort gate. Mirrors the chat template's ``reasoning_effort`` @@ -495,6 +525,7 @@ class GptOssRendererConfig(BaseRendererConfig): """ name: Literal["gpt-oss"] = "gpt-oss" + _template_fields = frozenset({"reasoning_effort", "conversation_start_date"}) reasoning_effort: Literal["low", "medium", "high"] = "medium" """Harmony reasoning-effort tag. Mirrors the ``apply_chat_template`` @@ -551,6 +582,7 @@ class KimiK2RendererConfig(BaseRendererConfig): """ name: Literal["kimi-k2"] = "kimi-k2" + _template_fields = frozenset() enable_thinking: bool = True """No-op for Kimi K2 (template doesn't gate on it). Stored for @@ -563,6 +595,7 @@ class KimiK25RendererConfig(BaseRendererConfig): """Kimi K2.5 renderer config.""" name: Literal["kimi-k2.5"] = "kimi-k2.5" + _template_fields = frozenset({"thinking"}) thinking: bool = True """When ``True``, the generation prompt prefills ````; when @@ -580,6 +613,7 @@ class LagunaXS2RendererConfig(BaseRendererConfig): """Laguna XS.2 renderer config.""" name: Literal["laguna-xs.2"] = "laguna-xs.2" + _template_fields = frozenset({"enable_thinking", "render_assistant_messages_raw"}) enable_thinking: bool = False """When ``True``, the generation prompt includes ````. Mirrors @@ -605,6 +639,7 @@ class LagunaM1RendererConfig(BaseRendererConfig): """ name: Literal["laguna-m.1"] = "laguna-m.1" + _template_fields = frozenset({"enable_thinking", "render_assistant_messages_raw"}) enable_thinking: bool = False """When ``True``, the generation prompt includes ````. Mirrors @@ -626,6 +661,7 @@ class LagunaXS21RendererConfig(BaseRendererConfig): """ name: Literal["laguna-xs-2.1"] = "laguna-xs-2.1" + _template_fields = frozenset({"enable_thinking"}) enable_thinking: bool = False """When ``True``, the generation prompt ends with ```` and @@ -649,6 +685,7 @@ class LagunaS21RendererConfig(BaseRendererConfig): """ name: Literal["laguna-s-2.1"] = "laguna-s-2.1" + _template_fields = frozenset({"enable_thinking", "preserve_thinking"}) enable_thinking: bool = True """When ``True``, the generation prompt ends with ```` and every @@ -678,6 +715,7 @@ class Llama3RendererConfig(BaseRendererConfig): """ name: Literal["llama-3"] = "llama-3" + _template_fields = frozenset({"date_string", "tools_in_user_message"}) date_string: str = "26 Jul 2024" """``Today Date`` value injected into the system preamble. Pinned to @@ -696,6 +734,7 @@ class MiniMaxM2RendererConfig(BaseRendererConfig): """MiniMax M2 / M2.5 renderer config.""" name: Literal["minimax-m2"] = "minimax-m2" + _template_fields = frozenset({"model_identity"}) model_identity: str = "You are a helpful assistant. Your name is MiniMax-M2.5 and is built by MiniMax." """Fallback persona used when no system message is supplied. Mirrors @@ -713,6 +752,9 @@ class Nemotron3RendererConfig(BaseRendererConfig): """ name: Literal["nemotron-3"] = "nemotron-3" + _template_fields = frozenset( + {"enable_thinking", "truncate_history_thinking", "low_effort"} + ) enable_thinking: bool = True """When ``True``, the generation prompt includes ````. Mirrors @@ -755,6 +797,9 @@ class Nemotron3UltraRendererConfig(BaseRendererConfig): """ name: Literal["nemotron-3-ultra"] = "nemotron-3-ultra" + _template_fields = frozenset( + {"enable_thinking", "truncate_history_thinking", "medium_effort"} + ) enable_thinking: bool = True """See :class:`Nemotron3RendererConfig.enable_thinking`.""" @@ -790,6 +835,7 @@ class Nemotron35RendererConfig(BaseRendererConfig): """ name: Literal["nemotron-3.5"] = "nemotron-3.5" + _template_fields = frozenset({"enable_thinking", "truncate_history_thinking"}) enable_thinking: bool = True """See :class:`Nemotron3RendererConfig.enable_thinking`.""" @@ -817,6 +863,7 @@ class DeepSeekV3RendererConfig(BaseRendererConfig): """ name: Literal["deepseek-v3"] = "deepseek-v3" + _template_fields = frozenset() class DeepSeekR1RendererConfig(BaseRendererConfig): @@ -834,6 +881,7 @@ class DeepSeekR1RendererConfig(BaseRendererConfig): """ name: Literal["deepseek-r1"] = "deepseek-r1" + _template_fields = frozenset() RendererConfig = Annotated[ diff --git a/tests/test_renderer_config.py b/tests/test_renderer_config.py index a35f270..b4b0433 100644 --- a/tests/test_renderer_config.py +++ b/tests/test_renderer_config.py @@ -3,12 +3,14 @@ configs.""" from types import SimpleNamespace +from typing import Literal import pytest from pydantic import TypeAdapter, ValidationError from renderers import ( AutoRendererConfig, + BaseRendererConfig, DefaultRendererConfig, GLM5RendererConfig, GptOssRendererConfig, @@ -23,6 +25,14 @@ ) +def test_renderer_specific_fields_require_explicit_classification(): + with pytest.raises(TypeError, match="missing=\\['unclassified'\\]"): + + class _InvalidRendererConfig(BaseRendererConfig): + name: Literal["invalid-test"] = "invalid-test" + unclassified: bool = False + + def test_per_renderer_config_rejects_unknown_fields(): """``extra="forbid"`` on every typed variant catches bogus keys at construction: ``add_vision_id`` doesn't exist on ``Qwen3RendererConfig`` @@ -163,7 +173,7 @@ def test_auto_unknown_model_rejects_chat_template_kwargs(): create_renderer(tok, chat_template_kwargs={"enable_thinking": False}) -def test_chat_template_kwargs_validate_against_resolved_config(monkeypatch): +def test_chat_template_kwargs_validate_against_explicit_allowlist(monkeypatch): class _FakeQwen3: def __init__(self, tokenizer, config): self.config = config @@ -171,13 +181,50 @@ def __init__(self, tokenizer, config): monkeypatch.setitem(base.RENDERER_REGISTRY, "qwen3", _FakeQwen3) monkeypatch.setitem(base.MODEL_RENDERER_MAP, "fake/qwen3", "qwen3") - with pytest.raises(ValidationError, match="enable_thinkng"): + with pytest.raises(ValueError, match="Allowed: enable_thinking"): create_renderer( SimpleNamespace(name_or_path="fake/qwen3"), chat_template_kwargs={"enable_thinkng": False}, ) +def test_chat_template_kwargs_reject_renderer_internal_fields(monkeypatch): + class _FakeQwen35: + def __init__(self, tokenizer, config): + self.config = config + + monkeypatch.setitem(base.RENDERER_REGISTRY, "qwen3.5", _FakeQwen35) + + with pytest.raises(ValueError, match="image_cache_max"): + create_renderer( + SimpleNamespace(name_or_path="fake/qwen35"), + Qwen35RendererConfig(), + chat_template_kwargs={"image_cache_max": 1}, + ) + + renderer = create_renderer( + SimpleNamespace(name_or_path="fake/qwen35"), + Qwen35RendererConfig(image_cache_max=1), + ) + assert renderer.config.image_cache_max == 1 + + +def test_default_renderer_chat_template_kwargs_remain_open_ended(): + renderer = create_renderer( + SimpleNamespace(name_or_path="unknown/text-model"), + DefaultRendererConfig(), + chat_template_kwargs={"custom_jinja_kwarg": True}, + ) + assert renderer.config.model_extra == {"custom_jinja_kwarg": True} + + with pytest.raises(ValueError, match="tool_parser"): + create_renderer( + SimpleNamespace(name_or_path="unknown/text-model"), + DefaultRendererConfig(), + chat_template_kwargs={"tool_parser": "qwen3"}, + ) + + def test_chat_template_kwargs_conflict_with_explicit_config(): with pytest.raises(ValidationError, match="thinking_retention"): create_renderer(