Skip to content

Commit fe67f9f

Browse files
authored
feat: bind preserve_*_thinking defaults at create_renderer construction (#4)
1 parent d222cf3 commit fe67f9f

4 files changed

Lines changed: 185 additions & 14 deletions

File tree

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
44

55
[project]
66
name = "renderers"
7-
version = "0.1.7"
7+
version = "0.1.8"
88
description = "Chat template renderers — deterministic message-to-token conversion for LLM training"
99
readme = "README.md"
1010
requires-python = ">=3.10,<3.14"

renderers/base.py

Lines changed: 110 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,8 @@ def create_renderer_pool(
343343
*,
344344
tool_parser: str | None = None,
345345
reasoning_parser: str | None = None,
346+
preserve_all_thinking: bool = False,
347+
preserve_thinking_between_tool_calls: bool = False,
346348
) -> RendererPool:
347349
"""Create a RendererPool with *size* independent tokenizer copies.
348350
@@ -352,6 +354,11 @@ def create_renderer_pool(
352354
353355
``tool_parser`` and ``reasoning_parser`` are forwarded to
354356
``create_renderer`` when the pool falls back to ``DefaultRenderer``.
357+
358+
``preserve_all_thinking`` and ``preserve_thinking_between_tool_calls``
359+
are forwarded to ``create_renderer`` and bound as defaults on each
360+
pooled instance — every ``render`` / ``render_ids`` call uses them
361+
unless an explicit kwarg overrides at the call site.
355362
"""
356363

357364
def factory() -> Renderer:
@@ -365,6 +372,8 @@ def factory() -> Renderer:
365372
renderer=renderer,
366373
tool_parser=tool_parser,
367374
reasoning_parser=reasoning_parser,
375+
preserve_all_thinking=preserve_all_thinking,
376+
preserve_thinking_between_tool_calls=preserve_thinking_between_tool_calls,
368377
)
369378

370379
return RendererPool(factory, size=size)
@@ -376,6 +385,8 @@ def create_renderer(
376385
*,
377386
tool_parser: str | None = None,
378387
reasoning_parser: str | None = None,
388+
preserve_all_thinking: bool = False,
389+
preserve_thinking_between_tool_calls: bool = False,
379390
) -> Renderer:
380391
"""Create a Renderer by name, or auto-detect from the tokenizer's model name.
381392
@@ -389,6 +400,15 @@ def create_renderer(
389400
have their own parsing wired in.
390401
reasoning_parser: Name of a reasoning parser registered in
391402
``renderers.parsers``. Only consumed by DefaultRenderer.
403+
preserve_all_thinking: Bind as default for every ``render`` /
404+
``render_ids`` call on the returned instance. Same
405+
semantics as the call-site kwarg of the same name —
406+
see ``Renderer.render`` and
407+
``should_preserve_past_thinking``. Off by default
408+
(zero behaviour change).
409+
preserve_thinking_between_tool_calls: Bind as default for every
410+
``render`` / ``render_ids`` call on the returned
411+
instance. See ``Renderer.render`` for semantics.
392412
"""
393413
_populate_registry()
394414

@@ -405,15 +425,21 @@ def create_renderer(
405425
f"Unknown renderer {renderer!r}. Available: {', '.join(sorted(RENDERER_REGISTRY))}"
406426
)
407427
if renderer == "default":
408-
return cls(tokenizer, **default_kwargs)
409-
if default_kwargs:
410-
logger.info(
411-
"tool_parser / reasoning_parser are only consumed by "
412-
"DefaultRenderer; ignoring for renderer=%r which has "
413-
"built-in behavior.",
414-
renderer,
415-
)
416-
return cls(tokenizer)
428+
instance = cls(tokenizer, **default_kwargs)
429+
else:
430+
if default_kwargs:
431+
logger.info(
432+
"tool_parser / reasoning_parser are only consumed by "
433+
"DefaultRenderer; ignoring for renderer=%r which has "
434+
"built-in behavior.",
435+
renderer,
436+
)
437+
instance = cls(tokenizer)
438+
return _bind_preserve_defaults(
439+
instance,
440+
preserve_all_thinking=preserve_all_thinking,
441+
preserve_thinking_between_tool_calls=preserve_thinking_between_tool_calls,
442+
)
417443

