Skip to content

fix: run tool calls concurrently in the Python client - #137

Merged
Elias Posen (eliasposen) merged 3 commits into
mainfrom
fix/concurrency
Jul 27, 2026
Merged

fix: run tool calls concurrently in the Python client#137
Elias Posen (eliasposen) merged 3 commits into
mainfrom
fix/concurrency

Conversation

@eliasposen

Copy link
Copy Markdown
Contributor

Problem

The Python client awaited each tool inside its WebSocket read loop, so it never read the next request until the current tool finished. Code fanning out with Promise.all took the sum of its calls instead of the slowest one.

Everything above the client was already concurrent — the Deno op is #[op2(async)], the registry holds no lock across its await, and the server gives each call its own request id and response channel. The requests left the server together and queued at the client.

Fix

pctx-py/src/pctx_client/_websocket_client.py:

  • Each ExecuteToolRequest runs in its own task, so the read loop stays free. Tasks are held in a set (asyncio keeps only weak references) and cancelled as a group on disconnect.
  • Sync tools run via asyncio.to_thread — calling one inline blocked the event loop for its whole duration, stalling every other in-flight call behind it.

4 × 2s tool under Promise.all: 8.12s → 2.11s, all four starting at t=0.14.

Tests

test_concurrent_tool_calls_run_in_parallel and test_concurrent_sync_tool_calls_run_in_parallel assert on observed overlap (peak in-flight) rather than wall time, so they fail on serialization rather than on a slow machine. Both verified to fail on the unfixed client (peaked at 1). Full suite: 150 passed.

Notes

  • Behaviour change: sync tool bodies now execute off the main thread, so anything they share must be thread-safe. Async tools are unaffected.
  • Async dispatch is now unbounded. Options for adding concurrency controls — and why a naive semaphore would cause timeouts, since the server's per-tool clock starts at dispatch — are written up in plans/tool-call-concurrency-controls.md. Deliberately out of scope here.
  • Also carries an earlier commit reducing info-level log verbosity in the Rust crates.

🤖 Generated with Claude Code

@michaelmunson-pctx michaelmunson-pctx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM!

@eliasposen
Elias Posen (eliasposen) merged commit 9710d8a into main Jul 27, 2026
15 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