Skip to content

Review large PRs in complete 272k-token portions - #424

Merged
Vilin97 merged 3 commits into
mainfrom
fix/complete-review-portions
Sep 12, 2026
Merged

Vilin97 merged 3 commits into
mainfrom
fix/complete-review-portions

Conversation

@Vilin97

@Vilin97 Vilin97 commented Sep 12, 2026 •

Copy link
Copy Markdown
Collaborator

Large project reviews currently remove the largest file bodies to fit a 180k-token budget, then cannot approve because their input is incomplete. This change raises the per-call input budget to 272k estimated tokens and reviews every diff character in numbered source portions, followed by an integration review under each rubric.

  • Prefer file/declaration boundaries; explicitly track continuations, ranges, and a SHA-256 coverage manifest. Missing GitHub patches are reconstructed from hash-checked source blobs rather than omitted. A moving PR head aborts source acquisition.
  • Integration receives all portion evidence, findings, and open questions. It can retrieve exact declaration/file excerpts from the original diff. Unresolved obligations stay visible and prevent approval; failed or oversized evidence is never silently dropped.
  • Run at most three source calls concurrently per rubric, cancel queued work after failure, and account for retry work and unknown usage. Keep the existing blocking/advisory verdict semantics and Azure Codex authentication with no paid API fallback.
  • Upload structured review evidence for inspection and disclose coverage for every review mode.

Validation: all 360 Python tests pass; Ruff check/format pass. Complete-source preflight covers PRs #385, #393, #394, #395, #396, and #420 within the 272k per-call ceiling. A live Azure faithfulness review of #420, forced into three smaller portions to exercise integration, passed. The repeated live Azure test passed (3 portions, 4 model calls). Regression tests also ensure findings and unanswered source requests survive every integration follow-up.

@Vilin97
Vilin97 marked this pull request as ready for review September 12, 2026 23:22
@Vilin97
Vilin97 merged commit 097aaa6 into main Sep 12, 2026
7 checks passed
@greptile-apps

greptile-apps Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Advisory cross-file review against Lean Pool's supervisory rubric. This is not by itself a significance, source-verification, or merge verdict.

Retrigger

The PR is not safe to merge until final integration findings are made approval-blocking and the explicit repository requirement governing workflow changes is satisfied.

Findings

  1. P1 Final Findings Bypass Approval ▶
  2. P2 Excerpt Budget Uses Wrong Units ▶
  3. P2 Workflow Change Violates Requirement ▶

Summary

  • Adds contiguous source-range partitioning and SHA-256 coverage manifests.
  • Reconstructs omitted GitHub patches from hash-checked blobs and checks for a moving PR head.
  • Preserves portion evidence and unresolved obligations through bounded integration follow-ups.
  • Adds extensive tests for coverage, retries, reconstruction, accounting, and evidence persistence.
  • The final integration response can still approve while carrying its own unresolved findings, and exact-source follow-ups receive an incorrectly scaled budget.

Reviews (1) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..."

Comment on lines +1244 to +1245
if not queries:
break

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Final Findings Bypass Approval

When the final integration response has no source_requests, this loop exits before recording that response's own findings or open questions as obligations. Since enforce_resolutions only checks previously recorded obligations, a final payload can contain verdict: "pass" and unresolved findings while still allowing the aggregate review to approve.

Comment on lines +1257 to +1260
available = budget - _message_tokens(updated)
allowance = max(0, (available - 512) // len(queries))
bundle["source_followups"].extend(
review_portions.source_excerpts(diff, query, allowance) for query in queries

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Excerpt Budget Uses Wrong Units

available is measured in estimated tokens, but allowance is passed to source_excerpts as a character limit. With the configured estimate of two characters per token, this rejects excerpts that would fit in the remaining context, leaving answerable source requests unresolved and unnecessarily downgrading the review.

Comment on lines +224 to +234
REVIEW_EVIDENCE_PATH: ${{ runner.temp }}/review-evidence.json
run: uv run python -m lean_pool.review

- name: Upload review evidence
if: success() && steps.ci.outputs.conclusion == 'success'
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02
with:
name: review-evidence
path: ${{ runner.temp }}/review-evidence.json
if-no-files-found: ignore
retention-days: 30

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Workflow Change Violates Requirement

This adds an environment variable and artifact-upload step to a file under .github/workflows/. The repository directive says not to modify workflows, checks, or gates unless explicitly asked. That requirement must be satisfied before merging, either through the required explicit exception or by removing this workflow change.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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