feat(thinking): replace preserve_* bools with thinking_retention, respected by the bridge

[hallerite]

What

Two changes that go hand in hand: a single, clearer thinking-retention knob, and making the bridge actually respect it during RL.

1. thinking_retention replaces the preserve_* bools

Replaces the two confusing boolean flags on _BaseRendererConfig:

preserve_all_thinking: bool = False
preserve_thinking_between_tool_calls: bool = False

with a single ordinal field whose default names the floor:

thinking_retention: Literal["template", "tool_cycle", "all"] = "template"

• "template" (default) — defer to the chat template (the floor; never drops what the template keeps)
• "tool_cycle" — additionally keep thinking inside the in-flight tool cycle (the A-T-…-A block after the last user turn, when it contains a tool response)
• "all" — additionally keep thinking on every past assistant turn

preserve_all_thinking=False never meant "drop" — it meant "defer to the template." A plain bool reads as a symmetric on/off switch, so False looked like "drop." The ordinal gives a named baseline ("template"), encodes the "all" ⊇ "tool_cycle" ⊇ "template" nesting, and removes the nonsensical both=True combo.

2. The bridge now respects thinking_retention (faithfulness fix)

render() is byte-faithful to each model's chat template, but bridge_to_next_turn carried prior tokens verbatim — so a multi-turn RL rollout kept <think> blocks the template strips once a new user turn arrives, feeding the model context its own template would never produce (unfaithful to native serving, OOD).

The bridge now consults thinking_retention:

Situation	Bridge behavior
new user query + prior thinking + template/tool_cycle	declines → caller re-renders, byte-faithful to apply_chat_template (stale thinking dropped)
thinking_retention="all"	extends verbatim (template keeps it too)
in-flight tool cycle (no new user query)	extends verbatim (template keeps in-cycle thinking)
no prior thinking	extends verbatim (nothing to strip)

This makes thinking_retention the single knob governing thinking end-to-end (render and the bridge). The extension property and "drop old thinking across a user turn" are mutually exclusive, so faithfulness requires a re-render at that boundary — exactly the path the caller already falls back to.

Wired across every thinking renderer via one shared reject_thinking_strip_in_extension helper (sibling to reject_assistant_in_extension), so each bridge is a one-liner. The helper detects a sampled thinking block by a token-subsequence marker:

• qwen3, qwen35, qwen36, glm45, glm5, nemotron3 — single </think> token.
• kimi_k25 — multi-token </think> close.
• gpt_oss — harmony analysis-channel header <|channel|>analysis<|message|>.

(minimax_m2 has no bridge — it always re-renders, already faithful. deepseek_r1 strips history unconditionally with no override — edge case, not wired.)

3. Reconcile the per-renderer thinking-kwargs with thinking_retention

GLM's clear_thinking and Nemotron's truncate_history_thinking are byte-equivalent to thinking_retention (=False ≡ "all"). Rather than silently resolving a contradiction, a model_validator on GLM5/GLM51/Nemotron3/Nemotron3Ultra raises when both are explicitly set to conflicting values (e.g. clear_thinking=False + thinking_retention="template"). It only fires when both are explicit, so the per-kwarg parity matrix (one field at a time) is unaffected. When the keep-history kwarg is set alone (a legit "keep all" intent), the glm5/nemotron3 bridges fold it into the effective retention (=False → "all") so the guard doesn't over-decline and re-tokenize sampled thinking.

Changes

• should_preserve_past_thinking branches on the level (all → True, template → False, else tool-cycle logic).
• All 8 renderer call sites, the AutoRendererConfig carry-over + guard, and DefaultRenderer's guard updated for thinking_retention.
• New shared introduces_user_query(), _contains_subsequence(), reject_thinking_strip_in_extension() in base.py; wired into all eight thinking-renderer bridges.
• _reject_thinking_retention_conflict() + model_validators on the four configs that carry a thinking-kwarg; DefaultRendererConfig (which is extra="allow") also rejects the removed preserve_* keys instead of silently forwarding them to apply_chat_template.
• Docstrings/comments across the no-op renderers, plus README and the renderer-config doc.

Breaking change

The old boolean keys are no longer accepted (extra="forbid" rejects them at config-load). Downstream TOML/configs (prime-rl, verifiers) must migrate:

Old	New
(unset)	thinking_retention="template"
preserve_thinking_between_tool_calls=True	thinking_retention="tool_cycle"
preserve_all_thinking=True	thinking_retention="all"

The README + renderer-config doc are reframed to match: faithfully reproducing the chat template is the goal (the bridge preserves sampled tokens as a means to that, not a license to diverge); thinking-stripping across a user turn is intended template behaviour the renderer reproduces, not an "avoided failure mode"; thinking_retention is the explicit override.

Follow-ups (separate PRs)

• Downstream preserve_* → thinking_retention migration in prime-rl / verifiers.

