fix(workflow): classify oversized PR diffs safely (#7695) - #7747
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 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. Comment |
ll7
left a comment
There was a problem hiding this comment.
Exact-head review
- Head reviewed:
9bbf1024fad60f9c66371ec59adb36f442f13313 - Base reviewed:
2d221f7c3af34e9ae9db4a563c576fb6e919771e - Decision: acceptable for merge after all hosted checks pass and the guarded merge-ready label is admitted.
Scope and correctness
The change preserves the fast gh pr diff --name-only path and adds a strict, bounded GitHub REST pagination fallback for oversized diffs. It validates response shape, requires a terminal short/empty page, and fails closed on malformed or exhausted enumeration. The CLI now emits structured JSON for unknown/error outcomes instead of silently omitting a result. The focused contract tests cover the fast path, 406 fallback, multi-page terminal behavior, malformed responses, and structured error output.
Evidence
- Focused new contract tests: 5 passed.
- Related base-sensitive gate tests: 16 passed.
- Selected contract tests: 6 passed.
- Final readiness on the exact base/head: passed, including the full core lane, CUDA preflight, docstring/broad-exception ratchets, and clean-tree stamp.
- Live diagnostic reproduced GitHub HTTP 406 for PR #7469 and verified the REST fallback returned a complete non-success classification without claiming base-sensitive files.
This is support tooling only; it makes no benchmark, planner, or paper-facing claim. Merge remains contingent on hosted checks and the repository's guarded merge procedure.
🔍 PR Contract Check SummaryOverall Status: 🟢 PASSED
|
ll7
left a comment
There was a problem hiding this comment.
Exact-head self-review
- Reviewed head:
9bbf1024fad60f9c66371ec59adb36f442f13313 - Reviewed base:
2d221f7c3af34e9ae9db4a563c576fb6e919771e - Intended design: #7695 preserves the fast
gh pr diff --name-onlypath and adds a strict, bounded GitHub REST pagination fallback for oversized diffs. Malformed or incomplete enumeration remains unknown/fail-closed, and the CLI emits structured JSON for unknown/error outcomes. - Findings: none within the issue scope.
- Focused proof:
5 passedfor the new changed-file enumeration contract tests;16 passedfor related base-sensitive gate tests;6 passedfor selected contract tests. - Additional proof: exact
pr_ready_check.shpassed at the reviewed head, including the full core lane, CUDA preflight, docstring/broad-exception ratchets, and clean-tree stamp. A live diagnostic reproduced GitHub HTTP 406 for PR #7469 and verified the REST fallback returned a complete non-success classification without claiming base-sensitive files. - This is support/tooling work with no research, benchmark, planner, or paper-facing claim and no domain review required.
single-account internal review waiver: no distinct implementation-review identity is available; exact-head self-review, independent hosted checks, and the guarded merge receipt are required instead.
base-policy: ordinary-cas @ 9bbf1024fad60f9c66371ec59adb36f442f13313
pr-metadata: reconciled @ 6b769a84825ca85fe87ae840ca701591eb7834a7783de61903f93ca2ad1d35ff
gate-verdict: accepted @ 9bbf1024fad60f9c66371ec59adb36f442f13313
Summary
Make the base-sensitive gate recover complete changed-file inventories from GitHub's paginated pull-request files API when
gh pr diff --name-onlyis rejected for an oversized diff, while preserving fail-closed behavior for incomplete or malformed data.Linked Issues
Stack / Dependency
origin/main2d221f7c3af34e9ae9db4a563c576fb6e919771eWhat Changed
gh pr diff --name-onlypath for ordinary small diffs.pulls/<number>/filesas the fallback source.docs/dev_guide.md.Why It Matters
PullRequest.diff_too_largeand currently prevents the gate from producing a useful JSON result.Research Result Guidance
gh pr diff --name-onlyfast path.Domain-Aware Approval
Falsification / Non-Transfer Check
Next Empirical Action
Validation / Proof
uv run ruff format --check scripts/dev/check_base_sensitive_gates.py tests/dev/test_base_sensitive_gate_contract.py— passuv run ruff check scripts/dev/check_base_sensitive_gates.py tests/dev/test_base_sensitive_gate_contract.py— passgh pr diff 7469 --repo ll7/robot_sf_ll7 --name-only— expected HTTP 406 reproducedpython scripts/dev/check_base_sensitive_gates.py --pr 7469 --repo ll7/robot_sf_ll7 --json— structured ordinary classification from REST fallbackRisks / Rollout
unknownrather than admitting a PR.Docs / Provenance
docs/dev_guide.mdmerge-race prevention section.scripts/dev/merge_queue_gate.py.Downstream Propagation
Follow-Up Issues
Reviewer Notes