fix(review): inspect complete file lists for large pull requests - #984
fix(review): inspect complete file lists for large pull requests#984vincentkoc wants to merge 2 commits into
Conversation
|
Codex review: needs changes before merge. Reviewed August 2, 2026, 5:03 PM ET / 21:03 UTC. ClawSweeper reviewWhat this changesAdds complete changed-file hydration for truncated OpenClaw pull requests so deterministic configuration and stored-data classifiers can inspect files omitted from the bounded model context. Merge readiness⛔ Blocked by patch quality or review findings - 6 items remain Keep this PR open: the complete-file classification improvement remains useful, but the branch still implements it only in the pre-refactor monolith and cannot reach the current review pipeline. The author is a repository member, so this workflow must not auto-close it; the narrow port to current modules is the actionable next step. Priority: P2 Review scores
Verification
How this fits togetherClawSweeper hydrates GitHub pull-request metadata into a bounded review context, then runs deterministic classifiers before Codex reviews the item. Complete file-list data must stay outside prompts and cache identity while still reaching the active configuration and stored-data classifiers. flowchart LR
A[GitHub pull request] --> B[Bounded file context]
B --> C[Complete file sidecar]
C --> D[Configuration classifier]
C --> E[Stored-data classifier]
D --> F[Merge-risk decision]
E --> F
B --> G[Codex review prompt]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Rebase and transplant the complete, fail-closed pagination sidecar into the current item-context and change-detection modules, then prove both complete classification and bounded prompt/cache behavior on the rebased head. Do we have a high-confidence way to reproduce the issue? Yes—current source shows truncated pull files are still bounded in the active item-context path and the active classifiers only read that bounded list. The PR body also supplies a live 118-file pagination transcript, but the proposed implementation must first be moved onto current main's path. Is this the best way to solve the issue? No—the complete-sidecar approach is appropriate, but this branch applies it in a removed monolithic integration path. Porting the same fail-closed behavior into the current item-context and change-detection modules is the narrower maintainable solution. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against de31c9959070. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (16 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Maintainer decision for head 4a434e7: accept the bounded API budget of at most 30 paginated file-list reads plus one refreshed pull snapshot at the 3,000-file ceiling. Keep stable-snapshot validation, page reuse, OpenClaw-only scope, and fail-closed behavior above the ceiling. |
|
@clawsweeper re-review |
Related: #983
What Problem This Solves
Fixes an issue where large OpenClaw pull requests could hide config or persistent-data files in the omitted middle of the bounded 80-file review context, leaving deterministic merge-risk classification incomplete.
Why This Change Was Made
For truncated openclaw/openclaw pull requests only, ClawSweeper now fetches the complete file list into a compact non-enumerable sidecar. It reuses every bounded-context page, requires the exact count and unique filenames, verifies that base and head SHAs stayed stable across pagination, and fails closed above GitHub's 3,000-file response limit. Deterministic config and data-model classifiers consume that sidecar, while prompts, ledgers, and cache digests keep the existing bounded context.
User Impact
Large OpenClaw pull requests receive complete deterministic surface classification without increasing model prompt size or changing review cache identity. Fetch or count failures retain the existing unknown-truncated fail-closed marker.
Evidence
git diff --check: clean.List pull requests filescontract caps responses at 3,000 files: https://docs.github.com/en/rest/pulls/pulls#list-pull-requests-files. Totals above that limit fail closed before complete pagination.origin/mainrelease-owned exception applies to OpenClaw's changelog during foreign-target PR work, not this repository's own operational release notes.Real Behavior Proof
[1,2]exactly once rather than refetching both pages. The semantic false-positive cleanup is independently reviewable in fix(review): avoid false persistent data-model findings #983.unknown-truncated-pull-files.Live Pagination And Merge-Order Proof - July 31, 2026
Redacted live pagination transcript for OpenClaw PR 115869:
The transcript was produced with authenticated read-only
ghx apirequests to:Both exact merge orders were tested from base
16f01b29508632ab3a904087191e39b20968ab68:983 -> 984: clean cherry-picks; 66/66 focused tests passed.984 -> 983: clean cherry-picks; 66/66 focused tests passed.609478921e5b04e2b1844001bfe81ca713f4f099, with 6 files changed, 560 insertions, and 25 deletions.git diff --check 16f01b29508632ab3a904087191e39b20968ab68..HEADpassed in both worktrees.{ "config": { "change": false, "keys": [] }, "data": { "change": false, "surfaces": [] } }over the live 118-file payload.unknown-truncated-pull-filesfallback.Commands used direct
tscplusnode --test test/context.test.ts test/pr-surface-policy.test.ts test/review-prompt-context.test.ts; no dependency reconciliation occurred in the proof worktrees.Maintainer Decision - July 31, 2026
Approved: accept the bounded pagination cost. For a truncated OpenClaw pull request at GitHub's 3,000-file ceiling, the complete classifier path performs at most 30 file-list page reads plus one refreshed pull snapshot. Already-read bounded-context pages are reused, non-OpenClaw targets do not enter this path, and totals above 3,000 or any count, uniqueness, fetch, or source-snapshot mismatch fail closed to the existing unknown-truncated marker. This trade is acceptable for deterministic config and stored-data classification.