fix(code): count distinct targets in grouped tool summaries - #5409
Merged
Conversation
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
github-actions
Bot
added
dcode
Mason Daugherty (mdrxy)
marked this pull request as ready for review
August 11, 2026 05:50
Note that the past verb in `_TOOL_SUMMARY_PHRASES["read"]` is unreachable, record why the noun phrasing is past-only, and document that digit-leading segments pass through `_join_segments` lowercasing unchanged.
The carve-out comment in `_summary_segment` already records why settled reads skip the table, so restating the consequence beside the table only adds a second place to keep in sync — and its diff-eligibility claim would silently rot if `read` were ever added to `_DIFF_HEADER_CATEGORIES`.
The group summary counted calls while phrasing the count as nouns, so one file read twice rendered as "Reading 2 files" / "Read 2 files", overstating how much of the tree the step touched. The same defect hit every category whose noun is an object: "Edited 2 files" for one file edited twice, "Fetched 2 URLs" for one URL fetched twice. Collapse repeat calls on a target before counting, keyed on the arg that names the object (`file_path`/`path`, or `url`). Categories that count attempts rather than objects — shell, js, task, search — are left alone, since running one command twice is genuinely two runs. `ls` is excluded because a bare `ls()` means the backend's working directory, which `execute` can change mid-step. Deduping happens per tense bucket, so a file whose second read is still in flight is still reported as being read. Targets that cannot be identified (missing or unparsed args) are always counted, so the fallback is the old behavior rather than an undercount. This supersedes the "N file reads" phrasing, which only covered reads, only in the past tense, and broke the parallelism of mixed lines; summaries read "Edited 1 file, wrote 1 file, read 1 file" again.
Mason Daugherty (mdrxy)
changed the title
github-actions
Bot
added
fix
![open-swe[bot]](https://avatars.githubusercontent.com/in/1395372?s=60&v=4){kind=small}
Counting distinct targets fixed the inflated file counts but hid churn on
mutating tools: three edits of one file read as "Edited 1 file", when each
edit is an event that changed the tree and owns its own diff. Reads are
different -- a repeat read is usually pagination, where the count is noise.
Report both numbers for mutations, with the operation count trailing in
parentheses only when it differs from the target count:
Edited 1 file (3 edits) 3 edits of one file
Edited 2 files (3 edits) both numbers survive when they disagree
Edited 2 files no repeats, no parenthetical
Read 1 file 3 reads of one file, collapsed silently
Applies to edit and write. `delete` is excluded: a second successful delete
of one path cannot happen, since a failed retry is evicted from the group
before it is summarized.
The summary API now takes `(name, args)` pairs rather than bare tool names.
Passing names alone is what made the original overcount possible -- the
phrasing claims nouns, which cannot be counted without the argument naming
each call's target -- so the impoverished signature is gone rather than
worked around. `_tally_categories` returns distinct targets and total calls
together, and the live-line cache key pairs each name with its target so a
second read of a new file still invalidates.
`normpath` was applied to every summary target, including the `fetch`
category's URL. Path rules are wrong for a URL, and each rewrite it makes
merges two addresses a server can answer differently:
/a//b and /a/b -> https:/x/a/b
/a/ and /a -> https:/x/a
/a/../b and /b -> https:/x/b
Each collision reports "Fetched 1 URL" for two distinct fetches, an
undercount -- the one direction this code is supposed to never move in, since
an unrecognizable target is deliberately counted rather than merged.
Restrict `normpath` to the categories whose target is a genuine filesystem
path and compare anything else exactly. Naming those categories in a
frozenset rather than testing for `fetch` keeps a future non-path category
from silently inheriting path semantics.
…etes The grouped tool-summary line identified a call's target with `os.path.normpath`, which resolves to Windows rules on a Windows client. The paths belong to the backend, so `a\b` — a legal POSIX file name — merged with `a/b` and the line reported one file where two were read. Normalize with `posixpath` on every platform instead. `normpath` also resolves `..` lexically, so `d/../a.py` matched `a.py` even when `d` is a symbolic link to somewhere else. A `..` segment now turns the normalization off: a missed match only restores the old count, while a wrong match hides work. `delete` gains a repeat noun. A group spans a whole step, so `delete a.py`, `write_file a.py`, `delete a.py` is two real deletions of one path, and the line rendered `Deleted 1 file` — the second deletion was invisible. It now reads `Deleted 1 file (2 deletions)`, like the other mutating categories. The comments claiming otherwise are corrected. `ls` was said to be excluded because a bare `ls()` names the backend's working directory, which `execute` can change mid-step; `LsSchema.path` is required and `execute` cannot change that directory. A listing is excluded because it is a snapshot, not a durable object. The `delete` omission was said to be impossible rather than merely unusual. Two docstrings describing the summary as built from tool names alone now say names and arguments. `ToolCallMessage.summary_call` reads the arguments without copying them. The group rebuilds its cache key from every member on each spinner tick, and `args` copies the dict on every access, so the 10 Hz path allocated per member per tick to decide whether cached text was still valid. Tests cover the live render path, which could previously be wired to empty arguments with the suite still green, and freeze the invariants that the tables and the shared `seen` set depend on.
Four rationale comments in the grouped-summary path asserted things that are not true of the code they sit on: - `summary_call` claimed the tick path "avoids all per-tick work". The cache key is rebuilt per member per tick, so it does not. The no-copy choice is still right; only the stated cost model was wrong. Also scopes the immutability claim to this widget, since `_args` is the caller's dict and is shared with `textual_adapter` and `message_store`. - `_summary_cache_key` claimed membership is append-only. Eviction removes members. The conclusion survives because eviction clears the cache too, so the comment now names the property that actually carries the argument. - `_summary_segment` said "mutating category", but `fetch` appends a repeat count and is not mutating. Names the gate instead of paraphrasing it. - `_REPEAT_COUNT_NOUNS` said only "read" is absent, true only among categories that name a target. Also lists "todos" among the categories deliberately kept out of `_TOOL_SUMMARY_TARGET_ARGS`, so the set does not read as an oversight, and points the URL comment at `_PATH_TARGET_CATEGORIES` rather than "the type's docstring".
Adds cases for behavior that was live but unpinned, so a plausible cleanup
could break it while the suite stayed green:
- The `("file_path", "path")` fallback scans past a `file_path` that is
present but empty or None. Verified by mutation: rewriting the loop to take
the first key that appears silently disables the fallback and previously
passed every test.
- Trailing-slash and `d/./a.py` spellings collapse to one target. These are
collapses, the direction that must never be wrong, and were the only
collapsing rules with no test.
- An edit that errors is evicted from the repeat count, so two edits of one
file where one fails closes as "Edited 1 file". Both edits are settled and
rendered before the failure so the past-tense line is cached as "(2 edits)"
first; that ordering is what makes the cache reset in `_evict_unfoldable`
load-bearing, and the test fails if the reset is dropped.
Pins `_normalize_path_target` against `validate_path` itself rather than
hand-written expectations. The identity only has to be the middleware's
canonical form, so comparing the two directly is what catches drift, and the
traversal case pins the other half: a path the middleware rejects is returned
verbatim so it can never fold into a valid target's tally.
Also corrects a docstring in `TestCaveatedRowsLeaveTheGroup` still describing
the summary as built from tool names alone.
Summary target identities normalize file paths so two spellings of one file count as one file. The normalizer preserved only `..` traversal, but the filesystem middleware also rejects a leading `~` and a drive prefix like `C:/`. Those spellings were canonicalized anyway, so a read of `~` and a read of the valid virtual path `/~` produced the same identity and the group reported "Read 1 file" for two different targets. Delegate to `validate_path` instead of reimplementing it: the identity is now exactly the string the file tools act on, and any path the middleware refuses is returned verbatim, so no call that could not have run is folded into a valid target's count.
Mason Daugherty (mdrxy)
pushed a commit
that referenced
this pull request
Aug 12, 2026
> [!CAUTION] > Merging this PR will automatically publish to **PyPI** and create a **GitHub release**. For the full release process, see [`.github/RELEASING.md`](https://github.com/langchain-ai/deepagents/blob/main/.github/RELEASING.md). --- _Release notes preview: keep this section in sync with the package `CHANGELOG.md`. Publish reads the merged CHANGELOG via `release.yml`, not this PR description — keep them aligned anyway so the PR stays an accurate historical record for reviewers and anyone returning later._ --- ## [0.1.55](deepagents-code==0.1.54...deepagents-code==0.1.55) (2026-08-12) ### Features - Added a `/context` usage report for inspecting context consumption ([#5407](#5407)). - Added a cache and context status row for at-a-glance session state ([#5408](#5408)). - Added configurable warnings when a session exceeds the configured cost threshold ([#5405](#5405)). - Added support for persisting and reconfiguring ACP sessions ([#5366](#5366)). - Added automatic updates for installed plugins ([#5368](#5368)). - Added a toggle for diff line numbers ([#5427](#5427)). - `Ctrl+S` in `/auto model` now stores `[models].auto_classifier` ([#5313](#5313)). ### Fixes - Restored edit diffs in resumed threads ([#5391](#5391)). - Added a resume hint after crashes ([#5412](#5412)). - Clarified the project hooks trust prompt and stopped prompting for user hooks ([#5426](#5426)). - Cleared dynamic subagents on the next turn ([#5437](#5437)). - Improved grouped tool summaries by counting distinct targets ([#5409](#5409)). - Improved ask-user choice wrapping and selection styling ([#5442](#5442)). - Serialized `dcode` self-upgrades across processes ([#5252](#5252)). - Added warnings for stale dependencies in editable installs ([#5386](#5386)). - Hid incomplete extras from version output ([#5352](#5352)). - Removed the optional-provider startup tip ([#5421](#5421)). - Removed the “Message restored to input” toast ([#5253](#5253)). _End release notes preview._ --- > [!NOTE] > A **New Contributors** section is appended to the GitHub release notes automatically at publish time (see [Release Pipeline](https://github.com/langchain-ai/deepagents/blob/main/.github/RELEASING.md#release-pipeline), step 2). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: langchain-oss-automated-triage[bot] <248757908+langchain-oss-automated-triage[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The summary line for a step counts the tool calls, but it shows the count with a noun. If the agent reads one file two times, the line shows
Read 2 files. Only one file is correct.a.pytwo timesRead 2 filesRead 1 filea.py, readb.pyRead 2 filesRead 2 filesa.pythree timesEdited 3 filesEdited 1 file (3 edits)a.pytwo times, editb.pyEdited 3 filesEdited 2 files (3 edits)Fetched 2 URLsFetched 1 URL (2 calls)The line now counts the different targets. A target is the file or the URL of a call, from the
file_path,pathorurlargument. The present-tense line uses the same counts, for exampleReading 1 file.Every mutating category (
edit,write,delete) andfetchalso show the number of operations in parentheses. Each mutation writes to the file and has a diff, so the number of files alone is not sufficient: a group spans a whole step, sodelete a.py,write_file a.py,delete a.pyis two real deletions of one path. A repeated fetch of one URL is a deliberate new request, soFetched 1 URLalone would hide the second call. A repeated read keeps one number, because the agent usually reads a large file in parts.Limits
shell,js,taskandsearchcount attempts, not objects. Two runs of one command are two operations.web_searchshows repeated calls with a different phrase, for exampleSearched the web 2 times.lscounts every listing. A listing is a snapshot, not a durable object, so an intervening write can make the second listing of one directory show different contents.Paths are compared in the canonical form the filesystem middleware itself produces.
_normalize_path_targetmirrorsvalidate_path, so two spellings that the middleware resolves to one file count as one file. The normalization is purely lexical and never touches the filesystem.a.pyand/a.pyare one file.a\banda/bare one file.d//a.py,d/./a.pyandd/a.py/are alld/a.py...segment turns the normalization off. The middleware rejects those paths, so there is no canonical form to agree with, and folding one would count a call that could not have run.