Skip to content

avoid emitting partial tool call locations - #90

Open
davidhewitt wants to merge 1 commit into
svkozak:mainfrom
davidhewitt:partial-tool-locations
Open

avoid emitting partial tool call locations#90
davidhewitt wants to merge 1 commit into
svkozak:mainfrom
davidhewitt:partial-tool-locations

Conversation

@davidhewitt

Copy link
Copy Markdown

Using pi-acp I experienced some major issues with Zed getting extremely memory hungry and installing file watchers for pretty much my whole disk. Eventually I tracked the issue down to Zed logs like this:

2026-07-20T13:42:48+01:00 ERROR [crates/acp_thread/src/acp_thread.rs:1138] reading bytes of the file "/Users"
2026-07-20T13:42:48+01:00 ERROR [crates/acp_thread/src/acp_thread.rs:1138] reading bytes of the file "/Users/david"
2026-07-20T13:42:48+01:00 ERROR [crates/acp_thread/src/acp_thread.rs:1138] reading bytes of the file "/Users/david/Dev"
2026-07-20T13:42:48+01:00 ERROR [crates/acp_thread/src/acp_thread.rs:1138] reading bytes of the file "/Users/david/Dev"
2026-07-20T13:42:48+01:00 ERROR [crates/acp_thread/src/acp_thread.rs:1138] reading bytes of the file "/Users/david"
2026-07-20T13:42:48+01:00 ERROR [crates/acp_thread/src/acp_thread.rs:1138] reading bytes of the file "/"

from what I can tell from debugging, this was happening when streaming tool call updates from pi-acp emitted partial locations before the LLM had finished emitting the tool path. Zed would receive the tool call update and start building worktrees related to the partial path for file analysis.

You could argue it's a Zed (or pi) bug, however it seems unhelpful to emit these partial locations so this is the patch I applied in pi-acp to avoid the bug. Happy to move the discussion elsewhere if you think better.

@nertzy

nertzy commented Jul 31, 2026

Copy link
Copy Markdown

Corroborating this from Zed, where the same bug has a second, very visible
symptom: each truncated path becomes its own editor tab, with names like gr,
code, p, pi-rp, comm.

Zed calls project.open_local_buffer() for every entry in locations on every
tool_call_update (crates/acp_thread/src/acp_thread.rs), then builds a new
follower view per distinct buffer — so every streamed prefix opens a tab, and
some of them are directories. Measured on a single read of one file: 13
locations entries, 9 distinct paths.

/Users/
/Users/gr
/Users/grant/
/Users/grant/code
/Users/grant/code/p
/Users/grant/code/pi-acp/
/Users/grant/code/pi-acp/src/pi-rp
/Users/grant/code/pi-acp/src/pi-rpc/comm
/Users/grant/code/pi-acp/src/pi-rpc/command.ts   <- the only real one

This patch fixes it. Verified on top of simonsmh/pi-acp at 0.0.34 (merges
clean): npm test 102/102, npm run typecheck clean, and reverting just the
one-line session.ts change makes the updated test fail with
actual: [{ path: '/' }].

Worth noting for anyone landing here: the partial values are not malformed, just
short. pi-ai's anthropic-messages.js runs parseStreamingJson over the
accumulated partialJson on every partial_json delta, so arguments.path
is a syntactically valid string that happens to be a prefix — indistinguishable
from a finished path at the consumer. Gating on toolcall_end is the only
reliable signal.

@vlad-moyai

vlad-moyai commented Aug 4, 2026

Copy link
Copy Markdown

Corroborating this on pi-acp 0.0.33 with Zed on macOS. I independently hit the same whole-disk worktree behavior while using Pi through ACP.

Observed impact

Zed's reliability diagnostics reached:

  • 75.8 million total worktree entries
  • 49.6 million entries in a synthetic worktree rooted at /
  • about 48 GiB Zed physical footprint, mostly compressed/swapped
  • about 41.7 GiB system swap in use
  • basedpyright opened / directly and reached roughly 1.5-1.7 GiB while consuming more than one CPU core

The Pi and pi-acp processes themselves remained small (roughly 25-100 MiB), which initially made the source of the behavior non-obvious.

Root cause confirmation

toToolCallLocations() turns any current path/file_path argument into an absolute ACP location. During toolcall_start and toolcall_delta, a streamed absolute path is already valid JSON but is only a prefix. Zed receives locations such as /, /Users, and successively longer prefixes, opens them as local buffers/worktrees, and starts filesystem scanning and language servers for them.

This matches both the logs in this PR and the implementation path in PiAcpSession.handlePiEvent().

Verification

I replayed commit f3a1251 onto current upstream main (pi-acp 0.0.33). It applied cleanly and passed:

  • npm test: 95/95
  • npm run typecheck
  • npm run lint

The regression test correctly proves that /, /tmp, and the other streamed prefixes carry no locations, while the completed /tmp/test.txt path is emitted only on toolcall_end.

This PR addresses the memory trigger and should prevent the synthetic / worktree from being created by partial path streaming.

As optional defense in depth, it may also be worth considering either:

  1. only emitting tool locations contained by the ACP session cwd, or
  2. adding a setting to suppress external tool locations.

That would prevent completed reads of global documentation/configuration paths from creating additional external Zed worktrees, while this PR remains the essential fix for the whole-disk / case.

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.

3 participants