Repository navigation
Review large PRs in complete 272k-token portions - #424
Conversation
|
Advisory cross-file review against Lean Pool's supervisory rubric. This is not by itself a significance, source-verification, or merge verdict. 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
Summary
Reviews (1) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." |
| if not queries: | ||
| break |
There was a problem hiding this comment.
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.
| 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 |
There was a problem hiding this comment.
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.
| 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 |
There was a problem hiding this comment.
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!
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.
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.