Skip to content

fix: concatenate list values in deep_merge_dicts during parallel tool call merge - #5191

Open
enjoykumawat wants to merge 2 commits into
google:mainfrom
enjoykumawat:fix/parallel-state-delta-list-merge
Open

fix: concatenate list values in deep_merge_dicts during parallel tool call merge#5191
enjoykumawat wants to merge 2 commits into
google:mainfrom
enjoykumawat:fix/parallel-state-delta-list-merge

Conversation

@enjoykumawat

Copy link
Copy Markdown
Contributor

Summary

  • Bug: When multiple tool calls run in parallel and each writes to the same state_delta key containing a list value, deep_merge_dicts silently drops all but the last value because lists hit the else branch and get overwritten.
  • Fix: Add a list-type check in deep_merge_dicts so list values are concatenated (d1[key] + value) instead of overwritten, preserving all entries from parallel function responses.
  • Tests: Added 5 unit tests covering list concatenation, scalar overwrite, nested dict merge, mixed-type handling, and an integration test for merge_parallel_function_response_events with state_delta lists.

Reproduction

# Tool A's delta: {"state_delta": {"items": ["a"]}}# Tool B's delta: {"state_delta": {"items": ["b"]}}# Before fix: {"state_delta": {"items": ["b"]}} — item "a" is lost# After fix: {"state_delta": {"items": ["a", "b"]}} — both preserved

Test plan

  • All 5 new unit tests pass (test_deep_merge_dicts_* and test_merge_parallel_function_response_events_merges_state_delta_lists)
  • All 32 existing tests in test_functions_simple.py pass (0 regressions)
  • Manual: create parallel tool calls that accumulate list state and verify no data loss

Fixes#5190

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please move the tests to test_functions_parallel.py

@wuliang229wuliang229 self-assigned this Apr 7, 2026
@enjoykumawat

Copy link
Copy Markdown
ContributorAuthor

Done! Moved all the deep_merge_dicts and state_delta list tests to test_functions_parallel.py in commit 4b3db55. Also merged latest main to pick up any upstream changes. CI is re-running now.

@enjoykumawat

Copy link
Copy Markdown
ContributorAuthor

Hi @wuliang229, just following up — the tests have been moved to test_functions_parallel.py as requested (commit 4b3db55) and CI is passing. Would you have a chance to re-review when you get a moment? Happy to make any additional changes if needed.

@rohityanrohityan added needs review [Status] The PR/issue is awaiting review from the maintainer core [Component] This issue is related to the core interface and implementation labels Apr 9, 2026
copybara-serviceBot pushed a commit that referenced this pull request Jun 22, 2026
… call merge
Merge #5191
## Summary
- **Bug:** When multiple tool calls run in parallel and each writes to the same `state_delta` key containing a list value, `deep_merge_dicts` silently drops all but the last value because lists hit the `else` branch and get overwritten.
- **Fix:** Add a list-type check in `deep_merge_dicts` so list values are concatenated (`d1[key] + value`) instead of overwritten, preserving all entries from parallel function responses.
- **Tests:** Added 5 unit tests covering list concatenation, scalar overwrite, nested dict merge, mixed-type handling, and an integration test for `merge_parallel_function_response_events` with `state_delta` lists.
## Reproduction
```python
# Tool A's delta: {"state_delta": {"items": ["a"]}}
# Tool B's delta: {"state_delta": {"items": ["b"]}}
# Before fix: {"state_delta": {"items": ["b"]}} — item "a" is lost
# After fix: {"state_delta": {"items": ["a", "b"]}} — both preserved
```
## Test plan
- [x] All 5 new unit tests pass (`test_deep_merge_dicts_*` and `test_merge_parallel_function_response_events_merges_state_delta_lists`)
- [x] All 32 existing tests in `test_functions_simple.py` pass (0 regressions)
- [ ] Manual: create parallel tool calls that accumulate list state and verify no data loss
Fixes#5190
Co-authored-by: George Weale <gweale@google.com>
COPYBARA_INTEGRATE_REVIEW=#5191 from enjoykumawat:fix/parallel-state-delta-list-merge 7425481
PiperOrigin-RevId: 936119923
@adk-bot

Copy link
Copy Markdown
Collaborator

Thank you @enjoykumawat for your contribution! 🎉

Your changes have been successfully imported and merged via Copybara in commit 1ff84eb.

Closing this PR as the changes are now in the main branch.

@adk-botadk-bot added the merged [Status] This PR is merged label Jun 22, 2026
@adk-botadk-bot closed this Jun 22, 2026
copybara-serviceBot pushed a commit that referenced this pull request Jun 22, 2026
… call merge
Merge #5191
## Summary
- **Bug:** When multiple tool calls run in parallel and each writes to the same `state_delta` key containing a list value, `deep_merge_dicts` silently drops all but the last value because lists hit the `else` branch and get overwritten.
- **Fix:** Add a list-type check in `deep_merge_dicts` so list values are concatenated (`d1[key] + value`) instead of overwritten, preserving all entries from parallel function responses.
- **Tests:** Added 5 unit tests covering list concatenation, scalar overwrite, nested dict merge, mixed-type handling, and an integration test for `merge_parallel_function_response_events` with `state_delta` lists.
## Reproduction
```python
# Tool A's delta: {"state_delta": {"items": ["a"]}}
# Tool B's delta: {"state_delta": {"items": ["b"]}}
# Before fix: {"state_delta": {"items": ["b"]}} — item "a" is lost
# After fix: {"state_delta": {"items": ["a", "b"]}} — both preserved
```
## Test plan
- [x] All 5 new unit tests pass (`test_deep_merge_dicts_*` and `test_merge_parallel_function_response_events_merges_state_delta_lists`)
- [x] All 32 existing tests in `test_functions_simple.py` pass (0 regressions)
- [ ] Manual: create parallel tool calls that accumulate list state and verify no data loss
Fixes#5190
PiperOrigin-RevId: 936249245
@GWealeGWeale reopened this Jun 23, 2026
@enjoykumawat

