docs(pr-review): describe new manifest-based diff payload accurately#3239
docs(pr-review): describe new manifest-based diff payload accurately#3239simonrosenberg wants to merge 2 commits into
Conversation
Replace the obsolete "10K per file / 100K total" claim — there was no per-file cap in the code. The PR review plugin in OpenHands/extensions now uses a manifest + per-file budget (OpenHands/extensions#233). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste - Clean documentation fix that corrects misleading information.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
Documentation-only change with no code modifications. Fixes incorrect claims about per-file truncation limits and adds clarity about the manifest-based diff payload structure.
VERDICT:
✅ Worth merging: Accurate documentation that fixes a real documentation bug.
KEY INSIGHT:
This corrects a documentation hallucination - the old "10,000 characters per file" claim never existed in the code, and the new description accurately reflects the manifest + per-file budget implementation from OpenHands/extensions#234.
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
✅ QA Report: PASS
Documentation accurately reflects the actual implementation. Verified the old docs were incorrect (no 10K per-file cap ever existed) and the new docs correctly document the manifest-based system introduced in OpenHands/extensions#233.
Does this PR achieve its stated goal?
Yes. The PR set out to fix incorrect documentation that claimed a "10,000 characters per file" truncation limit that never existed in the code. I verified:
- Old docs were wrong: Checked the git history of
OpenHands/extensionsand confirmed there was never a 10,000 per-file cap—only a 100,000 total limit that did simple character slicing. - New docs are accurate: Cross-referenced the claimed values (
MAX_PER_FILE_PATCH = 12,000,MAX_TOTAL_DIFF = 150,000) against the actual implementation in OpenHands/extensions commit 8c8b046 and they match exactly. - Manifest system exists: Confirmed the implementation includes
format_files_manifest()that creates a "Files Changed" section listing all files, with per-file patch abbreviation markers as documented.
| Phase | Result |
|---|---|
| Environment Setup | ✅ Already on PR branch, no build needed |
| CI Status | ✅ All critical checks passing (21 successful, 9 pending build jobs) |
| Functional Verification | ✅ Documentation accuracy verified against actual code |
Functional Verification
Test 1: Verify old documentation was incorrect
Step 1 — Check what the old README claimed:
Ran git show eebb2e95 to view the diff:
-- **Complete Diff Upfront**: The agent receives the complete git diff in the initial message for efficient review
- - Large file diffs are automatically truncated to 10,000 characters per file
- - Total diff is capped at 100,000 charactersThe old README claimed a "10,000 characters per file" limit.
Step 2 — Check the actual implementation history:
Cloned OpenHands/extensions and checked the code before the manifest PR (commit 8c8b046~1):
cd /tmp/extensions && git show 8c8b046~1:plugins/pr-review/scripts/agent_script.py | grep -A5 "MAX_"Output:
# Maximum total diff size
MAX_TOTAL_DIFF = 100000
def truncate_text(diff_text: str, max_total: int = MAX_TOTAL_DIFF) -> str:
if len(diff_text) <= max_total:
return diff_text
return diff_text[:max_total] + "..."Interpretation: There was never a per-file limit—only a 100K total limit with simple character slicing. The old README's "10,000 characters per file" claim was incorrect.
Test 2: Verify new documentation accurately describes the implementation
Step 1 — Check what the new README claims:
Viewed the updated README (lines 30-33):
- Each file's patch is capped at `MAX_PER_FILE_PATCH` (12,000 chars)
- The combined patch block is capped at `MAX_TOTAL_DIFF` (150,000 chars)
- files past the cap appear in the manifest but their patch is replaced with a `[patch omitted: ...]` markerStep 2 — Check the actual implementation:
Checked the new code in OpenHands/extensions commit 8c8b046:
cd /tmp/extensions && git show 8c8b046 | grep -A5 "MAX_PER_FILE_PATCH\|MAX_TOTAL_DIFF"Output:
# Maximum total size of all patches combined in the prompt
MAX_TOTAL_DIFF = 150000
# Maximum size for a single file's patch body. Prevents a single huge file
# (e.g. a regenerated lockfile) from starving smaller files' patches.
MAX_PER_FILE_PATCH = 12000Interpretation: The values match exactly—MAX_PER_FILE_PATCH = 12000 and MAX_TOTAL_DIFF = 150000. ✅
Test 3: Verify manifest system exists as documented
Step 1 — Check the manifest claim:
The README claims: "preceded by a Files Changed manifest listing every file in the PR"
Step 2 — Check the implementation:
Searched for manifest-related code:
cd /tmp/extensions && git show 8c8b046 | grep -A10 "format_files_manifest"Output:
def format_files_manifest(files: list[dict[str, Any]]) -> str:
"""Build the 'Files Changed' manifest shown before the patch block.
Invariant: every file in `files` appears exactly once in the output,
regardless of patch size or budget.
"""
header = (
f"## Files Changed ({len(files)} files, "
f"+{total_additions} / -{total_deletions})\n\n"
"All files in the PR are listed here. If a file's patch is missing "
"or abbreviated in the Patches section below, read the file from the "
"workspace (it is checked out) rather than treating it as absent.\n"
)Interpretation: The manifest system exists as documented. The code creates a "Files Changed" section that lists every file, with guidance to read from workspace when patches are abbreviated. ✅
Issues Found
None. This is a straightforward documentation fix that accurately corrects the README to match the actual implementation.
Minor note: The PR description references "OpenHands/extensions#234" but the actual PR number is #233 (visible in the commit message of 8c8b046). This doesn't affect the accuracy of the documentation change itself.
…r file) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 participants
Summary
The example README claimed the PR review action truncates "10,000 characters per file" — but no per-file cap ever existed in the actual code (only a global 100K byte cap). OpenHands/extensions#234 introduces a real per-file budget plus a manifest; this PR updates the docs to match.
Fixes the docs side of the bot-hallucination bug reported in OpenHands/extensions#233 (transferred from this repo's #3238).
Test plan
🤖 Generated with Claude Code
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:b62b934-pythonRun
All tags pushed for this build
About Multi-Architecture Support
b62b934-python) is a multi-arch manifest supporting both amd64 and arm64b62b934-python-amd64) are also available if needed