Skip to content

fix(agent): retire stale todo snapshots after completion - #89910

Open
eventh0riz0n wants to merge 1 commit into
NousResearch:mainfrom
eventh0riz0n:fix/compression-stale-todo-snapshot
Open

fix(agent): retire stale todo snapshots after completion#89910
eventh0riz0n wants to merge 1 commit into
NousResearch:mainfrom
eventh0riz0n:fix/compression-stale-todo-snapshot

Conversation

@eventh0riz0n

Copy link
Copy Markdown

Summary

  • Retire a preserved todo snapshot when the persisted store is authoritative but has no active tasks.
  • Preserve snapshots when the store is genuinely empty/unhydrated, maintaining compatibility with older sessions.
  • Strip stale todo scaffolding without losing multimodal user content, stale api_content, provenance, or strict role alternation.

Motivation and Context

After context compression, a previously injected todo snapshot could survive even after every persisted task was completed or cancelled. A later turn could then treat finished work as active again.

This extends the retained-todo behavior introduced in #69860: pending tasks are still preserved, while an authoritative completed/cancelled store now retires the old snapshot safely. This PR does not close #69860.

Related work and scope

How Has This Been Tested?

Red control on an unmodified origin/main with the new regressions applied:

  • stale completed snapshot: failed as expected
  • non-tail deletion/role repair: failed as expected
  • multimodal preservation/provenance: failed as expected

Green verification on this branch:

python -m pytest -q -o addopts= \
  tests/agent/test_compression_rotation_state.py \
  tests/run_agent/test_message_sequence_repair.py \
  tests/agent/test_context_compressor_zero_user_provenance.py \
  tests/agent/test_skill_todo_retention_parity.py \
  tests/tools/test_todo_tool.py
# 92 passed on current main

ruff check agent/agent_runtime_helpers.py agent/conversation_compression.py \
  tests/agent/test_compression_rotation_state.py
# All checks passed

git diff --check origin/main...HEAD
# clean

A separate final read-only review exercised 94 relevant tests and found no HIGH, MEDIUM, or LOW findings.

Test Configuration

  • Python: 3.11
  • OS: Linux

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Performance improvement
  • Code refactoring

Checklist

  • My code follows the code style of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have run pytest tests/ -q and all tests pass
  • Any dependent changes have been merged and published

Screenshots / Logs

Not applicable.

Reviewer Notes

The key compatibility guard is intentional: TodoStore.has_items() == False does not authorize deleting a retained snapshot, because an empty/unhydrated store cannot prove that the snapshot is stale. Only a non-empty authoritative store whose active rendering is empty retires it.

@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 area/compression Context compression and continuation sessions P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 19, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review - automated review for reference; please use your judgment.

Reviewed the diff. Carefully engineered retirement semantics: an authoritative-but-completed store now clears prior snapshots instead of resurrecting finished work, while genuinely empty stores (fresh gateway agents that cannot rehydrate post-compaction) keep theirs; deletion of standalone synthetic rows triggers _repair_message_sequence so assistant alternation survives, mixed rows are stripped in place and their synthetic provenance flag cleared, every content rewrite drops the stale api_content sidecar, and unknown/raising stores fail conservative. The test set covers each branch including the tricky middle-of-history deletion and multimodal part survival.

  • agent/conversation_compression.py ~3300 - only ONE stale snapshot is retired per pass (the loop breaks after the first match). History produced by several earlier compactions can contain multiple snapshot rows; the oldest would survive this pass and get re-injected under until a later boundary retires it. Converges eventually, but if that latency matters, iterate all matches and run the sequence repair once at the end (the repair helper already handles merged spans).

  • Nit: the _todo_has_items = getattr(agent._todo_store, "has_items", None) / callable / try-except ladder reads heavier than needed - hasattr plus try around the call would say the same thing; fine either way given the deliberate conservatism.

No blocking issues found.

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: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.

3 participants