Skip to content

Enforce explicit chat-template kwarg allowlists - #126

Merged
hallerite merged 1 commit into
mainfrom
codex/template-kwarg-contract
Aug 12, 2026
Merged

Enforce explicit chat-template kwarg allowlists#126
hallerite merged 1 commit into
mainfrom
codex/template-kwarg-contract

Conversation

@hallerite

@hallerite hallerite commented Aug 11, 2026

Copy link
Copy Markdown
Member

Summary

Introduces an explicit, positive allowlist for the chat-template kwargs accepted by every model-specific renderer.

This PR is stacked on #103. Merge #123, then #103, then this PR.

Changes

  • Every renderer config declares its chat-template fields explicitly.
  • Class creation fails if a renderer-specific field is unclassified, classified twice, or references a nonexistent field.
  • chat_template_kwargs rejects unknown, shared, and renderer-internal fields with an error that lists the allowed names.
  • Renderer-internal options remain valid through the typed config.
  • DefaultRenderer remains open to opaque Jinja kwargs while reserving its typed config fields.

Impact

This closes a validation hole where fields such as image_cache_max could be supplied through chat_template_kwargs even though they are not Jinja template inputs.

Verification

  • 644 passed, 4 skipped in the config and chat-template kwarg parity suites
  • Ruff lint and formatting pass
  • Ty completes with the existing repository warning baseline

Note

Enforce explicit chat-template kwarg allowlists and add InklingRenderer with multimodal support

  • Adds _template_fields and _allow_opaque_template_kwargs to BaseRendererConfig, requiring every renderer config subclass to explicitly classify its fields; unclassified fields raise TypeError at import time.
  • Updates _merge_chat_template_kwargs in renderers/base.py to validate chat_template_kwargs against each config's declared allowlist, raising ValueError for unknown or renderer-internal keys.
  • Introduces InklingRenderer and InklingRendererConfig in renderers/inkling.py supporting image and audio multimodal content, tool declaration/invocation, reasoning_effort control, thinking retention, and bridge_to_next_turn.
  • Adds parse_inkling in renderers/parsing.py to decode Inkling model outputs into structured ParsedResponse objects including reasoning content and tool calls.
  • Risk: passing previously-accepted but undeclared chat_template_kwargs to typed renderers now raises ValueError instead of silently merging.

Macroscope summarized 8d5965a.


Note

Medium Risk
Changes how invalid chat_template_kwargs fail (often ValueError at merge time instead of pydantic ValidationError), which may break callers that relied on passing internal fields through kwargs; behavior for valid configs is stricter and more explicit.

Overview
Enforces explicit allowlists for chat_template_kwargs so renderer-internal options (e.g. image_cache_max, tool_parser) can’t be smuggled in as if they were Jinja template knobs.

Each renderer config now declares _template_fields and _internal_fields; template_field_names() returns only the template set. __pydantic_init_subclass__ fails at class definition time if any field is missing, duplicated, or unknown in that split.

_merge_chat_template_kwargs checks incoming keys against that allowlist and raises ValueError with allowed names; renderer-only fields must go on the typed config. DefaultRendererConfig keeps _allow_opaque_template_kwargs for arbitrary Jinja extras but still blocks typed fields like tool_parser from the kwargs map.

README and docs/renderer-config.md describe the new contract; tests cover allowlist validation, internal-field rejection, and default-renderer open-ended vs reserved behavior.

Reviewed by Cursor Bugbot for commit 635d351. Bugbot is set up for automated code reviews on this repo. Configure here.

@hallerite
hallerite marked this pull request as ready for review August 11, 2026 22:23
@macroscopeapp

macroscopeapp Bot commented Aug 11, 2026

Copy link
Copy Markdown

Approvability

Verdict: Approved 82bff80

This PR adds explicit kwarg validation and field classification to renderer configs - a guardrails/type-safety improvement that makes the system more restrictive rather than enabling new behavior. Changes are self-contained, well-tested, and the author has prior contributions to these files.

No code changes detected at 635d351. Prior analysis still applies.

You can customize Macroscope's approvability policy. Learn more.

@hallerite
hallerite force-pushed the codex/template-kwarg-contract branch from 82bff80 to fd260d5 Compare August 12, 2026 18:10
stack merge was automatically disabled August 12, 2026 18:34

Pull Request is not mergeable

stack merge was automatically disabled August 12, 2026 18:45

Pull Request is not mergeable

@hallerite
hallerite force-pushed the codex/template-kwarg-contract branch from fd260d5 to 8d5965a Compare August 12, 2026 18:46
Base automatically changed from feat/inkling-renderer to main August 12, 2026 18:46
@hallerite
hallerite force-pushed the codex/template-kwarg-contract branch from 8d5965a to 635d351 Compare August 12, 2026 18:46
@hallerite
hallerite merged commit c35f506 into main Aug 12, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant