Add paged or per-file diff #124

Closed
opened 2026-05-08 11:47:23 +00:00 by fiesh · 2 comments
fiesh commented 2026-05-08 11:47:23 +00:00 (Migrated from codeberg.org)

Currently, it seems that get_pull_request_diff (which seems to be lacking from README.md) is the only means for the agent to request a diff between source and target branch. However, since it supports no paging, this can easily lead to a result that blows up the model's context window. We thus do a manual cut-off for the return value, but then the agent only gets part of the diff and has no means of getting the rest.

It would probably be best to give the agent the means to query the files that changed (maybe paged, but that's less crucial here) as well as the possibility to get the diff for a specific file (definitely paged). This would allow it to not look at every diff but be more selective.

Currently, it seems that `get_pull_request_diff` (which seems to be lacking from `README.md`) is the only means for the agent to request a diff between source and target branch. However, since it supports no paging, this can easily lead to a result that blows up the model's context window. We thus do a manual cut-off for the return value, but then the agent only gets part of the diff and has no means of getting the rest. It would probably be best to give the agent the means to query the files that changed (maybe paged, but that's less crucial here) as well as the possibility to get the diff for a specific file (definitely paged). This would allow it to not look at every diff but be more selective.
fiesh commented 2026-05-08 11:54:46 +00:00 (Migrated from codeberg.org)

Actually, come to think of it, also list_pull_request_files and list_pull_reviews are susceptible to having too long an output. I restricted the length to 16kB, and they easily break this limit. So maybe a uniform approach to all functions might be more sensible.

EDIT: And of course get_file_content is a heavy hitter as well. Here specifying a line-based range might be best, like most tools do for agent interaction.

Actually, come to think of it, also `list_pull_request_files` and `list_pull_reviews` are susceptible to having too long an output. I restricted the length to 16kB, and they easily break this limit. So maybe a uniform approach to all functions might be more sensible. EDIT: And of course `get_file_content` is a heavy hitter as well. Here specifying a line-based range might be best, like most tools do for agent interaction.
goern commented 2026-05-09 20:55:27 +00:00 (Migrated from codeberg.org)

Thanks for the report — confirmed valid against current main (dec5be5). Triage notes:

What's already paged

  • list_pull_request_files and list_pull_reviews already accept page / limit (defaults 1 / 50). If your 16 kB envelope cap clips them, the fix on the agent side is to request a smaller limit. Server-side defaults could probably be tightened, but the knobs exist.

What's not paged (the real gaps)

  1. get_pull_request_diff — single blob, no paging, no per-file filter. Confirmed in operation/pull/pull.go:386-395. This is the worst offender for context blowups.
  2. get_file_content — no line-range params. Confirmed in operation/repo/file.go:25-33. A large file is returned in full.
  3. README gapget_pull_request_diff and list_pull_request_files are not in the tool table. Will fix alongside.

Proposed direction

  • Per-file diff: add an optional file_path parameter to get_pull_request_diff so the agent can fetch one file at a time. Implementable client-side by parsing the unified diff and slicing on diff --git boundaries if the SDK has no native support. Pair this with list_pull_request_files so the agent has a cheap file index first.
  • Diff paging: secondary to per-file. If still needed, add max_bytes / offset byte-range params on the raw diff. Per-file is the cleaner abstraction.
  • get_file_content line range: add start_line / end_line params; slice server-side from GetFile output. Cheap to implement, no SDK change needed.
  • Uniform approach: agree with the EDIT — every tool that can return unbounded text should expose a bound (line range, byte range, or page+limit). I will treat this issue as the umbrella for that policy and split out per-tool follow-ups as needed.

Marking Kind/Enhancement + Priority/Medium. Will scope an implementation PR; per-file diff first since it gives the biggest win for the smallest API surface.

Thanks for the report — confirmed valid against current `main` (`dec5be5`). Triage notes: ## What's already paged - `list_pull_request_files` and `list_pull_reviews` already accept `page` / `limit` (defaults 1 / 50). If your 16 kB envelope cap clips them, the fix on the agent side is to request a smaller `limit`. Server-side defaults could probably be tightened, but the knobs exist. ## What's not paged (the real gaps) 1. **`get_pull_request_diff`** — single blob, no paging, no per-file filter. Confirmed in `operation/pull/pull.go:386-395`. This is the worst offender for context blowups. 2. **`get_file_content`** — no line-range params. Confirmed in `operation/repo/file.go:25-33`. A large file is returned in full. 3. **README gap** — `get_pull_request_diff` and `list_pull_request_files` are not in the tool table. Will fix alongside. ## Proposed direction - **Per-file diff:** add an optional `file_path` parameter to `get_pull_request_diff` so the agent can fetch one file at a time. Implementable client-side by parsing the unified diff and slicing on `diff --git` boundaries if the SDK has no native support. Pair this with `list_pull_request_files` so the agent has a cheap file index first. - **Diff paging:** secondary to per-file. If still needed, add `max_bytes` / `offset` byte-range params on the raw diff. Per-file is the cleaner abstraction. - **`get_file_content` line range:** add `start_line` / `end_line` params; slice server-side from `GetFile` output. Cheap to implement, no SDK change needed. - **Uniform approach:** agree with the EDIT — every tool that can return unbounded text should expose a bound (line range, byte range, or page+limit). I will treat this issue as the umbrella for that policy and split out per-tool follow-ups as needed. Marking `Kind/Enhancement` + `Priority/Medium`. Will scope an implementation PR; per-file diff first since it gives the biggest win for the smallest API surface.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
agentic-forges/forgejo-mcp#124
No description provided.