418444
# Auto-detect from model name via exact match on the canonical HF id.
419445
# Fine-tunes and renamed checkpoints miss on purpose — their chat
@@ -423,7 +449,11 @@ def create_renderer(
423449
model_name = getattr(tokenizer, "name_or_path", "")
424450
renderer_name = MODEL_RENDERER_MAP.get(model_name)
425451
if renderer_name is not None:
426-
return RENDERER_REGISTRY[renderer_name](tokenizer)
452+
return _bind_preserve_defaults(
453+
RENDERER_REGISTRY[renderer_name](tokenizer),
454+
preserve_all_thinking=preserve_all_thinking,
455+
preserve_thinking_between_tool_calls=preserve_thinking_between_tool_calls,
456+
)
427457

428458
# No match — fall back to default (apply_chat_template). For fine-tunes
429459
# with customized chat templates this is the *correct* choice, so we don't
@@ -434,7 +464,76 @@ def create_renderer(
434464
"reasoning_parser=<name> to enable structured output parsing.",
435465
model_name or "<unnamed tokenizer>",
436466
)
437-
return RENDERER_REGISTRY["default"](tokenizer, **default_kwargs)
467+
return _bind_preserve_defaults(
468+
RENDERER_REGISTRY["default"](tokenizer, **default_kwargs),
469+
preserve_all_thinking=preserve_all_thinking,
470+
preserve_thinking_between_tool_calls=preserve_thinking_between_tool_calls,
471+
)
472+
473+
474+
def _bind_preserve_defaults(
475+
renderer: Renderer,
476+
*,
477+
preserve_all_thinking: bool,
478+
preserve_thinking_between_tool_calls: bool,
479+
) -> Renderer:
480+
"""Bind ``preserve_*_thinking`` defaults onto a renderer instance.
481+
482+
When neither flag is set, returns ``renderer`` unchanged — zero-cost
483+
no-op for the common path. Otherwise wraps ``render`` and
484+
``render_ids`` so every call defaults the kwargs to the bound values
485+
while still letting an explicit call-site kwarg of ``True`` override
486+
upward (the flags only ever add thinking, never strip it).
487+
488+
The two flags also become introspectable on the instance as
489+
``_preserve_all_thinking`` / ``_preserve_thinking_between_tool_calls``
490+
so tests and downstream code can confirm what was bound.
491+
"""
492+
renderer._preserve_all_thinking = preserve_all_thinking # type: ignore[attr-defined]
493+
renderer._preserve_thinking_between_tool_calls = ( # type: ignore[attr-defined]
494+
preserve_thinking_between_tool_calls
495+
)
496+
if not (preserve_all_thinking or preserve_thinking_between_tool_calls):
497+
return renderer
498+
499+
orig_render = renderer.render
500+
orig_render_ids = renderer.render_ids
501+
502+
def render(
503+
messages: list[Message],
504+
*,
505+
tools: list[ToolSpec] | None = None,
506+
add_generation_prompt: bool = False,
507+
preserve_all_thinking: bool = preserve_all_thinking,
508+
preserve_thinking_between_tool_calls: bool = preserve_thinking_between_tool_calls,
509+
) -> RenderedTokens:
510+
return orig_render(
511+
messages,
512+
tools=tools,
513+
add_generation_prompt=add_generation_prompt,
514+
preserve_all_thinking=preserve_all_thinking,
515+
preserve_thinking_between_tool_calls=preserve_thinking_between_tool_calls,
516+
)
517+
518+
def render_ids(
519+
messages: list[Message],
520+
*,
521+
tools: list[ToolSpec] | None = None,
522+
add_generation_prompt: bool = False,
523+
preserve_all_thinking: bool = preserve_all_thinking,
524+
preserve_thinking_between_tool_calls: bool = preserve_thinking_between_tool_calls,
525+
) -> list[int]:
526+
return orig_render_ids(
527+
messages,
528+
tools=tools,
529+
add_generation_prompt=add_generation_prompt,
530+
preserve_all_thinking=preserve_all_thinking,
531+
preserve_thinking_between_tool_calls=preserve_thinking_between_tool_calls,
532+
)
533+
534+
renderer.render = render # type: ignore[method-assign]
535+
renderer.render_ids = render_ids # type: ignore[method-assign]
536+
return renderer
438537

439538

440539
# ---------------------------------------------------------------------------

tests/test_preserve_thinking.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,3 +362,75 @@ def test_default_renderer_raises_on_flags():
362362
renderer.render_ids(CONVERSATION, preserve_all_thinking=True)
363363
with pytest.raises(NotImplementedError):
364364
renderer.render_ids(CONVERSATION, preserve_thinking_between_tool_calls=True)
365+
366+
367+
# ---------------------------------------------------------------------------
368+
# Construction-time defaults via create_renderer(...) kwargs
369+
# ---------------------------------------------------------------------------
370+
371+
372+
def test_create_renderer_binds_preserve_all_thinking_default(
373+
model_name, renderer_name, tokenizer
374+
):
375+
"""``create_renderer(..., preserve_all_thinking=True)`` should make a
376+
bare ``render_ids`` call behave as if the flag were passed explicitly."""
377+
from renderers import create_renderer
378+
from renderers.default import DefaultRenderer
379+
380+
bound = create_renderer(
381+
tokenizer,
382+
renderer=renderer_name,
383+
preserve_all_thinking=True,
384+
)
385+
if isinstance(bound, DefaultRenderer):
386+
with pytest.raises(NotImplementedError):
387+
bound.render_ids(CONVERSATION)
388+
return
389+
390+
bound_ids = bound.render_ids(CONVERSATION)
391+
unbound = create_renderer(tokenizer, renderer=renderer_name)
392+
explicit_ids = unbound.render_ids(CONVERSATION, preserve_all_thinking=True)
393+
assert bound_ids == explicit_ids, (
394+
f"{model_name}: bound default must equal explicit kwarg "
395+
f"(bound={len(bound_ids)}, explicit={len(explicit_ids)})"
396+
)
397+
assert bound._preserve_all_thinking is True
398+
assert bound._preserve_thinking_between_tool_calls is False
399+
400+
401+
def test_create_renderer_no_flags_is_zero_cost(renderer_name, tokenizer):
402+
"""When no flags are bound, attributes record the False bindings and
403+
``render_ids`` is unchanged from the underlying impl."""
404+
from renderers import create_renderer
405+
406+
r = create_renderer(tokenizer, renderer=renderer_name)
407+
assert r._preserve_all_thinking is False
408+
assert r._preserve_thinking_between_tool_calls is False
409+
410+
411+
def test_create_renderer_btc_default_matches_explicit(
412+
model_name, renderer_name, tokenizer
413+
):
414+
"""``preserve_thinking_between_tool_calls`` bound at construction
415+
must match the explicit-kwarg call."""
416+
from renderers import create_renderer
417+
from renderers.default import DefaultRenderer
418+
419+
bound = create_renderer(
420+
tokenizer,
421+
renderer=renderer_name,
422+
preserve_thinking_between_tool_calls=True,
423+
)
424+
if isinstance(bound, DefaultRenderer):
425+
with pytest.raises(NotImplementedError):
426+
bound.render_ids(CONVERSATION)
427+
return
428+
429+
bound_ids = bound.render_ids(CONVERSATION)
430+
unbound = create_renderer(tokenizer, renderer=renderer_name)
431+
explicit_ids = unbound.render_ids(
432+
CONVERSATION, preserve_thinking_between_tool_calls=True
433+
)
434+
assert bound_ids == explicit_ids, (
435+
f"{model_name}: bound btc default must equal explicit kwarg"
436+
)

uv.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)