Skip to content

fix(planners): keep all leading parallel function calls#6141

Closed
nyxst4ck wants to merge 1 commit into
google:mainfrom
nyxst4ck:fix/planner-leading-parallel-function-calls
Closed

fix(planners): keep all leading parallel function calls#6141
nyxst4ck wants to merge 1 commit into
google:mainfrom
nyxst4ck:fix/planner-leading-parallel-function-calls

Conversation

@nyxst4ck

Copy link
Copy Markdown
Contributor

Link to Issue or Description of Change

No existing issue — describing the bug here.

Problem:

PlanReActPlanner.process_planning_response drops every parallel function call
except the first when the model's response starts with a function call.

The trailing-group collector is guarded by:

first_fc_part_index = -1
for i in range(len(response_parts)):
    if response_parts[i].function_call:
        ...
        first_fc_part_index = i
        break
    ...
if first_fc_part_index > 0:   # <-- bug
    j = first_fc_part_index + 1
    while j < len(response_parts):
        ...

first_fc_part_index is the index of the first function call (sentinel -1).
When the first part is a function call its index is 0, so > 0 is false and
the loop that collects the rest of the parallel call group never runs — the
first call is kept, the rest are silently dropped. Responses that begin with
text (index >= 1) work, which is why this wasn't noticed. Gemini emitting a
group of parallel function calls as the first parts of a turn is a realistic
case (and is what the planner instruction encourages).

Solution:

Change the guard to >= 0 so a leading function call is handled the same as one
preceded by text.

Testing Plan

Unit Tests:

  • Added tests/unittests/planners/test_plan_re_act_planner.py.
  • All unit tests pass locally.

test_preserves_all_leading_parallel_function_calls is red on main
(returns only ["get_weather"]) and green with this change (returns
["get_weather", "get_time"]). A companion test confirms the leading-text case
still works.

$ pytest tests/unittests/planners/test_plan_re_act_planner.py -q
2 passed
$ pytest tests/unittests/flows/llm_flows/test_nl_planning.py -q
7 passed

pyink + isort clean.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have added tests that prove my fix is effective.
  • New and existing unit tests pass locally with my changes.

PlanReActPlanner.process_planning_response guarded the trailing
parallel-call collection loop with `if first_fc_part_index > 0`, but
first_fc_part_index is the index of the first function call (initialized to
-1). When the model response *begins* with a function call the index is 0, so
the guard was false and every parallel function call after the first was
silently dropped. Responses that start with text (index >= 1) were unaffected,
which is why this went unnoticed.

Use `>= 0` so the first part being a function call is handled too. Adds unit
tests for both the leading-call and leading-text cases.
@wyf7107 wyf7107 self-assigned this Jun 16, 2026
copybara-service Bot pushed a commit that referenced this pull request Jun 17, 2026
Merge #6141

### Link to Issue or Description of Change

No existing issue — describing the bug here.

**Problem:**

`PlanReActPlanner.process_planning_response` drops every parallel function call
except the first when the model's response **starts** with a function call.

The trailing-group collector is guarded by:

```python
first_fc_part_index = -1
for i in range(len(response_parts)):
    if response_parts[i].function_call:
        ...
        first_fc_part_index = i
        break
    ...
if first_fc_part_index > 0:   # <-- bug
    j = first_fc_part_index + 1
    while j < len(response_parts):
        ...
```

`first_fc_part_index` is the index of the first function call (sentinel `-1`).
When the first part is a function call its index is `0`, so `> 0` is false and
the loop that collects the rest of the parallel call group never runs — the
first call is kept, the rest are silently dropped. Responses that begin with
text (index `>= 1`) work, which is why this wasn't noticed. Gemini emitting a
group of parallel function calls as the first parts of a turn is a realistic
case (and is what the planner instruction encourages).

**Solution:**

Change the guard to `>= 0` so a leading function call is handled the same as one
preceded by text.

### Testing Plan

**Unit Tests:**

- [x] Added `tests/unittests/planners/test_plan_re_act_planner.py`.
- [x] All unit tests pass locally.

`test_preserves_all_leading_parallel_function_calls` is **red on `main`**
(returns only `["get_weather"]`) and **green** with this change (returns
`["get_weather", "get_time"]`). A companion test confirms the leading-text case
still works.

```
$ pytest tests/unittests/planners/test_plan_re_act_planner.py -q
2 passed
$ pytest tests/unittests/flows/llm_flows/test_nl_planning.py -q
7 passed
```
pyink + isort clean.

### Checklist

- [x] I have read the CONTRIBUTING.md document.
- [x] I have performed a self-review of my own code.
- [x] I have added tests that prove my fix is effective.
- [x] New and existing unit tests pass locally with my changes.

Co-authored-by: Yifan Wang <wanyif@google.com>
COPYBARA_INTEGRATE_REVIEW=#6141 from nyxst4ck:fix/planner-leading-parallel-function-calls 7058f5b
PiperOrigin-RevId: 933812571
@adk-bot

adk-bot commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Thank you @nyxst4ck for your contribution! 🎉

Your changes have been successfully imported and merged via Copybara in commit 054da5d.

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

@adk-bot adk-bot added the merged [Status] This PR is merged label Jun 17, 2026
@adk-bot adk-bot closed this Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged [Status] This PR is merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants