feat(run_agent): opt-in prune of historical reasoning_content beyond N turns - #22068
feat(run_agent): opt-in prune of historical reasoning_content beyond N turns#22068Julientalbot wants to merge 1 commit into
Conversation
…N turns Long agent sessions (Telegram bots, long CLI runs) accumulate reasoning_content / reasoning / reasoning_details on every assistant turn. Refs PR NousResearch#15250 (DeepSeek) and PR NousResearch#17400 (Kimi) made these fields sticky in the message store because those models *require* their own reasoning to be replayed for correctness. The downside: on grok-4.x and other agentic stacks, replaying every historical reasoning block can amount to several MB of cached tokens per turn, biasing the model toward narrative continuation rather than fresh tool execution. On a real Alfred Telegram session, the pivot into IWE happens around turn NousResearch#12 right after the assistant history accumulates dense reasoning content from earlier productive turns. This change adds an *opt-in*, off-by-default config knob to cap the reasoning history depth on the per-call payload only: agent: max_reasoning_history_turns: 5 # int or null/missing When unset (default), behaviour is unchanged — full history replays. When set to N (positive int), only the last N assistant turns retain their reasoning_content / reasoning / reasoning_details keys; older turns have these keys removed from the per-call copy. The stored session messages are never mutated. Models matching the new _REASONING_HISTORY_REQUIRED_MODELS class attribute — currently ("deepseek", "kimi", "moonshot") — short-circuit the prune with a one-time warning. Pruning would break tool-call accuracy on these models per the linked PRs. The new method _prune_reasoning_history is called right after _sanitize_api_messages at both call sites (the main loop and _handle_max_iterations), keeping the flow consistent. Tests (9 new) in TestPruneReasoningHistory: - test_default_no_op - test_prune_keeps_last_n_on_grok - test_blocklist_deepseek_no_op_with_warning - test_blocklist_kimi_no_op - test_blocklist_warning_is_idempotent - test_negative_n_is_no_op - test_assistant_count_below_n_no_op - test_non_assistant_messages_untouched - test_does_not_mutate_input Last in the IWE detection series for grok-4.x: - NousResearch#22055 — pass reasoning.effort to xAI Responses - NousResearch#22059 — extend codex intermediate-ack to French + relax bail - NousResearch#22061 — runtime tool_use_enforcement: required nudge - this PR — opt-in reasoning history prune (Draft for design review)
… pruning Three fixes for issues causing deepseek-v4-flash to stop/hang: 1. _has_separate_reasoning (NousResearch#22685): Skip post-tool empty nudge when thinking models return structured reasoning fields (reasoning_content/reasoning/reasoning_details) but empty content. Previously only inline <think> tags were checked. 2. Context compressor reads reasoning (NousResearch#19003): Use extract_content_or_reasoning() instead of reading only message.content. Thinking models (DeepSeek v4, Qwen3, GLM-5.1) return content in reasoning fields with empty content — causing the compressor to produce empty summaries. 3. Reasoning accumulation pruning (NousResearch#22068): Opt-in config agent.prune_reasoning_turns (default 0=disabled). When set, replaces reasoning_content on older assistant turns with space placeholder to prevent IWE (Intention Without Execution) in long sessions. Tests: tests/run_agent/test_deepseek_reasoning_content_echo.py (39) tests/agent/test_context_compressor.py (69) — all pass
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused, opt-in proposal. Current main has moved the affected request-building paths, and this needs design rework before it can be safely salvaged.
Problems
- The moving N-turn cutoff changes historical API messages after they have already been sent. That conflicts with the prompt-caching invariant in
AGENTS.md:1132-1137; current request assembly also normalizes payloads for stable prefixes inagent/conversation_loop.py:915+. - The proposed field list does not cover the stated xAI/OpenAI Responses case. Current xAI Responses tests require replaying encrypted reasoning in
tests/run_agent/test_codex_xai_oauth_recovery.py:399-476. - The model-substring blocklist conflicts with current endpoint-based Kimi/Moonshot detection in
run_agent.py:5486-5504, and omits the existing MiMo replay requirement. - The PR does not add the required
DEFAULT_CONFIGentry (AGENTS.md:584-590).
Suggested changes
- Settle the cache-safe replay design first, then implement it at the current request builders:
agent/conversation_loop.pyandagent/chat_completion_helpers.py. - Use the existing provider/transport capability predicates and add transport-level replay tests.
Automated hermes-sweeper review.
|
|
||
| _VALID_API_ROLES = frozenset({"system", "user", "assistant", "tool", "function", "developer"}) | ||
|
|
||
| # Models whose own reasoning_content must be replayed in history for |
There was a problem hiding this comment.
This model-substring blocklist does not match the current provider contract: run_agent.py:5486-5504 deliberately detects Kimi/Moonshot from the active provider/endpoint because OpenRouter-style re-exports can use the same model name but reject their native reasoning replay protocol. It also omits the current MiMo requirement.
| @@ -11582,6 +11676,7 @@ def run_conversation( | |||
| # gated on context_compressor — so orphans from session loading or | |||
There was a problem hiding this comment.
A turn that was previously sent with reasoning changes once it ages beyond N, invalidating the historical request prefix. AGENTS.md:1132-1137 explicitly forbids altering past context mid-conversation to preserve prompt caching; this needs a cache-safe design rather than a per-call moving cutoff.
Problem
Long agent sessions (Telegram bots, long CLI/headless runs) accumulate
reasoning_content/reasoning/reasoning_detailson every assistant turn. PRs #15250 (DeepSeek) and #17400 (Kimi) made these fields sticky in the message store because those models require their own reasoning to be replayed for correctness.The downside on other agentic stacks (grok-4.x, OpenAI o1/o4 via Responses, etc.): replaying every historical reasoning block can amount to several MB of cached tokens per turn, biasing the model toward narrative continuation rather than fresh tool execution.
This is empirically observable on a real-world Alfred Telegram session shipped with this branch's repro corpus: the pivot into IWE (Intention Without Execution — model says "I'll check the directory" and emits no tool_call) happens around turn #12, immediately after the assistant history accumulates dense reasoning content from earlier productive turns. The reasoning replay biases the model toward "reasoning about the next reasoning" rather than acting.
Fix (opt-in, off by default)
A new config key on the agent section, off by default — no behavioural drift for current users:
When unset (default
None), behaviour is unchanged — full history replays.When set to
N(positive int), only the last N assistant turns retain theirreasoning_content/reasoning/reasoning_detailskeys; older assistant turns have these keys removed from the per-call copy. The stored session messages are never mutated — session continuity, replays, and the session JSON on disk all remain untouched.Models matching the new class attribute
_REASONING_HISTORY_REQUIRED_MODELS = ("deepseek", "kimi", "moonshot")short-circuit the prune with a one-time warning, since pruning would break tool-call accuracy on these models (per #15250 / #17400).The new method
_prune_reasoning_historyis called right after_sanitize_api_messagesat both call sites (the main loop and_handle_max_iterations), keeping the flow consistent.Tests
9 new cases in
TestPruneReasoningHistory:test_default_no_op— no config = no behavioural changetest_prune_keeps_last_n_on_grok— N=2 on grok-4.3 strips reasoning from older assistant turns, keeps last 2test_blocklist_deepseek_no_op_with_warning— blocklist short-circuits + warningtest_blocklist_kimi_no_op— same for kimitest_blocklist_warning_is_idempotent— warning fires only once per agent instancetest_negative_n_is_no_op—-1is normalised toNoneat inittest_assistant_count_below_n_no_op— fewer assistant turns than N: nothing to prunetest_non_assistant_messages_untouched— user/tool messages never modifiedtest_does_not_mutate_input— caller's message list is never mutatedpytest tests/run_agent/test_run_agent.py -v -k PruneReasoningHistory→ 9 passed.Open questions for design review
Naming.
max_reasoning_history_turnsis descriptive but verbose. Alternatives:keep_recent_reasoning_turns,reasoning_history_window,prune_old_reasoning. Preference?Blocklist scope. Current list is
("deepseek", "kimi", "moonshot"). Should we add others (e.g. Qwen-3 / GLM that also usereasoning_contentchannels)? Should the blocklist be config-overridable, or kept hardcoded as a safety rail?Per-call vs stored. Current implementation prunes only the per-call copy (the conservative choice — preserves session JSON). Alternative: prune
self._session_messagesin place, freeing memory and shrinking the on-disk session file. Comments?Default policy. Should there be a sensible default for non-blocklisted models when the config is missing? (My current answer: no — opt-in only, no surprises.)
Interaction with
compression. Should the prune respectcompression.protect_last_n(don't strip reasoning from the protected window) or stay simple (independent N)? Current implementation: independent N.Pairing
Last in a small series of PRs improving IWE detection on grok-4.x and adjacent agentic stacks:
reasoning.effortto xAI Responses (otherwise silently dropped on the xAI direct path)_looks_like_codex_intermediate_ackto French + relax the global bail-on-prior-tooltool_use_enforcement: required(extends the existing detector to non-codex_responses paths)The four are independent and can land in any order.