Skip to content

feat: only disable parallel tool calls when injectable tools are present#211

Merged
dannykopping merged 6 commits intomainfrom
dk/parallel-tool-calls
Mar 12, 2026
Merged

feat: only disable parallel tool calls when injectable tools are present#211
dannykopping merged 6 commits intomainfrom
dk/parallel-tool-calls

Conversation

@dannykopping
Copy link
Collaborator

@dannykopping dannykopping commented Mar 11, 2026

This PR refines when parallel tool calls are disabled. Previously, the system disabled parallel tool calls whenever any tools were present in a request; now it only disables them when injectable MCP tools are present.

This table describes what happens to the parallel tool call setting across all 3 interceptor types:

  ┌────────────────────────────────┬─────────────────┬─────────────────┬─────────────────┐
  │           Dimension            │    Anthropic    │ ChatCompletions │    Responses    │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ injected+builtin, set=enabled  │ forced disabled │ forced disabled │ forced disabled │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ injected+builtin, set=disabled │ forced disabled │ forced disabled │ forced disabled │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ injected+builtin, unset        │ forced disabled │ forced disabled │ forced disabled │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ injected only, set=enabled     │ forced disabled │ forced disabled │ forced disabled │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ injected only, set=disabled    │ forced disabled │ forced disabled │ forced disabled │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ injected only, unset           │ forced disabled │ forced disabled │ forced disabled │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ builtin only, set=enabled      │ preserved       │ preserved       │ preserved       │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ builtin only, set=disabled     │ preserved       │ preserved       │ preserved       │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ builtin only, unset            │ absent          │ absent          │ absent          │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ no tools, set=enabled          │ preserved       │ preserved       │ preserved       │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ no tools, set=disabled         │ preserved       │ preserved       │ preserved       │
  ├────────────────────────────────┼─────────────────┼─────────────────┼─────────────────┤
  │ no tools, unset                │ absent          │ absent          │ absent          │
  └────────────────────────────────┴─────────────────┴─────────────────┴─────────────────┘

(builtin tools mean tools defined in the initial request)

Copy link
Collaborator Author

@dannykopping dannykopping changed the title fix: only disable parallel tool calls when injectable tools are present feat: only disable parallel tool calls when injectable tools are present Mar 11, 2026
@dannykopping dannykopping force-pushed the dk/parallel-tool-calls branch from a0e150e to b2e4f03 Compare March 11, 2026 12:15
Signed-off-by: Danny Kopping <danny@coder.com>
@dannykopping dannykopping force-pushed the dk/parallel-tool-calls branch from b2e4f03 to 732f3d2 Compare March 11, 2026 12:27
@dannykopping dannykopping marked this pull request as ready for review March 11, 2026 12:30
return
}

// Disable parallel tool calls when injectable tools are present to simplify the inner agentic loop.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was somehow missed previously; we weren't setting parallel tool calls to disabled in blocking mode :thinking_face:

Copy link
Contributor

@ssncferreira ssncferreira left a comment

Choose a reason for hiding this comment

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

Nice change overall. But I think there's an edge case where parallel_tool_calls isn't disabled when the client sends no tools but MCP has tools to inject. Might not be a frequent scenario, but we should still handle it.

…y if injected tools provided

Signed-off-by: Danny Kopping <danny@coder.com>
…jected tools provided

Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Signed-off-by: Danny Kopping <danny@coder.com>
Copy link
Collaborator Author

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

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

@ssncferreira thanks for the review! This made me realize there were several shortcomings in both the implementation and the tests. They're now all aligned, and I've updated the description to reflect the desired behaviour.

Copy link
Contributor

@ssncferreira ssncferreira left a comment

Choose a reason for hiding this comment

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

LGTM 🌟 super clean, and really like the table in the PR description 🚀
I think we are missing 2 tests from anthropic for:

  • injected+builtin, set=enabled
  • injected+builtin, set=disabled

But non-blocking

expectDisableParallel: utils.PtrTo(true),
expectToolChoiceTypeInRequest: toolChoiceAuto,
},
// Request already has disable_parallel_tool_use set - without injected tools it should be preserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think we might be missing cases for injected+builtin tools with disable_parallel_tool_use explicitly set to true/false. The injected-only and builtin-only cases cover it, but the combination isn't tested.

Signed-off-by: Danny Kopping <danny@coder.com>
Copy link
Collaborator Author

dannykopping commented Mar 12, 2026

Merge activity

  • Mar 12, 1:48 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 12, 1:48 PM UTC: @dannykopping merged this pull request with Graphite.

@dannykopping dannykopping merged commit 4fd7ded into main Mar 12, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants