fix(agents): preserve todos state across node updates#3180
Open
Huixin615 wants to merge 1 commit into
Open
Conversation
ThreadState.todos had no reducer, so any downstream node returning a partial state without todos was implicitly setting it to None, which LangGraph then used to overwrite the previously streamed value. This caused the to-do list to render correctly during streaming but vanish once streaming completed. Add a merge_todos reducer that keeps the last non-None value, mirroring the merge_artifacts pattern already used in the same file. An explicit empty list is still respected so that 'user cleared todos' works. Tests: 10 new unit tests in tests/test_thread_state_reducers.py covering merge_todos plus regression coverage for merge_artifacts and merge_viewed_images. All 69 thread-related tests pass locally. Closes bytedance#3123
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a reducer for ThreadState.todos so LangGraph partial state updates that omit todos do not clear previously streamed todo state, addressing issue #3123.
Changes:
- Adds
merge_todos(existing, new)with “preserve onNone, replace otherwise” semantics. - Annotates
ThreadState.todoswith the new reducer. - Adds reducer unit tests for todos plus sanity checks for existing artifact/image reducers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
backend/packages/harness/deerflow/agents/thread_state.py |
Defines and wires the merge_todos reducer into ThreadState.todos. |
backend/tests/test_thread_state_reducers.py |
Adds unit coverage for reducer behavior and existing reducer sanity checks. |
| title: NotRequired[str | None] | ||
| artifacts: Annotated[list[str], merge_artifacts] | ||
| todos: NotRequired[list | None] | ||
| todos: Annotated[list | None, merge_todos] |
Comment on lines
+8
to
+12
| from packages.harness.deerflow.agents.thread_state import ( | ||
| merge_artifacts, | ||
| merge_todos, | ||
| merge_viewed_images, | ||
| ) |
Collaborator
|
@Huixin615, thanks for your contribution. Please take a look at the review comment of Copilot. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a
merge_todosreducer toThreadState.todosso that a downstream node returning a partial state withouttodosno longer wipes the previously streamed value.Why
Closes #3123.
ThreadState.todoswas declared asNotRequired[list | None]with no reducer. In LangGraph, fields without a reducer use last-write-wins, and a missing key in a partial state update is materialized asNone, which overwrites the previous value. This is exactly the root cause @WillemJiang identified in the issue: the to-do list renders correctly during streaming, then disappears once a later node finishes with notodosin its return.How
merge_todos(existing, new)reducer inbackend/packages/harness/deerflow/agents/thread_state.py. Semantics:new is None→ preserveexisting(the actual fix for To-dos 列表在流式输出过程中显示,但输出完成后消失 #3123)new is not None(including[]) →newwins, so explicit clears still worktodos: Annotated[list | None, merge_todos], mirroring the existingartifacts: Annotated[list[str], merge_artifacts]pattern in the same file.Tests
New
backend/tests/test_thread_state_reducers.pywith 10 cases:merge_todos(5 cases, including the regression test for To-dos 列表在流式输出过程中显示,但输出完成后消失 #3123):new=Nonepreserves existing ← regression coverage for To-dos 列表在流式输出过程中显示,但输出完成后消失 #3123existing=Noneaccepts new[]is treated as explicit clearmerge_artifacts(3 cases) andmerge_viewed_images(2 cases) to lock in current behaviour.All 69 thread-related tests pass locally:
make lintandmake formatboth pass cleanly (only the 2 files in this PR are touched).Checklist
make lintpasses (ruff check+ruff format --check)make formatapplied