Skip to content

feat: bind preserve_*_thinking at construction (drop _bind_preserve_defaults)#5

Merged
hallerite merged 2 commits intomainfrom
feat/preserve-thinking-as-attribute
May 7, 2026
Merged

feat: bind preserve_*_thinking at construction (drop _bind_preserve_defaults)#5
hallerite merged 2 commits intomainfrom
feat/preserve-thinking-as-attribute

Conversation

@hallerite
Copy link
Copy Markdown
Member

Summary

Moves `preserve_all_thinking` / `preserve_thinking_between_tool_calls` from `render` / `render_ids` call-site kwargs to constructor-only kwargs on every renderer. Each renderer stores them as `_preserve_all_thinking` / `_preserve_thinking_between_tool_calls` instance attributes and consults them internally via `should_preserve_past_thinking`.

Kills the method-monkey-patching `_bind_preserve_defaults` helper from #4: with construction-time configuration there is nothing left to bind after the fact. `Renderer.render` / `render_ids` signatures shrink back to their pre-#3 shape.

Why

In real downstream usage (`verifiers.RendererClient`, prime-rl's `RendererConfig`), the flags are config-time, not per-render — bound once at pool creation, never changed per render. Call-site kwargs were over-engineered for that shape, and the wrapper that made them work hid the real method signature from type-checkers and IDE tooling.

"Renderer = configuration" is cleaner: build a different renderer (or pool) when you want a different configuration.

What changed

  • `renderers/base.py`: drop `_bind_preserve_defaults`; `create_renderer` / `create_renderer_pool` forward the flags directly to the constructor. `Renderer` Protocol's `render` / `render_ids` no longer expose the flags.
  • Every concrete renderer (`glm5`, `glm45`, `qwen3`, `qwen35`, `qwen36`, `qwen3_vl`, `minimax_m2`, `nemotron3`, `kimi_k2`, `kimi_k25`, `gpt_oss`, `deepseek_v3`, `default`): accept the flags in `init`, store as attrs, drop them from render signatures.
  • `DefaultRenderer` raises `NotImplementedError` at construction now (fail-fast) rather than at first render call.
  • README §1: new "Preserving past reasoning (constructor-only overrides)" subsection with a worked compaction example motivating why someone would flip `preserve_all_thinking=True` (a user-turn summariser would otherwise lose all of the trajectory's reasoning).
  • README §3.d: cross-link to the constructor-only override.
  • Tests: rewrite to build per-flag renderers (no more call-site kwargs).
  • Bump 0.1.8 → 0.1.9.

Test plan

  • `pytest tests/` — 893 passed, 48 skipped, 1 xfailed locally (no parity regressions).
  • `tests/test_preserve_thinking.py` covers: byte-identical default render, monotonic token-count growth under `preserve_all_thinking=True` (or no-op for renderers whose template never preserves), strict-subset relation `default ≤ btc ≤ all`, sentinel-visibility on the two-block fixture, `DefaultRenderer` raising at construction.
  • `should_preserve_past_thinking` helper unaffected — same signature, same semantics.

🤖 Generated with Claude Code

hallerite and others added 2 commits May 7, 2026 13:31
…efaults)

Move preserve_all_thinking and preserve_thinking_between_tool_calls from
``render`` / ``render_ids`` call-site kwargs to constructor kwargs on every
renderer. Each renderer stores them as ``_preserve_all_thinking`` /
``_preserve_thinking_between_tool_calls`` instance attributes and consults
them internally via ``should_preserve_past_thinking``.

This kills the method-monkey-patching ``_bind_preserve_defaults`` helper
introduced in #4: with construction-time configuration there is nothing
left to bind after the fact. The Renderer Protocol's ``render`` /
``render_ids`` signatures shrink back to their pre-#3 shape.

Why: in real usage downstream of this package (verifiers ``RendererClient``,
prime-rl ``RendererConfig``) the flags are config-time, not per-render.
Call-site kwargs were over-engineered for that shape — and the wrapper
that made them work was a method-rebind trick that hid the real method
signature from type-checkers and IDE tooling. "Renderer = configuration"
is cleaner: build a different renderer (or pool) when you want a
different configuration.

Changes:
- ``renderers/base.py``: drop ``_bind_preserve_defaults``; ``create_renderer``
  / ``create_renderer_pool`` forward the flags directly to the constructor.
  Protocol's ``render`` / ``render_ids`` no longer expose the flags.
- Every concrete renderer (``glm5``, ``glm45``, ``qwen3``, ``qwen35``,
  ``qwen36``, ``qwen3_vl``, ``minimax_m2``, ``nemotron3``, ``kimi_k2``,
  ``kimi_k25``, ``gpt_oss``, ``deepseek_v3``, ``default``): accept the
  flags in ``__init__``, store as attrs, drop them from render signatures.
- ``DefaultRenderer`` raises NotImplementedError at construction now
  (fail fast) rather than at first render call.
- README §1: new "Preserving past reasoning (constructor-only overrides)"
  subsection, with a worked compaction example motivating why one would
  flip ``preserve_all_thinking=True`` (a user-turn summariser would
  otherwise lose all of the trajectory's reasoning).
- README §3.d: cross-link to the constructor-only override.
- Tests: rewrite to build per-flag renderers (no call-site kwargs).
- Bump 0.1.8 → 0.1.9.

893 passed, 48 skipped, 1 xfailed locally — no parity regressions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-commit auto-applied line wrapping on the long
``preserve_thinking_between_tool_calls`` attribute assignments.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@hallerite hallerite merged commit 815610e into main May 7, 2026
6 checks passed
@hallerite hallerite deleted the feat/preserve-thinking-as-attribute branch May 7, 2026 13:40
hallerite added a commit that referenced this pull request May 7, 2026
PyPI's latest published renderers is 0.1.5 — the in-tree bumps in #3#5 → this PR (0.1.6 → 0.1.7 → 0.1.8 → 0.1.9 → 0.1.10) accumulated
against PyPI without ever publishing. Roll forward to 0.1.6 once for
the actual release; tag ``renderers-v0.1.6`` post-merge to fire the
publish workflow (.github/workflows/publish.yml asserts the tag's
``-vX.Y.Z`` suffix matches pyproject.toml).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hallerite added a commit that referenced this pull request May 7, 2026
…preserve_thinking) (#6)

* feat: drop legacy chat-template-pass-through kwargs

``GLM5Renderer.__init__`` no longer accepts ``clear_thinking``, and
``Qwen36Renderer.__init__`` no longer accepts ``preserve_thinking``.
Both were chat-template-kwarg pass-throughs whose default values are
now baked straight into the renderer:

- GLM-5 default: drop ``<think>`` on past-cycle assistants, keep on
  in-flight cycle and trailing assistant. (What ``clear_thinking=True``
  did before.) The render gate is now ``msg_idx > last_user_index or
  preserve_thinking`` — clean and consistent with the rest of the
  family.
- Qwen3.6 default: identical to Qwen3.5 (``msg_idx > last_query_index``
  via the inherited ``_should_render_thinking``). The class shrinks to
  just the ``_render_arg_value`` override, which is the only real
  template delta vs Qwen3.5.

Callers who want the toggled-on behaviour pass
``preserve_all_thinking=True`` to ``create_renderer`` — the
renderer-agnostic spelling of the same intent landed in #3/#5. Two
regression tests pin the dropped kwargs (constructor must raise
``TypeError`` when either is passed).

The README's §1 "Preserving past reasoning" subsection (added in #5)
already motivates the override flags; no new prose needed here. §3.d
is unchanged (still cross-links to the override flag for compaction).

Bump 0.1.9 → 0.1.10. 895 tests pass locally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: consolidate version to 0.1.6 (PyPI-aligned)

PyPI's latest published renderers is 0.1.5 — the in-tree bumps in #3#5 → this PR (0.1.6 → 0.1.7 → 0.1.8 → 0.1.9 → 0.1.10) accumulated
against PyPI without ever publishing. Roll forward to 0.1.6 once for
the actual release; tag ``renderers-v0.1.6`` post-merge to fire the
publish workflow (.github/workflows/publish.yml asserts the tag's
``-vX.Y.Z`` suffix matches pyproject.toml).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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