Skip to content

fix(strands-command): return full PR diffs and offload large tool results#63

Open
yonib05 wants to merge 6 commits into
strands-agents:mainfrom
yonib05:fix/pr-diff-truncation
Open

fix(strands-command): return full PR diffs and offload large tool results#63
yonib05 wants to merge 6 commits into
strands-agents:mainfrom
yonib05:fix/pr-diff-truncation

Conversation

@yonib05

@yonib05 yonib05 commented Jun 10, 2026

Copy link
Copy Markdown
Member

Problem

get_pr_files had two ways of hiding code from the reviewer:

  1. It truncated each changed file's diff to the first 50 lines (# Limit diff size to avoid overwhelming output), so issues below line 50 were invisible.
  2. It read only the first page of GitHub's PR-files endpoint (max 100 items/page), so on a large PR, changed files past the first page were silently never reviewed.

Change

  • get_pr_files now returns each file's full, untruncated diff, and follows pagination (new _github_get_all_pages helper, which follows Link rel="next" with a page-count safety cap) so every changed file is included.
  • Large tool results are managed centrally by the Strands SDK ContextOffloader plugin (agent_runner.py). When a result exceeds the token threshold, the plugin offloads it to storage, replaces it in-context with a preview + reference, and registers a retrieve_offloaded_content tool so the agent pulls the full content on demand.

This handles oversized results generically for every tool (diffs, file reads, shell output), not just get_pr_files, and removes ~60 lines of bespoke per-call budgeting/head-slicing logic.

Storage backend

Uses S3Storage, reusing the bucket already configured for session management. The offloaded result is replaced in the conversation by a storage reference that the session manager persists; a resumed session in a later process must still resolve that reference, which in-memory storage could not guarantee.

Tests

tests/test_github_tools.py (get_pr_files):

  • full diff returned untruncated (guards the original 50-line-cap regression),
  • every changed file inlined in API order,
  • pagination is followed across pages (guards against reviewing only the first page),
  • pagination is bounded (no unbounded follow loop),
  • binary / no-patch files handled,
  • request errors surfaced as-is.

Full unit suite: 38 passed.

yonib05 added 3 commits June 10, 2026 12:57
get_pr_files capped every file's patch at 50 lines, so the reviewer
silently lost the tail of any larger change and reasoned about code it
never saw. Replace the per-file line cap with a total character budget:
inline full patches until the budget is reached, then list remaining
files for on-demand fetch instead of truncating mid-file.
…ping it

A file whose patch alone exceeds the budget previously produced no inline
diff at all, leaving the reviewer dependent on fetching it. Inline a
line-trimmed head slice when enough budget remains so there is always some
inline signal, and note the omitted remainder is fetchable. Adds tests for
the partial-head and mixed inline/overflow cases.
Guard the partial-head branch so a patch starting with a newline (whose
line-boundary trim leaves an empty head) is fully deferred instead of
emitting an empty "Diff (head only):" block. Add tests covering the
full-deferral branch, the exact budget boundary, and the empty-head edge,
merge the two near-duplicate over-budget tests, and note the accepted
file-order dependence of the diff budget.
if head:
used += len(head)
omitted = len(patch) - len(head)
overflow.append(f"{filename} (+{additions} -{deletions}, partially shown)")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we write the overflow to disk and let the agent know where it is?


@tool
@log_inputs
def get_pr_files(pr_number: int, repo: str | None = None) -> str:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is PR output?

Can we just leverage the Strands Tool Offloader here and let it manage the truncation?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good call will swap it should simplify alot

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in 52c3ecf. Swapped the bespoke budgeting for the SDK's ContextOffloader plugin (wired in agent_runner.py with InMemoryStorage). get_pr_files now just returns the full diff, and the plugin offloads any oversized tool result — not only diffs but file reads and shell output too — registering a retrieval tool so the agent fetches full content on demand. Net: ~60 lines of truncation logic + its constants removed, and truncation is handled generically for every tool.

yonib05 added 2 commits June 22, 2026 09:21
…oader

Replace the bespoke per-call diff budgeting in get_pr_files with the SDK's
ContextOffloader plugin. get_pr_files now returns the full untruncated diff;
the plugin offloads any oversized tool result (diffs, file reads, shell
output) to storage and registers a retrieval tool so the agent fetches the
full content on demand. This handles truncation generically for every tool
rather than just this one, per review feedback.
@yonib05 yonib05 changed the title fix(strands-command): inline full PR diffs within a total budget fix(strands-command): return full PR diffs and offload large tool results Jun 22, 2026
…reviewed

GitHub list endpoints cap at 100 items per page, so reading a single response
silently dropped changed files past the first page on large PRs -- the reviewer
would never see them. Add a paginated GET helper that follows Link rel="next"
(bounded by a page cap) and use it for the PR files list.
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