Implementation Plan: Local Review Rounds Before PR Creation (Reduce GitHub API Usage)#1970
Merged
Trecek merged 11 commits intoMay 6, 2026
Conversation
Adds ReviewConfig with local_review_rounds (default 3) and wires it through:
- AutomationConfig.from_dynaconf and defaults.yaml
- resolve_ingredient_defaults bridge (as "local_review_rounds" string)
- annotate_pr_diff callable: computes review_mode ("local" or "github")
based on iteration < local_rounds, using git diff in local mode
- All three looping recipes (implementation, implementation-groups, remediation):
- ingredients: adds local_review_rounds with default ""
- annotate_pr_diff: passes base_branch, local_review_rounds, current_iteration;
captures review_mode; adds optional_context_refs
- review_pr: appends mode=${{ context.review_mode }}
- resolve_review: appends mode=${{ context.review_mode }}
- Callable contracts for remediation and merge-prs
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…olve-review
Part B of the local review rounds feature (Part A covered config/callable/ingredient/recipe plumbing).
review-pr (mode=local):
- Skips all GitHub API calls for comment posting
- Writes findings to local_findings_{pr_number}.json with iteration tracking
- Still writes diff_context and raw_findings (mode-independent handoffs)
- Gate tokens emitted identically in both modes
resolve-review (mode=local):
- Reads from local_findings JSON instead of GitHub API
- ACCUMULATES DISCUSS findings to deferred_observations_{pr_number}.json (with round number)
- ACCUMULATES REJECT findings to reject_patterns_{pr_number}.json (stable filename, no timestamp)
- Skips all GitHub thread resolution and inline reply API calls
- Deduplicates before appending (guards against Issue #1604 duplicate comments)
- task test-check runs identically in both modes
resolve-review (mode=github):
- NEW: Before fetching findings, posts accumulated deferred_observations from prior local rounds
as batch PR review comments with REVIEW-FLAG markers
- Then proceeds with existing GitHub API flow unchanged
Tests: test_review_pr_local_mode.py (11 tests), test_resolve_review_local_mode.py (16 tests), test_review_local_mode_contracts.py (10 tests)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Trecek
commented
May 6, 2026
Collaborator
Author
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: changes_requested
Trecek
commented
May 6, 2026
Collaborator
Author
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review: 2 blocking (critical) issues found. See inline comments — changes are required before merging.
Critical findings:
src/autoskillit/smoke_utils.py:88— defense: int() conversions lack ValueError guard for non-numeric stringssrc/autoskillit/smoke_utils.py:89— bugs: review_mode='local' returned even when gh pr diff was called (empty base_branch fallback inconsistency)
…in annotate_pr_diff - Wrap local_review_rounds and current_iteration int() casts in try/except ValueError (non-numeric non-empty strings no longer raise uncaught exceptions) - Set review_mode="github" when local mode is requested but base_branch is empty, so the returned token matches the actual command executed (gh pr diff) - Add warning log on silent local→github downgrade for observability - Update T3.6 to supply base_branch so it actually tests local mode - Add T3.8 covering the empty-base_branch fallback edge case Addresses reviewer comments 3192654982, 3192654987, 3192654992, 3192654995, 3192655041 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_review_rounds - Add __post_init__ to ReviewConfig raising ValueError on negative values - Wrap settings.py int() cast in _parse_int_config() helper that raises ConfigSchemaError with a clear message for non-numeric env var / YAML values - Add local_review_rounds="0" fallback to ingredient_defaults.py except block so the key is always present even when config loading fails Addresses reviewer comments 3192654999, 3192655003, 3192655005 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…usion comment
- Remove unnecessary from __future__ import annotations in test_review_config.py
- Add comment explaining why merge-prs.yaml is excluded from
TestAnnotatePrDiffLocalReviewRounds parametrize fixture
- Drop trivially-satisfiable fallback disjunct "mode=" from test_resolve_review and
test_review_pr mode parameter assertions — only the specific form is meaningful
- Replace trivially-satisfiable "default:" check with specific phrase assertion
("absent or unrecognized") in test_review_pr_local_mode_default_is_github
Addresses reviewer comments 3192655014, 3192655016, 3192655025, 3192655029, 3192655035
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ist line numbers - T3.1 test_annotate_pr_diff_returns_review_mode_local: add base_branch="main" so the call actually exercises local mode (empty base_branch now falls back to github) - Update _LEGACY_JSON_WRITES allowlist in test_schema_version_convention.py to reflect smoke_utils.py line shifts caused by the try/except guard insertion: 115→126, 132→143, 346→357, 394→405 (also update list_site references) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Trecek
commented
May 6, 2026
Collaborator
Author
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: approved_with_comments
Trecek
commented
May 6, 2026
Collaborator
Author
Trecek
left a comment
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review: warning-only findings detected. See inline comments — no blocking changes required.
One finding (tests/infra/test_schema_version_convention.py L156) is marked requires_decision as it documents intentional dual membership of a smoke_utils.py line in both the legacy allowlist and list_sites — verify the comment is accurate or add inline documentation.
… in test_review_config.py Addresses reviewer comments 3192731887 and 3192731888. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
T3.3: move assert_called_once() after args[:3] check so assertion failures report the actual command args rather than the call count first. T3.4: add base_branch="" explicitly for symmetry with T3.1/T3.6. Addresses reviewer comments 3192731880 and 3192731884. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…7 in allowlist Lines 143 and 357 appear in both _LEGACY_JSON_WRITES and list_sites because the AST scanner cannot distinguish list vs dict write payloads. Add inline comment to prevent future confusion about the intentional dual membership. Addresses reviewer comment 3192731886. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Trecek
deleted the
local-review-rounds-before-pr-creation-reduce-github-api-usa/1945
branch
Trecek
added a commit
that referenced
this pull request
May 6, 2026
…tracts.py Two docstrings introduced by develop/#1970 exceeded the 99-char ruff limit. Shortened to pass pre-commit without changing assertion logic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Trecek
added a commit
that referenced
this pull request
May 6, 2026
…tracts.py Two docstrings introduced by develop/#1970 exceeded the 99-char ruff limit. Shortened to pass pre-commit without changing assertion logic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Trecek
added a commit
that referenced
this pull request
May 8, 2026
…itHub API Usage) (#1970) ## Summary Add a `local_review_rounds` configuration and plumbing so that the existing review loop steps (`annotate_pr_diff`, `review_pr`, `resolve_review`, `check_review_loop`) receive a `review_mode` context value (`"local"` or `"github"`) computed per-iteration. Part A covers the config dataclass, defaults, ingredient bridge, callable modification, recipe YAML wiring for all three looping recipes, and tests for all of the above. Part B will cover the skill SKILL.md behavioral changes (how `review-pr` and `resolve-review` branch on `mode=local` vs `mode=github`) — implement as a separate task. Closes #1945 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/impl-20260505-165914-350121/.autoskillit/temp/make-plan/local_review_rounds_plan_2026-05-05_170500_part_a.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) via AutoSkillit <!-- autoskillit:pipeline-signature steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr --> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a
local_review_roundsconfiguration and plumbing so that the existing review loop steps (annotate_pr_diff,review_pr,resolve_review,check_review_loop) receive areview_modecontext value ("local"or"github") computed per-iteration. Part A covers the config dataclass, defaults, ingredient bridge, callable modification, recipe YAML wiring for all three looping recipes, and tests for all of the above. Part B will cover the skill SKILL.md behavioral changes (howreview-prandresolve-reviewbranch onmode=localvsmode=github) — implement as a separate task.Closes #1945
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/impl-20260505-165914-350121/.autoskillit/temp/make-plan/local_review_rounds_plan_2026-05-05_170500_part_a.md🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary
Token Efficiency