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).

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.

1 participant