feat: Expand nightly review autofix with deterministic-first architecture - #74
Conversation
…eterministic-first architecture - Add fix-type registry (fix_type_registry.py) as single source of truth for autofix capabilities - Expand autofix from ruff format only to ruff format + ruff check --fix (F401, I001) - Add deterministic-first workflow: run ruff before LLM review to save API tokens - Update both Claude Code Action prompts to skip deterministic issues - Replace jq validation with strict schema validator (validate_findings_schema.py) - Extract finding_utils.py from deleted dead code (run_consensus_review.py) - Fix CI gate: read-only ruff check (no --fix), decouple pytest from ruff - Add 22 new tests covering registry, inference, eligibility, commands, validation - Multi-pass idempotency for lint fixes (up to 3 passes) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewer's GuideIntroduces a centralized fix-type registry and registry-driven autofix pipeline, expands deterministic fixes to include ruff lint (unused imports + import sorting) before LLM review, enforces strict schema validation of findings, updates nightly workflow prompts and controls, and factors out reusable finding utilities while adding comprehensive tests. Sequence diagram for deterministic-first nightly review workflowsequenceDiagram
actor Developer
participant GitHubActions as GitHub_Actions
participant ScopeSelector as scope_selector.py
participant Deterministic as deterministic_fixes_step
participant LLM as Claude_Code_Action
participant Validator as validate_findings_schema.py
participant Normalizer as normalize_findings.py
participant Autofix as apply_autofix.py
participant CIGate as ci_gate_steps
Developer->>GitHubActions: Trigger nightly-review workflow_dispatch
GitHubActions->>ScopeSelector: Select scope-selection.json
ScopeSelector-->>GitHubActions: files and metadata
GitHubActions->>Deterministic: Apply deterministic fixes
Deterministic->>Deterministic: ruff format file
Deterministic->>Deterministic: ruff check --select F401,I001 --fix file
Deterministic-->>GitHubActions: deterministic_fixed_count, fixes_applied
GitHubActions->>LLM: Run consensus review (skip deterministic issues)
LLM-->>GitHubActions: review-findings.json
GitHubActions->>Validator: python validate_findings_schema.py review-findings.json review-findings-validated.json
Validator->>Validator: validate_findings_file
Validator->>Validator: validate_finding for each finding
Validator-->>GitHubActions: sanitized review-findings.json or error
GitHubActions->>Normalizer: python normalize_findings.py
Normalizer->>Normalizer: infer_fix_type and get_fix_type
Normalizer-->>GitHubActions: normalized findings and autofix plans
GitHubActions->>Autofix: python apply_autofix.py
Autofix->>Autofix: apply_autofix_to_file with fix_type from registry
Autofix->>Autofix: check_idempotency, check_syntax, check_git_changes
Autofix-->>GitHubActions: autofix_applied, details
GitHubActions->>CIGate: ruff check src/
CIGate-->>GitHubActions: ci_ruff_passed
GitHubActions->>CIGate: pytest tests/
CIGate-->>GitHubActions: ci_tests_passed
GitHubActions-->>Developer: Create suggestion and autofix PRs (or rollback on failure)
Class diagram for fix-type registry and autofix pipelineclassDiagram
class FixType {
+str name
+float confidence_threshold
+tuple~str~ categories
+str tool_command
+bool safe
+int max_passes
+tuple~str~ ruff_select_codes
+str description
}
class fix_type_registry {
+dict~str, FixType~ FIX_TYPES
+list~tuple~list~str~, str~~ SUGGESTION_INFERENCE_RULES
+FixType get_fix_type(name str)
+dict~str, FixType~ get_all_fix_types()
+bool is_known_fix_type(name str)
+str infer_fix_type(suggestion str, explicit_fix_type str)
}
class normalize_findings {
+float CONFIDENCE_THRESHOLD
+dict~str, int~ SEVERITY_RANK
+bool is_finding_autofix_eligible(finding dict~str, any~)
+dict~str, any~ normalize_finding(finding dict~str, any~)
}
class apply_autofix {
+tuple~bool, str~ pre_check_file(file_path Path)
+list~str~ build_fix_command(fix_type FixType, file_path Path)
+tuple~bool, str~ apply_fix(file_path Path, fix_type_name str)
+tuple~bool, str~ check_idempotency(file_path Path, fix_type_name str)
+tuple~bool, str~ check_syntax(file_path Path)
+tuple~bool, str~ check_git_changes(file_path Path)
+tuple~bool, str, dict~str, any~~ apply_autofix_to_file(file_path Path, fix_type_name str)
+dict~str, any~ apply_autofix_to_category(category str, fix_plans_dir Path)
}
class finding_utils {
+dict~str, any~ FINDING_SCHEMA
+bool validate_finding(finding dict~str, any~)
+list~dict~str, any~~ deduplicate_findings(findings list~dict~str, any~~)
}
class validate_findings_schema {
+tuple~bool, str, dict~str, any~~ validate_findings_file(findings_path Path)
+main()
}
fix_type_registry "1" *-- "many" FixType : registers
normalize_findings ..> fix_type_registry : uses get_fix_type
normalize_findings ..> fix_type_registry : uses infer_fix_type
apply_autofix ..> fix_type_registry : uses get_fix_type
validate_findings_schema ..> finding_utils : uses validate_finding
validate_findings_schema ..> finding_utils : uses FINDING_SCHEMA
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughAdds a registry-driven, multi-type autofix system and strict finding validation to the nightly review pipeline, replacing ruff-only autofix and removing the old consensus script; normalizer and autofix flows now infer and apply per-fix-type commands, with validation, deduplication, and expanded tests. Changes
Sequence DiagramsequenceDiagram
participant WF as Nightly Workflow
participant LLM as LLM Review
participant Norm as Normalize Findings
participant Reg as Fix-Type Registry
participant Autofix as Apply Autofix
participant Git as Git
WF->>WF: Run pre-LLM deterministic fixes (ruff format/fix)
WF->>LLM: Send scoped files for review
LLM-->>WF: Findings with suggestions
WF->>Norm: Submit findings
Norm->>Reg: Infer/_resolve fix_type for each finding
Reg-->>Norm: Return FixType metadata (thresholds, commands, max_passes)
Norm->>Norm: Validate eligibility (confidence, category, allowlist, LOC)
Norm-->>Autofix: Pass eligible findings with _resolved_fix_type
Autofix->>Reg: Build command for fix_type
Reg-->>Autofix: Return command template
Autofix->>Autofix: Execute fix, run idempotency (multi-pass)
Autofix->>Git: Commit or report changes
Git-->>WF: Applied changes / status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
build_fix_command, splitting the interpolatedtool_commandstring with.split()is fragile (e.g., for paths with spaces or future flags containing spaces); consider storing the command as a list inFixTypeor usingshlex.splitto build a robust argv. - Both
normalize_findings.pyandvalidate_findings_schema.pymanipulatesys.pathto import sibling modules; it would be more maintainable to turnscriptsinto a proper package (with__init__.py) and use explicit relative or package imports instead. - Now that autofix confidence is driven by
FixType.confidence_threshold, the module-levelCONFIDENCE_THRESHOLDinnormalize_findings.pyappears unused; consider removing it to avoid confusion about which threshold is authoritative.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `build_fix_command`, splitting the interpolated `tool_command` string with `.split()` is fragile (e.g., for paths with spaces or future flags containing spaces); consider storing the command as a list in `FixType` or using `shlex.split` to build a robust argv.
- Both `normalize_findings.py` and `validate_findings_schema.py` manipulate `sys.path` to import sibling modules; it would be more maintainable to turn `scripts` into a proper package (with `__init__.py`) and use explicit relative or package imports instead.
- Now that autofix confidence is driven by `FixType.confidence_threshold`, the module-level `CONFIDENCE_THRESHOLD` in `normalize_findings.py` appears unused; consider removing it to avoid confusion about which threshold is authoritative.
## Individual Comments
### Comment 1
<location path="scripts/apply_autofix.py" line_range="86-94" />
<code_context>
-def apply_ruff_format(file_path: Path) -> Tuple[bool, str]:
- """Apply ruff format to a single file."""
- success, output = run_command(["ruff", "format", str(file_path)])
+def build_fix_command(fix_type: FixType, file_path: Path) -> List[str]:
+ """Build the shell command for a fix type, substituting placeholders."""
+ cmd_str = fix_type.tool_command.replace("{file}", str(file_path))
+
+ if fix_type.ruff_select_codes and "{codes}" in cmd_str:
+ codes = ",".join(fix_type.ruff_select_codes)
+ cmd_str = cmd_str.replace("{codes}", codes)
+
+ return cmd_str.split()
+
+
</code_context>
<issue_to_address>
**issue:** Splitting the command string with `str.split()` can break for paths with spaces or more complex arguments.
`build_fix_command` currently uses `cmd_str.split()`, which will break on file paths with spaces or quoted arguments. Prefer either (a) representing `tool_command` as a list of args and doing placeholder substitution per element, or (b) using `shlex.split` and documenting that `tool_command` must be a shell-style string.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def build_fix_command(fix_type: FixType, file_path: Path) -> List[str]: | ||
| """Build the shell command for a fix type, substituting placeholders.""" | ||
| cmd_str = fix_type.tool_command.replace("{file}", str(file_path)) | ||
|
|
||
| if fix_type.ruff_select_codes and "{codes}" in cmd_str: | ||
| codes = ",".join(fix_type.ruff_select_codes) | ||
| cmd_str = cmd_str.replace("{codes}", codes) | ||
|
|
||
| return cmd_str.split() |
There was a problem hiding this comment.
issue: Splitting the command string with str.split() can break for paths with spaces or more complex arguments.
build_fix_command currently uses cmd_str.split(), which will break on file paths with spaces or quoted arguments. Prefer either (a) representing tool_command as a list of args and doing placeholder substitution per element, or (b) using shlex.split and documenting that tool_command must be a shell-style string.
Claude Code Review (AWS Bedrock Sonnet 4.5)This PR expands nightly review autofix with an excellent deterministic-first architecture. New fix-type registry, multi-pass idempotency validation, and comprehensive safety checks. 32 new tests included. CRITICAL SECURITY ISSUES (must fix before merge):
HIGH PRIORITY: POSITIVE:
RATINGS: Security 3/5, Quality 4/5, Architecture 5/5, Testing 4/5 RECOMMENDATION: Approve with required changes. Fix security vulnerabilities before merging. Review completed 2026-03-13 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/nightly-review.yml (1)
741-760:⚠️ Potential issue | 🟠 MajorUse
PR_STALE_DAYShere instead of hard-coding 7 days.This PR introduces
stale_days/PR_STALE_DAYS, but both cleanup queries still compare against7 * 86400. Manual overrides will not affect stale PR cleanup.⏳ Proposed fix
- # Close stale nightly-review PRs (>7 days old with no activity) + # Close stale nightly-review PRs (>${{ env.PR_STALE_DAYS }} days old with no activity) gh pr list \ --label "nightly-review" \ --state open \ --json number,updatedAt \ - --jq '.[] | select((now - (.updatedAt | fromdateiso8601)) > (7 * 86400)) | .number' \ + --jq '.[] | select((now - (.updatedAt | fromdateiso8601)) > (${{ env.PR_STALE_DAYS }} * 86400)) | .number' \ | xargs -I {} gh pr close {} --comment "Closing stale nightly review PR (>7 days old)" - # Close stale autofix PRs (Phase 2) + # Close stale autofix PRs (>${{ env.PR_STALE_DAYS }} days old) gh pr list \ --label "nightly-review-autofix" \ --state open \ --json number,updatedAt \ - --jq '.[] | select((now - (.updatedAt | fromdateiso8601)) > (7 * 86400)) | .number' \ + --jq '.[] | select((now - (.updatedAt | fromdateiso8601)) > (${{ env.PR_STALE_DAYS }} * 86400)) | .number' \ | xargs -I {} gh pr close {} --comment "Closing stale autofix PR (>7 days old)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/nightly-review.yml around lines 741 - 760, Replace the hard-coded "7 * 86400" in the "Cleanup stale PRs" run block with a computed value derived from the existing PR_STALE_DAYS/stale_days variable: compute STALE_SECONDS=$((PR_STALE_DAYS * 86400)) at the start of the run script (or export it from env), then update both jq selectors in the gh pr list commands (the expressions that currently use (7 * 86400)) to compare against the numeric ${STALE_SECONDS} value instead; keep the same labels ("nightly-review" and "nightly-review-autofix") and gh pr close calls but use ${STALE_SECONDS} in place of the hard-coded expression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/nightly-review.yml:
- Around line 193-228: The step deterministic_fixes is currently using git add
-A which stages the workflow artifact scope-selection.json and other unrelated
files; replace the global add with a targeted add of only modified files (e.g.
replace git add -A with git add -- $(git diff --name-only --diff-filter=M) or
git add -- $SCOPE_FILES after filtering for existing modified files) so only the
files changed by ruff (tracked by FIXED_FILES/git diff) are staged; update the
git add invocation in the deterministic_fixes block accordingly and ensure it
handles empty lists safely.
In `@scripts/apply_autofix.py`:
- Around line 330-333: The loop currently only takes the first finding's
_resolved_fix_type per file (file_findings[0]) and skips other eligible fix
types; change it to iterate the distinct _resolved_fix_type values for each file
in a stable order (e.g., sorted set) and call
apply_autofix_to_file(Path(file_path), fix_type_name) for each distinct
fix_type; use the files_to_fix and file_findings variables to build the per-file
set of fix types (grouping by (file, _resolved_fix_type) semantics) so both
ruff_format and ruff_lint_fix are applied when present.
- Around line 209-214: Before calling apply_fix(file_path, fix_type_name)
capture the current file state (e.g., read file_path into original_text or
create a temporary backup); if apply_fix returns success == False, restore the
original file content (write original_text back to file_path or run a git
restore) before appending to details["checks_failed"] and returning; keep the
existing success/message handling and use the same local variables (apply_fix,
success, message, details, file_path) to locate where to add the backup/restore
logic.
- Around line 86-94: The build_fix_command function currently calls .split() on
the assembled command string which breaks file paths with spaces; instead, use
shlex.split on fix_type.tool_command to tokenize safely, then iterate tokens and
substitute the {file} and {codes} placeholders per token (using str(file_path)
for {file} and ",".join(fix_type.ruff_select_codes) for {codes} when
fix_type.ruff_select_codes is set) and return the final list of tokens; update
references in build_fix_command and keep using the FixType.tool_command and
FixType.ruff_select_codes symbols for locating the change.
In `@scripts/finding_utils.py`:
- Around line 12-35: FINDING_SCHEMA is too lax: add explicit types and
constraints for all fields used later (set "file", "issue", and "suggestion" to
{"type":"string"}, make "line_end" required with {"type":"integer","minimum":1}
and add a "fix_type" property (string or enum that matches normalize_findings
expectations)); then ensure the validator call (the function that currently
returns True on success, e.g. validate_finding) runs jsonschema.validate against
this tightened FINDING_SCHEMA and only returns True when validation passes so
downstream code (Path(file_path), line_end - line_start,
explicit_fix_type.lower()) never receives wrong types or missing fields.
In `@scripts/validate_findings_schema.py`:
- Around line 51-58: The code mutates the raw input by assigning
sanitized["summary"]["total"] which fails if data.get("summary") is None or not
a mapping; instead create a fresh summary dict for sanitized: inspect
data.get("summary") and if it's a mapping (e.g., isinstance(..., Mapping))
shallow-copy its keys into a new dict, otherwise create a new dict with "total"
and "by_category" defaults, then set sanitized["summary"] to that new dict and
finally set its "total" to len(valid_findings); update the logic around
sanitized, valid_findings, data and the sanitized["summary"]["total"] assignment
to use this safe-summary construction.
---
Outside diff comments:
In @.github/workflows/nightly-review.yml:
- Around line 741-760: Replace the hard-coded "7 * 86400" in the "Cleanup stale
PRs" run block with a computed value derived from the existing
PR_STALE_DAYS/stale_days variable: compute STALE_SECONDS=$((PR_STALE_DAYS *
86400)) at the start of the run script (or export it from env), then update both
jq selectors in the gh pr list commands (the expressions that currently use (7 *
86400)) to compare against the numeric ${STALE_SECONDS} value instead; keep the
same labels ("nightly-review" and "nightly-review-autofix") and gh pr close
calls but use ${STALE_SECONDS} in place of the hard-coded expression.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9afdd54-51b7-4d34-92bc-44ea1a65f502
📒 Files selected for processing (8)
.github/workflows/nightly-review.ymlscripts/apply_autofix.pyscripts/finding_utils.pyscripts/fix_type_registry.pyscripts/normalize_findings.pyscripts/run_consensus_review.pyscripts/validate_findings_schema.pytests/integration/test_nightly_review.py
💤 Files with no reviewable changes (1)
- scripts/run_consensus_review.py
| - name: Apply deterministic fixes (pre-LLM) | ||
| id: deterministic_fixes | ||
| if: steps.scope_selector.outputs.skip != 'true' | ||
| run: | | ||
| # Get list of Python files from scope selection | ||
| SCOPE_FILES=$(jq -r '.files[].path' scope-selection.json | grep '\.py$' || true) | ||
|
|
||
| if [[ -z "$SCOPE_FILES" ]]; then | ||
| echo "No Python files in scope - skipping deterministic fixes" | ||
| echo "fixes_applied=false" >> $GITHUB_OUTPUT | ||
| exit 0 | ||
| fi | ||
|
|
||
| pip install ruff | ||
|
|
||
| FIXED_FILES=0 | ||
| for file in $SCOPE_FILES; do | ||
| if [[ -f "$file" ]]; then | ||
| # Format | ||
| ruff format "$file" 2>/dev/null || true | ||
| # Fix unused imports and import sorting | ||
| ruff check --select F401,I001 --fix "$file" 2>/dev/null || true | ||
|
|
||
| # Check if file was modified | ||
| if ! git diff --quiet -- "$file" 2>/dev/null; then | ||
| FIXED_FILES=$((FIXED_FILES + 1)) | ||
| fi | ||
| fi | ||
| done | ||
|
|
||
| echo "deterministic_fixed_count=$FIXED_FILES" >> $GITHUB_OUTPUT | ||
|
|
||
| if [[ $FIXED_FILES -gt 0 ]]; then | ||
| echo "fixes_applied=true" >> $GITHUB_OUTPUT | ||
| echo "Applied deterministic fixes to $FIXED_FILES files" | ||
| git add -A |
There was a problem hiding this comment.
Don’t stage the whole workspace in the deterministic-fixes step.
By this point Line 167 has already created scope-selection.json. git add -A will stage that workflow artifact alongside Ruff’s source edits, so the eventual PR can pick up generated files that were never meant to be committed.
🧹 Proposed fix
- FIXED_FILES=0
- for file in $SCOPE_FILES; do
+ FIXED_FILES=0
+ MODIFIED_FILES=()
+ for file in $SCOPE_FILES; do
if [[ -f "$file" ]]; then
@@
if ! git diff --quiet -- "$file" 2>/dev/null; then
FIXED_FILES=$((FIXED_FILES + 1))
+ MODIFIED_FILES+=("$file")
fi
fi
done
@@
if [[ $FIXED_FILES -gt 0 ]]; then
echo "fixes_applied=true" >> $GITHUB_OUTPUT
echo "Applied deterministic fixes to $FIXED_FILES files"
- git add -A
+ git add -- "${MODIFIED_FILES[@]}"
else🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/nightly-review.yml around lines 193 - 228, The step
deterministic_fixes is currently using git add -A which stages the workflow
artifact scope-selection.json and other unrelated files; replace the global add
with a targeted add of only modified files (e.g. replace git add -A with git add
-- $(git diff --name-only --diff-filter=M) or git add -- $SCOPE_FILES after
filtering for existing modified files) so only the files changed by ruff
(tracked by FIXED_FILES/git diff) are staged; update the git add invocation in
the deterministic_fixes block accordingly and ensure it handles empty lists
safely.
| def build_fix_command(fix_type: FixType, file_path: Path) -> List[str]: | ||
| """Build the shell command for a fix type, substituting placeholders.""" | ||
| cmd_str = fix_type.tool_command.replace("{file}", str(file_path)) | ||
|
|
||
| if fix_type.ruff_select_codes and "{codes}" in cmd_str: | ||
| codes = ",".join(fix_type.ruff_select_codes) | ||
| cmd_str = cmd_str.replace("{codes}", codes) | ||
|
|
||
| return cmd_str.split() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
from scripts.apply_autofix import build_fix_command
from scripts.fix_type_registry import get_fix_type
path = Path("src/space dir/main file.py")
for fix_name in ("ruff_format", "ruff_lint_fix"):
print(fix_name, build_fix_command(get_fix_type(fix_name), path))
PYRepository: Tony363/SuperClaude
Length of output: 231
Use shlex.split() to tokenize the command template before substituting placeholders.
Calling .split() on a flat string breaks file paths containing spaces. With input src/space dir/main file.py, the current code produces ['ruff', 'format', 'src/space', 'dir/main', 'file.py'] instead of the correct ['ruff', 'format', 'src/space dir/main file.py'], causing the tool to receive wrong arguments. Parse the template with shlex.split() first, then replace {file} and {codes} on individual tokens.
🧩 Proposed fix
import argparse
import json
+import shlex
import subprocess
import sys
@@
def build_fix_command(fix_type: FixType, file_path: Path) -> List[str]:
"""Build the shell command for a fix type, substituting placeholders."""
- cmd_str = fix_type.tool_command.replace("{file}", str(file_path))
-
- if fix_type.ruff_select_codes and "{codes}" in cmd_str:
- codes = ",".join(fix_type.ruff_select_codes)
- cmd_str = cmd_str.replace("{codes}", codes)
-
- return cmd_str.split()
+ codes = ",".join(fix_type.ruff_select_codes or ())
+ return [
+ str(file_path) if token == "{file}" else codes if token == "{codes}" else token
+ for token in shlex.split(fix_type.tool_command)
+ ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/apply_autofix.py` around lines 86 - 94, The build_fix_command
function currently calls .split() on the assembled command string which breaks
file paths with spaces; instead, use shlex.split on fix_type.tool_command to
tokenize safely, then iterate tokens and substitute the {file} and {codes}
placeholders per token (using str(file_path) for {file} and
",".join(fix_type.ruff_select_codes) for {codes} when fix_type.ruff_select_codes
is set) and return the final list of tokens; update references in
build_fix_command and keep using the FixType.tool_command and
FixType.ruff_select_codes symbols for locating the change.
| # 2. Apply fix (dispatched from registry) | ||
| success, message = apply_fix(file_path, fix_type_name) | ||
| if not success: | ||
| details["checks_failed"].append(("ruff_format", message)) | ||
| print(f" ❌ Ruff format: {message}") | ||
| details["checks_failed"].append((fix_type_name, message)) | ||
| print(f" FAIL {fix_type_name}: {message}") | ||
| return (False, message, details) |
There was a problem hiding this comment.
Rollback the file when the fixer itself fails.
This early-return path does not restore the file. If a fixer rewrites the buffer before exiting non-zero, those partial edits stay in the worktree and can leak into later CI/PR steps.
🛑 Proposed fix
success, message = apply_fix(file_path, fix_type_name)
if not success:
details["checks_failed"].append((fix_type_name, message))
print(f" FAIL {fix_type_name}: {message}")
+ run_command(["git", "restore", "--source=HEAD", "--", str(file_path)])
return (False, message, details)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/apply_autofix.py` around lines 209 - 214, Before calling
apply_fix(file_path, fix_type_name) capture the current file state (e.g., read
file_path into original_text or create a temporary backup); if apply_fix returns
success == False, restore the original file content (write original_text back to
file_path or run a git restore) before appending to details["checks_failed"] and
returning; keep the existing success/message handling and use the same local
variables (apply_fix, success, message, details, file_path) to locate where to
add the backup/restore logic.
| for file_path, file_findings in files_to_fix.items(): | ||
| success, message, details = apply_autofix_to_file(Path(file_path)) | ||
| # Use the resolved fix type from normalization, default to ruff_format | ||
| fix_type_name = file_findings[0].get("_resolved_fix_type", "ruff_format") | ||
| success, message, details = apply_autofix_to_file(Path(file_path), fix_type_name) |
There was a problem hiding this comment.
Apply each resolved fix type for a file, not just the first one.
Once a file has both ruff_format and ruff_lint_fix findings, this path uses file_findings[0] and silently skips the other eligible fix. Group by (file, _resolved_fix_type) or iterate the distinct fix types for each file in a stable order.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/apply_autofix.py` around lines 330 - 333, The loop currently only
takes the first finding's _resolved_fix_type per file (file_findings[0]) and
skips other eligible fix types; change it to iterate the distinct
_resolved_fix_type values for each file in a stable order (e.g., sorted set) and
call apply_autofix_to_file(Path(file_path), fix_type_name) for each distinct
fix_type; use the files_to_fix and file_findings variables to build the per-file
set of fix types (grouping by (file, _resolved_fix_type) semantics) so both
ruff_format and ruff_lint_fix are applied when present.
| FINDING_SCHEMA = { | ||
| "type": "object", | ||
| "required": [ | ||
| "category", | ||
| "severity", | ||
| "file", | ||
| "line_start", | ||
| "issue", | ||
| "suggestion", | ||
| "confidence", | ||
| "actionable", | ||
| ], | ||
| "properties": { | ||
| "category": {"enum": ["security", "quality", "performance", "tests"]}, | ||
| "severity": {"enum": ["critical", "high", "medium", "low"]}, | ||
| "file": {"type": "string"}, | ||
| "line_start": {"type": "integer", "minimum": 1}, | ||
| "line_end": {"type": "integer", "minimum": 1}, | ||
| "issue": {"type": "string"}, | ||
| "suggestion": {"type": "string"}, | ||
| "confidence": {"type": "number", "minimum": 0, "maximum": 1}, | ||
| "actionable": {"type": "boolean"}, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Validate all schema-backed fields before returning True.
This now fronts the LLM-generated JSON, but it only enforces enums plus line_start/confidence/actionable. A finding with a non-string file, issue, suggestion, or fix_type, or with an invalid line_end, still passes here and then blows up later in scripts/normalize_findings.py when it hits Path(file_path), line_end - line_start, or explicit_fix_type.lower().
🛡️ Proposed fix
FINDING_SCHEMA = {
"type": "object",
@@
"properties": {
"category": {"enum": ["security", "quality", "performance", "tests"]},
"severity": {"enum": ["critical", "high", "medium", "low"]},
"file": {"type": "string"},
"line_start": {"type": "integer", "minimum": 1},
"line_end": {"type": "integer", "minimum": 1},
"issue": {"type": "string"},
"suggestion": {"type": "string"},
"confidence": {"type": "number", "minimum": 0, "maximum": 1},
"actionable": {"type": "boolean"},
+ "fix_type": {"type": "string"},
},
}
@@
def validate_finding(finding: Dict[str, Any]) -> bool:
"""Validate finding against schema."""
try:
+ if not isinstance(finding, dict):
+ return False
+
# Check required fields
@@
if finding["severity"] not in ["critical", "high", "medium", "low"]:
return False
# Check types
- if not isinstance(finding["line_start"], int) or finding["line_start"] < 1:
+ if not all(isinstance(finding[field], str) for field in ("file", "issue", "suggestion")):
+ return False
+ if type(finding["line_start"]) is not int or finding["line_start"] < 1:
return False
+ if "line_end" in finding and (
+ type(finding["line_end"]) is not int
+ or finding["line_end"] < finding["line_start"]
+ ):
+ return False
+ if "fix_type" in finding and not isinstance(finding["fix_type"], str):
+ return False
- if not isinstance(finding["confidence"], (int, float)) or not (
+ if isinstance(finding["confidence"], bool) or not isinstance(
+ finding["confidence"], (int, float)
+ ) or not (
0 <= finding["confidence"] <= 1
):
return FalseAlso applies to: 38-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/finding_utils.py` around lines 12 - 35, FINDING_SCHEMA is too lax:
add explicit types and constraints for all fields used later (set "file",
"issue", and "suggestion" to {"type":"string"}, make "line_end" required with
{"type":"integer","minimum":1} and add a "fix_type" property (string or enum
that matches normalize_findings expectations)); then ensure the validator call
(the function that currently returns True on success, e.g. validate_finding)
runs jsonschema.validate against this tightened FINDING_SCHEMA and only returns
True when validation passes so downstream code (Path(file_path), line_end -
line_start, explicit_fix_type.lower()) never receives wrong types or missing
fields.
| sanitized = { | ||
| "findings": valid_findings, | ||
| "summary": data.get( | ||
| "summary", | ||
| {"total": len(valid_findings), "by_category": {}}, | ||
| ), | ||
| } | ||
| sanitized["summary"]["total"] = len(valid_findings) |
There was a problem hiding this comment.
Sanitize summary instead of mutating the raw input.
If the LLM emits "summary": null (or a string/list), sanitized["summary"]["total"] = ... raises and you lose otherwise valid findings. Build a fresh summary dict unless the incoming value is already a mapping.
🧼 Proposed fix
- sanitized = {
- "findings": valid_findings,
- "summary": data.get(
- "summary",
- {"total": len(valid_findings), "by_category": {}},
- ),
- }
- sanitized["summary"]["total"] = len(valid_findings)
+ raw_summary = data.get("summary")
+ sanitized_summary = raw_summary.copy() if isinstance(raw_summary, dict) else {}
+ sanitized_summary["total"] = len(valid_findings)
+ sanitized_summary.setdefault("by_category", {})
+
+ sanitized = {
+ "findings": valid_findings,
+ "summary": sanitized_summary,
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/validate_findings_schema.py` around lines 51 - 58, The code mutates
the raw input by assigning sanitized["summary"]["total"] which fails if
data.get("summary") is None or not a mapping; instead create a fresh summary
dict for sanitized: inspect data.get("summary") and if it's a mapping (e.g.,
isinstance(..., Mapping)) shallow-copy its keys into a new dict, otherwise
create a new dict with "total" and "by_category" defaults, then set
sanitized["summary"] to that new dict and finally set its "total" to
len(valid_findings); update the logic around sanitized, valid_findings, data and
the sanitized["summary"]["total"] assignment to use this safe-summary
construction.
PAL MCP Consensus Code Review (via AWS Bedrock)OverviewThis PR expands the nightly review autofix capabilities from formatting-only to a deterministic-first architecture that handles both formatting ( Key Architectural Changes:
Critical IssuesNone found. The code demonstrates strong security practices with multiple layers of validation. High PriorityNone found. The implementation follows defensive coding principles with proper error handling and rollback mechanisms. Medium Priority1. Command Construction Safety (apply_autofix.py:86-94)Issue: Commands are built using string replacement and 2. Dead Code Reference (finding_utils.py:5)Issue: Docstring references 3. Duplicate Constants Across FilesIssue: Positive Observations
Review Summary
Recommendation: Approve with minor improvements. The medium-priority items are quality improvements, not blockers. This review was generated by PAL MCP Consensus Code Review (AWS Bedrock). |
The Claude agent inside claude-code-action couldn't post comments because Bash(gh pr comment:*) pattern didn't match the actual command, and Write wasn't in allowed-tools. Fix by: 1. Tell Claude to write review to pal-review.md (using Write tool) 2. Add Write to allowed-tools, remove Bash(gh pr comment) 3. Add dedicated shell steps that post the file via gh pr comment --body-file 4. Fix Bash tool patterns: colon separator to space (gh pr diff:* -> gh pr diff *) This is more robust than relying on Claude to format complex heredoc commands inside JSON-encoded tool calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Claude Code Review (via AWS Bedrock)OverviewThis PR introduces a deterministic-first architecture for the nightly review workflow, expanding autofix capabilities from basic formatting to include lint fixes with a centralized fix-type registry. The implementation adds 914 lines with significant architectural improvements including schema validation, better separation of concerns, and comprehensive test coverage (22 new tests added). Key Changes:
Critical IssuesNone identified - All critical security and safety checks are in place. High Priority1. Command Injection Prevention - EXCELLENT ✅Location: The
Validation: # Line 43: subprocess.run(cmd, ..., check=check)
# cmd is a list, not a string - prevents shell injection2. Path Traversal Protection - ROBUST ✅Location: try:
file_path.resolve().relative_to(Path.cwd().resolve())
except ValueError:
return (False, f"File outside repository: {file_path}")Strong protection against 3. Registry Immutability - EXCELLENT DESIGN ✅Location: @dataclass(frozen=True)
class FixType:Using 4. Idempotency Verification - CRITICAL SAFETY CHECK ✅Location: The multi-pass idempotency check is essential for automated fixes:
This prevents runaway modifications and ensures fixes are stable. Medium Priority1. Error Handling: Let It Crash PhilosophyLocation: The "Let It Crash" comment suggests intentional exception propagation: # Let It Crash: Don't catch exceptions - subprocess errors should propagate.Recommendation: While philosophically aligned with Erlang-style supervision, consider:
Example improvement: except subprocess.CalledProcessError as e:
# Expected failure for validation checks
return (False, e.stderr if e.stderr else str(e))
except Exception as e:
# Unexpected error - add context before propagating
raise RuntimeError(f"Unexpected error in run_command: {cmd}") from e2. Duplicate Constants Across FilesLocation:
Issue: These constants are duplicated with a comment saying "must match" (line 28): # Safety limits (must match normalize_findings.py)
MAX_FILES_PER_RUN = 5
MAX_LOC_PER_FILE = 200Recommendation: Move to a shared constants module to ensure single source of truth: # scripts/autofix_constants.py
MAX_FILES_PER_RUN = 5
MAX_LOC_PER_FILE = 200
MAX_LOC_PER_FINDING = 200Then import in both files. This prevents drift and eliminates manual synchronization. 3. Line Count Difference: Semantic IssueLocation:
These measure different things:
Risk: A file could pass normalization's LOC check but fail apply's check if the finding's line range is small but the file is large. Recommendation: Clarify in comments that these are intentionally different checks:
4. Magic Numbers in TestsLocation: assert len(pr_files) == 4 # security, quality, performance, testsRecommendation: Use named constants: EXPECTED_CATEGORIES = ["security", "quality", "performance", "tests"]
assert len(pr_files) == len(EXPECTED_CATEGORIES)Makes test intent clearer and easier to update if categories change. Positive Observations1. Registry-Driven Architecture 🎯The fix-type registry is exemplary separation of concerns:
This is textbook Open/Closed Principle - open for extension (add new fix types), closed for modification (existing types immutable). 2. Comprehensive Safety Checks 🛡️The 5-stage safety pipeline in
Automatic rollback on any failure (lines 225, 237) ensures safety. 3. Schema Validation with Sanitization ✨Location: The validator doesn't just reject bad data - it sanitizes by filtering: for i, finding in enumerate(data["findings"]):
if validate_finding(finding):
valid_findings.append(finding)
else:
invalid_count += 1This is resilient - partial failures don't crash the pipeline. Invalid findings are logged but don't block valid ones. 4. Test Coverage - Exceptional 🧪22 new tests covering:
1191 tests passed, 0 failed - this is outstanding test discipline. 5. Inference Rule Flexibility 🔍Location: The
This allows both structured and unstructured finding sources to work with autofix. 6. Security: No Shell Injection Vectors 🔒Comprehensive audit confirms:
7. Dead Code Removal 🧹Deleting 8. Workflow Separation of Concerns 📋Location: The new workflow step separates:
This is better than having Claude post directly:
Architecture AssessmentDeterministic-First Design ⭐⭐⭐⭐⭐The "run deterministic fixes before LLM review" approach is architecturally sound: Benefits:
Implementation:
This follows the principle of least powerful tool - use simple deterministic tools when possible, LLMs only when needed. Registry Pattern ⭐⭐⭐⭐⭐The fix-type registry is a data-driven architecture win:
Scalability: As more fix types are added (Black, isort, mypy autofixes), the registry scales linearly without complexity explosion. Pipeline Robustness ⭐⭐⭐⭐The multi-stage pipeline handles failures gracefully:
Each stage has clear inputs/outputs and defined failure modes. Performance NotesSubprocess OverheadLocation: The idempotency check runs the fix tool multiple times per file:
For 5 files: This means 10-20 tool invocations. Assessment: Acceptable for nightly runs (not real-time). Ruff is fast (~10ms per file), so total overhead is ~200ms worst case. If scaling to 100s of files: Consider:
File I/O in Idempotency CheckLocation: Reads file before/after each pass: with open(file_path, "r") as f:
content_before = f.read()
# ... apply fix ...
with open(file_path, "r") as f:
content_after = f.read()For small files (<200 LOC): Negligible overhead. Review Summary
Overall Assessment: This is high-quality, production-ready code with strong architectural foundations. The deterministic-first approach is innovative and cost-effective. The fix-type registry is a model for extensibility. Security checks are thorough. Recommendation: ✅ Approve with minor suggestions Suggested Follow-Ups (Non-Blocking)
Generated by Claude Code Review (AWS Bedrock) - Sonnet 4.5 |
PAL MCP Consensus Code ReviewOverviewThis PR expands the nightly review autofix capability from formatting-only ( Key architectural improvements:
Files Changed: 9 files, +914/-437 lines
Critical IssuesNone identified. No blocking security vulnerabilities or breaking changes detected. High Priority1. Command Injection Risk in
|
| Category | Rating | Notes |
|---|---|---|
| Security | 4.5/5 | Strong path validation, allowlist controls. Minor: command injection edge case (file paths with spaces) |
| Code Quality | 5/5 | Excellent architecture (registry pattern), clear separation of concerns, comprehensive error handling |
| Architecture | 5/5 | Deterministic-first design is innovative and cost-effective. Registry pattern enables easy extension |
| Testing | 4.5/5 | Comprehensive test coverage. Minor gap: multi-pass idempotency convergence test |
Overall Assessment: APPROVED with minor suggestions
This is a high-quality PR that significantly improves the nightly review workflow architecture. The deterministic-first approach is a clever optimization that will save API costs while improving review quality. The fix-type registry pattern demonstrates strong software engineering principles.
Recommendations Priority
- High Priority: Address command injection edge case (Welcome to SuperClaude Discussions! #1) - use
shlex.split() - High Priority: Fix allowlist pattern matching redundancy (remove deepwiki mcp support #2)
- Medium Priority: Improve error visibility in deterministic fixes (chore: cleanup temp files and update .gitignore #3)
- Low Priority: Add workflow input validation (chore: remove dead code, bloat, and redundant documentation #4)
- Low Priority: Add test for multi-pass convergence (test: Add coverage for critical backbone modules (cli.py, coordination.py, unified_store.py) #6)
- Low Priority: Keep backup of original findings before sanitization (chore: Stabilize CI and Implement Phased Coverage Strategy (30% → 80%) #7)
This review was generated by manual code analysis (PAL MCP tools unavailable in this environment).
Multiple architectural patterns and security concerns were analyzed systematically.
Review is advisory - please use human judgment for final decisions.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ai-review.yml:
- Around line 162-170: The "Post Bedrock review comment" step currently only
checks for pal-review.md existence and can post stale content; update this step
(and the analogous step that posts other provider reviews) to validate
provider-specific marker text inside pal-review.md (e.g., look for a header like
"Provider: Bedrock" or a known Bedrock marker) before running gh pr comment, or
change the workflow to write and check provider-specific filenames (e.g.,
pal-review-bedrock.md) and use that filename in the step; ensure the conditional
that runs gh pr comment verifies the marker content (or the provider-specific
filename) and emits a clear warning if the validation fails.
- Around line 162-173: Update the "Post Bedrock review comment" step to emit a
boolean step output named posted so the workflow can accurately reflect whether
a comment was actually published: convert the step to have an id (e.g., id:
post_bedrock), capture the success/failure of the gh pr comment invocation and
the pal-review.md file existence check, and write posted=true or posted=false to
GITHUB_OUTPUT (so downstream jobs can read steps.post_bedrock.outputs.posted);
apply the same pattern to the analogous post steps referenced at lines 259-269
to ensure all post-comment steps export a posted output based on the actual
publish result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5e3f43d-53ba-432e-ad48-064e754b4de3
📒 Files selected for processing (1)
.github/workflows/ai-review.yml
| - name: Post Bedrock review comment | ||
| if: steps.bedrock_review.outcome == 'success' | ||
| run: | | ||
| if [[ -f "pal-review.md" ]]; then | ||
| gh pr comment ${{ github.event.pull_request.number }} --body-file pal-review.md | ||
| echo "Posted PAL MCP review comment (Bedrock)" | ||
| else | ||
| echo "::warning::pal-review.md not found — Claude agent may not have written the review file" | ||
| fi |
There was a problem hiding this comment.
Prevent stale/mismatched review file from being posted.
Both post steps only check file existence. If pal-review.md is left from another provider path, the wrong content can be posted. Validate provider-specific marker text before posting (or use provider-specific filenames).
Suggested hardening patch
- name: Post Bedrock review comment
if: steps.bedrock_review.outcome == 'success'
run: |
- if [[ -f "pal-review.md" ]]; then
+ if [[ -f "pal-review.md" ]] && grep -Fq "via AWS Bedrock" pal-review.md; then
gh pr comment ${{ github.event.pull_request.number }} --body-file pal-review.md
echo "Posted PAL MCP review comment (Bedrock)"
else
- echo "::warning::pal-review.md not found — Claude agent may not have written the review file"
+ echo "::warning::pal-review.md missing or does not match Bedrock output"
fi
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
@@
- name: Post Anthropic review comment
if: steps.anthropic_review.outcome == 'success'
run: |
- if [[ -f "pal-review.md" ]]; then
+ if [[ -f "pal-review.md" ]] && grep -Fq "via Anthropic API - Fallback" pal-review.md; then
gh pr comment ${{ github.event.pull_request.number }} --body-file pal-review.md
echo "Posted PAL MCP review comment (Anthropic)"
else
- echo "::warning::pal-review.md not found — Claude agent may not have written the review file"
+ echo "::warning::pal-review.md missing or does not match Anthropic output"
fi
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}Also applies to: 259-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ai-review.yml around lines 162 - 170, The "Post Bedrock
review comment" step currently only checks for pal-review.md existence and can
post stale content; update this step (and the analogous step that posts other
provider reviews) to validate provider-specific marker text inside pal-review.md
(e.g., look for a header like "Provider: Bedrock" or a known Bedrock marker)
before running gh pr comment, or change the workflow to write and check
provider-specific filenames (e.g., pal-review-bedrock.md) and use that filename
in the step; ensure the conditional that runs gh pr comment verifies the marker
content (or the provider-specific filename) and emits a clear warning if the
validation fails.
| - name: Post Bedrock review comment | ||
| if: steps.bedrock_review.outcome == 'success' | ||
| run: | | ||
| if [[ -f "pal-review.md" ]]; then | ||
| gh pr comment ${{ github.event.pull_request.number }} --body-file pal-review.md | ||
| echo "Posted PAL MCP review comment (Bedrock)" | ||
| else | ||
| echo "::warning::pal-review.md not found — Claude agent may not have written the review file" | ||
| fi | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
There was a problem hiding this comment.
Track “posted” as step output for accurate final status.
Provider success does not guarantee comment publication. Emit a posted output in each post step and use that in the final summary to avoid false “posted” reporting.
Suggested status-accuracy patch
- name: Post Bedrock review comment
+ id: post_bedrock
if: steps.bedrock_review.outcome == 'success'
run: |
if [[ -f "pal-review.md" ]]; then
gh pr comment ${{ github.event.pull_request.number }} --body-file pal-review.md
echo "Posted PAL MCP review comment (Bedrock)"
+ echo "posted=true" >> "$GITHUB_OUTPUT"
else
echo "::warning::pal-review.md not found — Claude agent may not have written the review file"
+ echo "posted=false" >> "$GITHUB_OUTPUT"
fi
@@
- name: Post Anthropic review comment
+ id: post_anthropic
if: steps.anthropic_review.outcome == 'success'
run: |
if [[ -f "pal-review.md" ]]; then
gh pr comment ${{ github.event.pull_request.number }} --body-file pal-review.md
echo "Posted PAL MCP review comment (Anthropic)"
+ echo "posted=true" >> "$GITHUB_OUTPUT"
else
echo "::warning::pal-review.md not found — Claude agent may not have written the review file"
+ echo "posted=false" >> "$GITHUB_OUTPUT"
fi- if [[ "$BEDROCK_STATUS" == "success" ]] || [[ "$ANTHROPIC_STATUS" == "success" ]]; then
+ if [[ "${{ steps.post_bedrock.outputs.posted || 'false' }}" == "true" ]] || [[ "${{ steps.post_anthropic.outputs.posted || 'false' }}" == "true" ]]; then
echo "**Result**: Multi-model consensus code review posted to PR" >> $GITHUB_STEP_SUMMARY
else
- echo "**Result**: Review failed (both providers)" >> $GITHUB_STEP_SUMMARY
+ echo "**Result**: Review executed, but no PR comment was posted" >> $GITHUB_STEP_SUMMARY
fiAlso applies to: 259-269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ai-review.yml around lines 162 - 173, Update the "Post
Bedrock review comment" step to emit a boolean step output named posted so the
workflow can accurately reflect whether a comment was actually published:
convert the step to have an id (e.g., id: post_bedrock), capture the
success/failure of the gh pr comment invocation and the pal-review.md file
existence check, and write posted=true or posted=false to GITHUB_OUTPUT (so
downstream jobs can read steps.post_bedrock.outputs.posted); apply the same
pattern to the analogous post steps referenced at lines 259-269 to ensure all
post-comment steps export a posted output based on the actual publish result.
1 participant
Summary
fix_type_registry.py): Single source of truth for autofix capabilities — adding a new fix type requires only updating the registry dictruff format+ruff check --select F401,I001 --fixBEFORE LLM review, saving API tokens on issues tools can fixjq emptywithvalidate_findings_schema.pythat validates each finding against the schema and outputs sanitized JSONruff check(no--fix), pytest decoupled from ruff resultsrun_consensus_review.py(unused — workflow uses Claude Code Action directly), extracted reusable parts intofinding_utils.pyNew Files
scripts/fix_type_registry.pyscripts/finding_utils.pyvalidate_finding(),deduplicate_findings(),FINDING_SCHEMAscripts/validate_findings_schema.pyreview-findings.jsonTest Coverage
22 new tests added (30 total in file), covering:
Full suite: 1191 passed, 12 skipped, 0 failed
Test plan
python3 -m pytest tests/integration/test_nightly_review.py -v— 30/30 passruff format— all files cleanruff check— all checks passedpython3 -m pytest tests/ -v— 1191 passed, 12 skippeddry_run: trueto verify YAML syntax in CI🤖 Generated with Claude Code
Summary by Sourcery
Expand the nightly review workflow to apply deterministic code fixes before LLM analysis and centralize autofix behavior via a fix-type registry and strict schema validation for review findings.
New Features:
Enhancements:
CI:
Documentation:
Tests:
Chores:
Summary by CodeRabbit
New Features
Tests