Test plan

• tests/test_bridge.py — reject_thinking_strip_in_extension wired per renderer; test_bridge_declines_across_user_query_when_template_drops_thinking (qwen3 deep, diffs bridge decision + fallback render vs apply_chat_template) and a parametrized test_bridge_declines_across_user_turn_when_thinking_present across all nine guarded renderers; mechanic tests isolated via a thinking_retention="all" fixture
• tests/test_renderer_config.py — new test_thinking_retention_conflict_raises, test_default_renderer_config_rejects_legacy_preserve_flags; tests/test_bridge.py — test_bridge_keeps_thinking_when_history_kwarg_disables_truncation
• tests/test_renderer_config_parity.py (367 passed — validator does not break the per-kwarg matrix), test_preserve_thinking.py, test_incremental.py, test_client.py
• ruff check / ruff format --check clean (isolated defaults, line-length 88)

🤖 Generated with Claude Code

---

Note

Medium Risk
Breaking config API (preserve_* removed) and changed bridge behavior on multi-turn RL when past thinking meets a new user turn under default retention—more full re-renders but more faithful to apply_chat_template.

Overview
Replaces preserve_all_thinking / preserve_thinking_between_tool_calls with a single nested thinking_retention level ("template" → "tool_cycle" → "all") on all renderer configs, with should_preserve_past_thinking and create_renderer auto-carry updated accordingly. DefaultRenderer only allows "template"; legacy preserve_* keys are rejected (including on DefaultRendererConfig so they are not forwarded to Jinja).

bridge_to_next_turn now uses shared helpers (introduces_user_query, reject_thinking_strip_in_extension) so thinking renderers return None when a new real user message would cross a boundary where the chat template drops prior thinking but the bridge would keep those tokens verbatim—unless thinking_retention="all", thinking is off, or there is no thinking marker. GLM/Nemotron bridges fold clear_thinking=False / truncate_history_thinking=False into effective "all" retention; conflicting explicit template kwargs vs thinking_retention raise at config load.

Docs and tests are updated for the migration and for bridge decline/extend behavior across guarded models.

^{Reviewed by Cursor Bugbot for commit b140e28. Bugbot is set up for automated code reviews on this repo. Configure here.}

Note

Replace preserve_all_thinking/preserve_thinking_between_tool_calls bools with a single thinking_retention level in renderer configs and bridge logic

• Introduces ThinkingRetention as a Literal["template", "tool_cycle", "all"] type in renderers/configs.py, replacing the two boolean preserve_* flags on BaseRendererConfig.
• Updates should_preserve_past_thinking in renderers/base.py to accept thinking_retention and implement three distinct behaviors: "template" never overrides, "tool_cycle" preserves only within the current tool cycle, and "all" always preserves.
• Adds reject_thinking_strip_in_extension helper that causes bridge_to_next_turn to return None (forcing a full re-render) when a new user turn arrives and prior thinking markers are present, unless thinking_retention="all" or thinking is disabled.
• All thinking renderers (Qwen3, Qwen3.5, Nemotron3, GLM-4.5, GLM-5, KimiK2.5, GPT-OSS) updated to use thinking_retention in both render and bridge paths.
• Config validators now raise ValueError on conflicting settings (e.g., clear_thinking=False with thinking_retention!="all").
• Risk: bridges that previously extended across user turns may now return None and trigger a full re-render when past thinking is present and thinking_retention is not "all".

^{Macroscope summarized b140e28.}

[cursor]

DefaultRenderer legacy flags slip through

Medium Severity

The PR treats legacy preserve_* booleans as rejected at load time, but DefaultRendererConfig uses extra="allow", so those keys become model_extra instead of typed fields. DefaultRenderer only checks thinking_retention, so construction no longer raises NotImplementedError and extras may still reach apply_chat_template.

Additional Locations (1)

• renderers/configs.py#L108-L117

 

^{Reviewed by Cursor Bugbot for commit ce78cae. Configure here.}

[macroscopeapp]

Approvability

Verdict: Needs human review

While presented as a config refactor (two bools → one enum), this PR introduces new bridge behavior that declines to span user-query boundaries when thinking would be stripped. This runtime behavior change affects multiple renderers and warrants human review beyond the mechanical rename aspects.

No code changes detected at b140e28. Prior analysis still applies.

^{You can customize Macroscope's approvability policy. Learn more.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b140e28. Configure here.}

[cursor]

Parse calls use wrong kwarg

High Severity

parse_glm and parse_qwen35 still require think_end_id, but GLM and Qwen3.5-family parse_response paths now pass thinking_marker_ids instead. That raises a TypeError on the first parse call for those renderers, so completion parsing breaks entirely.

Additional Locations (2)

• renderers/glm45.py#L295-L296
• renderers/qwen35.py#L596-L597

 

^{Reviewed by Cursor Bugbot for commit b140e28. Configure here.}