Skip to content

fix(agent): preserve todos as tool state after compression - #58162

Closed
0xLeathery wants to merge 1 commit into
NousResearch:mainfrom
0xLeathery:fix/context-compression-todo-tool-history
Closed

fix(agent): preserve todos as tool state after compression#58162
0xLeathery wants to merge 1 commit into
NousResearch:mainfrom
0xLeathery:fix/context-compression-todo-tool-history

Conversation

@0xLeathery

Copy link
Copy Markdown
Contributor

Summary

  • Preserve active todo continuity across context compression as canonical assistant tool-call + tool result history instead of appending a synthetic user message.
  • Keep completed and cancelled todos out of the compressed continuation state.
  • Add regression coverage for user-turn preservation, provider-valid tool pairing, hydration, and insertion ordering.

Why

Context compression used to append the preserved todo snapshot as a fresh user message. That blurred internal continuity state with user intent, and could make preserved todo text look like the latest user turn after compaction.

This keeps the two concepts separate: real user messages stay user messages, and active todo state is carried as paired tool history that the existing todo hydration path can restore.

Unlike #35349, this does not fold todo state back into a user turn; it carries active todos as paired assistant/tool history.

Closes #26979.

Test plan

  • python3.11 manual compression smoke: verified latest real user turn remains the user tail, one paired todo tool result is inserted, only active todos are preserved, and completed todos are omitted.
  • python3 scripts/check-windows-footguns.py agent/conversation_compression.py tools/todo_tool.py tests/agent/test_context_compression_todo_leak.py tests/tools/test_todo_tool.py
  • Public diff hygiene scan: no secret-like assignments, local paths, or private process context in the changed diff.
  • scripts/run_tests.sh tests/agent/test_context_compression_todo_leak.py tests/tools/test_todo_tool.py tests/run_agent/test_run_agent.py tests/tools/test_read_loop_detection.py tests/agent/test_context_compressor.py tests/agent/test_context_compressor_summary_continuity.py tests/agent/test_context_compressor_temporal_anchoring.py tests/agent/test_compression_concurrent_fork.py tests/agent/test_compression_rotation_state.py tests/agent/test_compression_progress.py tests/agent/test_compressed_summary_metadata.py — 645 passed.

Platforms tested

  • macOS local development checkout.

@0xLeathery
0xLeathery marked this pull request as ready for review July 4, 2026 08:50
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 4, 2026
@0xLeathery
0xLeathery force-pushed the fix/context-compression-todo-tool-history branch from d4751e2 to 5579188 Compare July 4, 2026 08:55
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Competing fix cluster for #26979 (all open, different mechanisms): this PR carries active todos as paired assistant/tool history; #26981 removes the injection entirely; #18883 strips subjects from the injection. #35349 (closed) folded the snapshot back into a user turn, which this PR explicitly avoids. Not a duplicate of any -- flagging the cluster so a maintainer can pick the canonical approach.

@0xLeathery
0xLeathery marked this pull request as draft July 4, 2026 08:56

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The _STALE_TODO_SHOULD_NOT_BE_A_USER_MESSAGE sentinel strings are intentional test fixtures, not stale TODOs — confirmed by context. Core logic (preserving todos as tool state after compression) looks correct. Well-tested fix with dedicated unit tests.


Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for preserving todo state as a paired tool result rather than a synthetic user instruction. The underlying issue remains on current main: agent/conversation_compression.py:810-813 still appends format_for_injection() as role="user".

Problems

  • The current insertion order is stale against main. The PR returns unchanged history when it contains no user turn (agent/conversation_compression.py:126-134 in PR head 557918825581), but current main then restores a real user turn in _ensure_compressed_has_user_turn() (agent/conversation_compression.py:418-456, added by 6e176e4c). In that assistant/tool-only compression case, active todos are not preserved. Current main has an explicit regression for this shape at tests/agent/test_compression_concurrent_fork.py:206-241.

Suggested changes

  • Preserve the current user-turn guard, run it before inserting the assistant/tool todo pair, and add a regression covering the assistant-only compressor result plus todo hydration.

Automated hermes-sweeper review.

compressed.append({"role": "user", "content": todo_snapshot})
todo_state_messages = _todo_state_messages_for_compression(agent)
if todo_state_messages:
compressed = _insert_todo_state_messages(compressed, todo_state_messages)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main adds _ensure_compressed_has_user_turn() after this hunk. If compression returns assistant/tool-only history, this helper returns unchanged at line 134 and the later guard restores a user turn without any todo pair. Run the user-turn guard before this insertion and cover that current-main compatibility path.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026
@teknium1 teknium1 added the area/compression Context compression and continuation sessions label Jul 19, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #69860 (commit 2ca38e5). Issue #26979 is fixed via #69860 (merge-into-trailing-turn). Your preserve-as-tool-state direction is noted in the merged PR body as the possible longer-term shape — if you want to pursue it, it would now build on the merged behavior. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compression Context compression and continuation sessions comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avoid injecting raw todo snapshots as synthetic user messages after compression

4 participants