Copy link
Copy Markdown
ContributorAuthor

Hi @GWeale — I see this was reopened after the Copybara merge (commit 1ff84eb is already in main). Is there a follow-up you'd like from me, or was this reopened for internal tracking? Happy to help.

@enjoykumawat
enjoykumawatforce-pushed the fix/parallel-state-delta-list-merge branch from 7425481 to e625f51CompareAugust 5, 2026 10:10
@enjoykumawat

Copy link
Copy Markdown
ContributorAuthor

Quick question before this gets another look — digging into the Copybara history here, I see this was actually merged internally (1ff84eb, 2026-06-22) and then reverted about 4 hours later by an internal commit (fda23474, same day), with no reason given in either commit message. The PR was then reopened.

Was there a concern found internally with this approach (the list-concatenation logic in deep_merge_dicts, or something else) that led to the revert? I've since rebased this onto current main — the bug is still present there (list values in state_delta still get silently overwritten instead of concatenated) — and all tests pass, but I'd rather understand what went wrong the first time than have the same thing happen again. Happy to adjust the approach if there's a reason this shouldn't land as-is.

FrigaZzz pushed a commit to FrigaZzz/adk-python that referenced this pull request Aug 11, 2026
… call merge
Merge google#5191
## Summary
- **Bug:** When multiple tool calls run in parallel and each writes to the same `state_delta` key containing a list value, `deep_merge_dicts` silently drops all but the last value because lists hit the `else` branch and get overwritten.
- **Fix:** Add a list-type check in `deep_merge_dicts` so list values are concatenated (`d1[key] + value`) instead of overwritten, preserving all entries from parallel function responses.
- **Tests:** Added 5 unit tests covering list concatenation, scalar overwrite, nested dict merge, mixed-type handling, and an integration test for `merge_parallel_function_response_events` with `state_delta` lists.
## Reproduction
```python
# Tool A's delta: {"state_delta": {"items": ["a"]}}
# Tool B's delta: {"state_delta": {"items": ["b"]}}
# Before fix: {"state_delta": {"items": ["b"]}} — item "a" is lost
# After fix: {"state_delta": {"items": ["a", "b"]}} — both preserved
```
## Test plan
- [x] All 5 new unit tests pass (`test_deep_merge_dicts_*` and `test_merge_parallel_function_response_events_merges_state_delta_lists`)
- [x] All 32 existing tests in `test_functions_simple.py` pass (0 regressions)
- [ ] Manual: create parallel tool calls that accumulate list state and verify no data loss
Fixesgoogle#5190
Co-authored-by: George Weale <gweale@google.com>
COPYBARA_INTEGRATE_REVIEW=google#5191 from enjoykumawat:fix/parallel-state-delta-list-merge 7425481
PiperOrigin-RevId: 936119923
FrigaZzz pushed a commit to FrigaZzz/adk-python that referenced this pull request Aug 11, 2026
… call merge
Merge google#5191
## Summary
- **Bug:** When multiple tool calls run in parallel and each writes to the same `state_delta` key containing a list value, `deep_merge_dicts` silently drops all but the last value because lists hit the `else` branch and get overwritten.
- **Fix:** Add a list-type check in `deep_merge_dicts` so list values are concatenated (`d1[key] + value`) instead of overwritten, preserving all entries from parallel function responses.
- **Tests:** Added 5 unit tests covering list concatenation, scalar overwrite, nested dict merge, mixed-type handling, and an integration test for `merge_parallel_function_response_events` with `state_delta` lists.
## Reproduction
```python
# Tool A's delta: {"state_delta": {"items": ["a"]}}
# Tool B's delta: {"state_delta": {"items": ["b"]}}
# Before fix: {"state_delta": {"items": ["b"]}} — item "a" is lost
# After fix: {"state_delta": {"items": ["a", "b"]}} — both preserved
```
## Test plan
- [x] All 5 new unit tests pass (`test_deep_merge_dicts_*` and `test_merge_parallel_function_response_events_merges_state_delta_lists`)
- [x] All 32 existing tests in `test_functions_simple.py` pass (0 regressions)
- [ ] Manual: create parallel tool calls that accumulate list state and verify no data loss
Fixesgoogle#5190
PiperOrigin-RevId: 936249245
@enjoykumawat
enjoykumawatforce-pushed the fix/parallel-state-delta-list-merge branch from e625f51 to 7cb5d0eCompareAugust 12, 2026 11:47
… call merge
When multiple tool calls run in parallel and each writes to the same
state_delta key containing a list value, deep_merge_dicts silently drops
all but the last value because lists hit the else branch and get
overwritten.
Add a list-type check so that list values are concatenated instead of
overwritten, preserving all entries from parallel function responses.
Fixesgoogle#5190
…nctions_parallel
Move tests for deep_merge_dicts and merge_parallel_function_response_events
with list state_delta merging from test_functions_simple.py to
test_functions_parallel.py per reviewer feedback.
@enjoykumawat
enjoykumawatforce-pushed the fix/parallel-state-delta-list-merge branch from 7cb5d0e to 74cd173CompareAugust 23, 2026 07:29
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core[Component] This issue is related to the core interface and implementationmerged[Status] This PR is mergedneeds review[Status] The PR/issue is awaiting review from the maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parallel tool calls: state_delta list values silently overwritten during merge

5 participants

@enjoykumawat@adk-bot@wuliang229@GWeale@rohityan