fix(todo): filter completed and cancelled items when hydrating todos - #25945
fix(todo): filter completed and cancelled items when hydrating todos#25945doublerebel wants to merge 3 commits into
Conversation
Cancelled (or completed) todo items could resurrect after subsequent agent turns because _hydrate_todo_store restored every item from the latest todo tool response in conversation history. Filter restored todo items to pending/in_progress so hydration matches TodoStore.format_for_injection(), which intentionally excludes completed and cancelled items. Closes NousResearch#7597. Closes NousResearch#7599. Upstream-PR: NousResearch#7595 Upstream-Commit: b12f1a9 (cherry picked from commit b12f1a9)
Add regression coverage for restoring only active todo items from conversation history and for reporting the filtered count in the hydration log message.
|
Thanks for picking this up! I closed my original PR #7595 because I wasn't 100% confident in the fix at the time (it was my first dive into this part of the codebase), but seeing you independently validate it and add test coverage is really reassuring. The diff looks correct to me — the pending/in_progress filter in _hydrate_todo_store is exactly the right spot. Appreciate the credit and the regression tests! |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying a real hydration/injection mismatch. Current main still restores the unfiltered list in run_agent.py:3594-3598, while tools/todo_tool.py:129-136 injects only active items after compression.
Problems
run_agent.py:5885assumes everytodoslist member is a dict. CurrentTodoStore._validate()intentionally handles non-dict members attools/todo_tool.py:166-167; guard or normalize before calling.get()so hydration does not regress toAttributeError.- The new histories at
tests/run_agent/test_run_agent.py:956and:975omit the assistanttodocall. Current main requires that pairing inrun_agent.py:3602-3646, so these tests will not hydrate after salvage.
Suggested changes
- Add a non-dict-entry regression case for the hydration path.
- Add a matching assistant
todocall before each new tool result, using the existing current-main test helper pattern.
This is an automated hermes-sweeper review.
| # the next turn hydrates the store from conversation history. | ||
| active_items = [ | ||
| item for item in last_todo_response | ||
| if item.get("status") in ("pending", "in_progress") |
There was a problem hiding this comment.
This assumes every parsed todos member is a dict. Current TodoStore._validate() explicitly handles non-dict entries (tools/todo_tool.py:166-167), but this .get() would now raise before that normalization. Guard or normalize malformed entries here and add coverage.
| history = [ | ||
| {"role": "user", "content": "plan"}, | ||
| { | ||
| "role": "tool", |
There was a problem hiding this comment.
These new histories (including the one below) lack the preceding assistant todo tool call now required by _tool_response_matches_todo_call() on main (run_agent.py:3602-3646). Add the matching assistant call before this tool result so the tests exercise hydration after salvage.
What does this PR do?
Adopts the narrow fix from #7595 so
_hydrate_todo_store()only restores active todo items from conversation history.Previously, hydration replayed every item from the last todo tool response, including
completedandcancelleditems. That disagreed withTodoStore.format_for_injection(), which intentionally omits completed/cancelled items from context injection. After context compression or a fresh agent turn, cancelled work could therefore reappear as active state.This PR preserves only
pendingandin_progressitems during hydration and logs how many inactive items were filtered.Credit/provenance:
Related Issue
Addresses #7597 and #7599.
Note: both issues are currently closed upstream, but current
origin/mainstill restores inactive todo items from_hydrate_todo_store(). This PR applies the closed-but-unmerged fix path and adds regression coverage.Type of Change
Changes Made
run_agent.pypending/in_progressbefore writing them into the todo store.TodoStore.format_for_injection()so completed/cancelled work is not resurrected.tests/run_agent/test_run_agent.pyscripts/release.pyAUTHOR_MAPentries for the adopted upstream author and this branch author so the contributor attribution check can pass.How to Test
Reproduction on current
origin/main:_hydrate_todo_store()receives a todo tool response containing mixed statuses:pendingin_progresscompletedcancelled_hydrate_todo_store(history).origin/mainwrites all four items back to the todo store.Fixed behavior on this branch:
Run:
Confirm only
pendingandin_progressitems are restored.Confirm the restore message reports the filtered inactive count.
Security review:
origin/main.Full-suite local note:
I ran the full test suite locally via the repository runner, equivalent to the PR-template
pytest tests/ -qcheck:Result:
94 failed, 22771 passed, 90 skipped, 2 errorson macOS/darwin-arm64. The failures were broad and outside this PR's touched files, concentrated in existing LSP/client e2e, provider/auxiliary/compression, CLI setup/config/gateway/update/completion, process/terminal/live-system-guard/zombie cleanup, file-tool state/staleness guards, plugin/kanban discovery, browser/TTS/code-execution timeout, and TUI gateway goal-command tests. The two collection errors were intests/hermes_cli/test_web_oauth_dispatch.pyandtests/plugins/test_kanban_dashboard_plugin.py.Focused tests for this change passed:
scripts/run_tests.sh tests/run_agent/test_run_agent.py::TestHydrateTodoStore tests/tools/test_todo_tool.py -q->18 passed.E2E suite passed separately:
python -m pytest tests/e2e/ -v --tb=short->56 passed, 7 skipped.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — N/Acli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AFor New Skills
N/A — this PR does not add or modify a skill.
Screenshots / Logs
N/A — no UI screenshots or log excerpts are needed for this non-visual state-hydration fix.