Skip to content

fix(noema): paginate large pull request diffs - #1493

Draft
seonghobae wants to merge 6 commits into
mainfrom
fix/noema-pr-files-pagination-20260831
Draft

fix(noema): paginate large pull request diffs#1493
seonghobae wants to merge 6 commits into
mainfrom
fix/noema-pr-files-pagination-20260831

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the 300-file-limited pull request diff media request with GitHub List pull request files pagination
  • reconstruct the existing bounded diff context from file patches
  • compare paginated records with GraphQL changedFiles and fail closed on incomplete responses
  • mark unavailable file patches as truncated instead of presenting complete review evidence

Verification

Unblocks the exact current Noema review path for ContextualWisdomLab/LineageWeave#640 without changing reviewer identity, token selection, verdict, or current-head review submission semantics.


Devin Review

Use GitHub's paginated pull-request files API so Noema can review ContextualWisdomLab/LineageWeave#640 beyond the 300-file diff endpoint limit. Reconstruct bounded diff context and fail closed when the returned file count does not match the exact PR metadata.

Signed-off-by: Seongho Bae <me@seonghobae.me>
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

# Conflicts:
#	scripts/ci/noema_review_gate.py
#	tests/test_noema_review_gate.py
devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae enabled auto-merge (squash) August 31, 2026 08:49
Signed-off-by: Seongho Bae <me@seonghobae.me>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

Devin Review

Comment thread scripts/ci/noema_review_gate.py
@opencode-agent
opencode-agent Bot disabled auto-merge August 31, 2026 09:00
Signed-off-by: Seongho Bae <me@seonghobae.me>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Taking the repair of this PR (not its ownership: it stays a draft; no flip). Reason it matters now: the first post-#1944 Noema evidence artifacts show noema-review on #1555 (321 files) dying on gh: Sorry, the diff exceeded the maximum number of files (300) (HTTP 406) — main's fetch_diff still requests pulls/{n} with Accept: application/vnd.github.v3.diff (scripts/ci/noema_review_gate.py:476), which is exactly what this PR replaces. main has moved 468 commits since this branch's base and rewrote the surrounding pipeline (fetch_changed_files with statuses, expected_head re-validation, overlong-line truncation on the raw diff), so this is a re-implementation merge, not a marker resolution: I will keep main's pipeline and signatures and port this PR's essence — build the diff from the paginated Files API (per_page=100 --paginate --slurp), reconstruct diff --git sections with /dev/null for added/removed, changedFiles count check — into main's fetch_diff, then apply main's existing truncation to the reconstructed text. Both test sides are the oracle (this PR's paginated-fetch test; main's overlong-line tests). Non-force push to this branch after the full gate; full writeup in the next comment.

Re-implementation merge: main rewrote the review pipeline (fetch_changed_files
with statuses, expected_head re-validation, overlong-line truncation) while this
branch replaced the 300-file-limited .diff media request with the paginated
Files API. Kept main's pipeline and signatures; ported the paginated
reconstruction into fetch_diff; split main's truncation into _bound_diff and
apply it only when the reconstructed diff is over-long (the auto-merged chimera
would have replaced the last changed line of a short-but-incomplete diff with the
overlong marker). GraphQL query keeps both changedFiles and baseRefOid.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Repaired: main@fe827e13 merged as 79cebeeb (branch left as draft; no flip). A re-implementation merge — main had moved 468 commits and rewrote the pipeline around fetch_diff.

Block by block:

  • fetch_pr GraphQL query keeps both changedFiles (this PR) and baseRefOid (main).
  • fetch_diff(repo, number, expected_files=None): this PR's paginated Files-API reconstruction (pulls/{n}/files?per_page=100 --paginate --slurp, diff --git a/… b/… sections, /dev/null for added/removed, changedFiles count check, unavailable patch → truncated). The .diff media request that fails with HTTP 406 above 300 files is gone.
  • main's truncation moved into a new _bound_diff(diff), applied only when the reconstructed diff is over-long. The auto-merge had produced a chimera that ran the truncation whenever truncated was true — including the short-but-incomplete-patch case — which would have replaced the last changed line of a short diff with the [overlong changed line content omitted] marker.
  • inspect_and_review: main's block (fetch_changed_files with statuses, expected_head re-validation, submit_review(current_pr, …)) calling fetch_diff(repo, number, pr.get("changedFiles")); this PR's fetch_changed_file_paths dropped in favour of main's fetch_changed_files.
  • Tests: this PR's paginated-fetch test kept with its oversized patch as a proper hunk so main's marker semantics apply; main's three truncation cases plus a no-newline case moved to test_bound_diff_keeps_line_boundaries_and_marks_severed_changed_lines; seven fetch_diff monkeypatches take expected_files=None; the two fetch_changed_file_paths patches use fetch_changed_files.

Gate on 79cebeeb with GITHUB_ACTIONS=true: tests/test_noema_review_gate.py 123 passed; full suite 123 passed in 4.19s; coverage report --fail-under=100 → 100% (noema_review_gate.py 0 missed); interrogate → 100%.

Acceptance once this lands: a Noema run for a >300-file PR (#1555, 321 files) gets past Prepare Noema model verdict instead of gh: Sorry, the diff exceeded the maximum number of files (300) (seen in run 33996977334).

@seonghobae seonghobae added bug Something isn't working priority: medium Normal-priority or P2 work status: draft Draft pull request type: bug Defect or incorrect behavior labels Sep 7, 2026 — with ChatGPT Codex Connector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: medium Normal-priority or P2 work status: draft Draft pull request type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant