assistant-review

name	assistant-review
description	Review code and re-review fixes until clean. Use for explicit code review or the workflow Review phase; QA runs only when required.
effort	high
triggers	[{"pattern":"fix (all |the |review |reported )?issues|fix (all |the )?findings|apply (all )?fixes","priority":90,"reminder":"This request to fix review issues matches assistant-review. You MUST read this SKILL.md and contracts/index.yaml first, then load the applicable contract selector. The skill includes fix -> validation -> re-review steps that run before the final summary."},{"pattern":"review|fresh review|code review|review this|check the code|/review","priority":80,"reminder":"This request matches assistant-review. You MUST read this SKILL.md and contracts/index.yaml first, then load the applicable contract selector. Run the autonomous review-fix loop to its exit condition before reporting."}]

Autonomous Review And QA Evaluation

Contracts

Canonical input, output, phase-gate, and handoff schemas remain authoritative at their enforcement points. Read contracts/index.yaml first; do not load every contract at entry.

• entry: contracts/input.yaml at review entry selects review_scope, review_material_snapshot, scope_size, mode, change_kind, semantic_contract_review_required, behavioral_contract_review_required, and agentic_loop_safety_review_required.
• current_round: load the active round step from contracts/phase-gates.yaml at each transition.
• selected_handoff: select the compact dispatch pointer from contracts/handoffs.yaml before Reviewer or QAEvaluator dispatch.
• reviewer_context: resolve the fresh bounded Reviewer bundle only when a review pass begins.
• return_validation: select the canonical return pointer only after a worker/direct-fallback result exists.
• completion: load the applicable contracts/output.yaml artifacts at completion, before the final review exit.

Selectors use unique id, canonical path, exact section/key, and explicit or allowed runtime names. Entry declares no immediate principles, checklist, or rubric references.

If a selector is missing or invalid, apply load_full_authoritative_file: load the full named canonical file, validate the applicable rules, and record any recovery before proceeding.

Run autonomously until clean or the cap, keep intermediate results inside the loop, and present one final result. Required QA follows build/test and code-review evidence.

Goal

Find evidence-backed defects, regressions, and test gaps; fix them in review-fix mode; and return one policy-safe result. Required QA independently evaluates acceptance, evidence, scoped quality, progression, and readiness.

Success Criteria

• Scope, mode, and review material are resolved before the loop.
• Findings are severity-ranked with evidence and confidence.
• Every review applies the SOLID, KISS, DRY, YAGNI, and readability lens from references/review-principles.md.
• In review-fix mode, must-fix and should-fix findings are addressed or explicitly deferred.
• Validation and a fresh review follow fixes.
• QA evaluation runs after code-review/build evidence when qa_evaluation_mode=required, returns score progression and a final acceptance verdict, and does not replace code-reviewer.
• QA required positive triggers: explicit QA/acceptance evaluation request, accepted Done Contract, harness-capable acceptance scope, domain-scored scope, or scoped UI/visual/product/UX/docs/DX acceptance.
• QA non-triggers: template labels/placeholders, generic acceptance criteria labels, optional/not_required reasons, delegation/source-changing work alone, and ordinary medium+ code-review-only/source-changing work.
• QA evaluation loads references/domain-rubrics.md only when domain_context, explicit rubric_refs, or subjective/UI/visual/product/UX/docs/DX/domain acceptance criteria require scoped domain-quality scoring.

Constraints

• Default to audit mode when the user asks to provide, report, list, or summarize findings.
• Do not emit intermediate review summaries; present one final summary after loop exit.
• Use concrete risk categories for refactor-related findings.
• Treat clean-code principles as evidence lenses, not acronym-driven style rules.
• Keep QA evaluation separate from code review: QA focuses on acceptance criteria, Done Contract, verification evidence, UI/visual/product/UX/docs/DX/domain quality, score progression, and final result. Code Reviewer continues to own code defects, security, architecture, and test-coverage review.

Entry

Prefer explicit files/content/diff, then uncommitted changes, then the active task journal or packet, then requested current-file audit. Ask only when no review material can be determined.

A standalone review this with no carried workflow evidence performs Spec Review against the user request and user scope, records a PASS evidence pointer before Reviewer dispatch, and blocks dispatch on FAIL until the scope mismatch is fixed and Spec Review passes. Standalone review does not require a task journal; task_journal_path remains optional.

A workflow-composed review consumes the carried Spec Review PASS pointer and carried current build/test evidence. After any source fix, every subsequent Reviewer dispatch requires real current passed build/test evidence; a not-applicable marker is invalid.

Review Modes

Use the smallest applicable combination: spec, regression, test, maintainability, bugfix evidence, semantic contract, behavioral contract, agentic loop safety, and security. Contract and loop modes are enabled by the three entry flags; security-sensitive surfaces route to assistant-security.

Findings include severity (must-fix, should-fix, or nit), file/line evidence, concrete impact, smallest useful fix, and evidence-calibrated confidence. Speculative concerns remain non-blocking Observations.

QA evaluation runs after code-review/build evidence and only when qa_evaluation_mode=required. Load references/qa-evaluation-loop.md at that later boundary. Load references/domain-rubrics.md only when acceptance criteria, Done Contract, domain_context, or explicit rubric_refs scope domain quality; selected_domain_rubrics/domain_quality_scores when scoped. Code Reviewer still owns code defects, security, architecture, and test coverage.

Company-Safe Review Rules

Prefer local diffs and repo-native checks. Do not require external scanners, remote review, or unapproved installs; redact secrets and proprietary data. Offer local/manual equivalents when policy blocks an external scan.

Mandatory Review Checklists

The fresh Reviewer context bundle points to references/review-checklists.md and supplies only the applicable checklist sections. Checklist headings alone are not evidence; each selected area produces findings or an explicit "no concrete risk found" check.

• Agentic loop flag -> Agentic Loop Safety Checklist -> agentic_loop_safety_checks.
• Behavioral flag -> Behavioral Contract Review Checklist -> behavioral_contract_checks.
• Semantic flag -> Semantic Contract Review Checklist -> semantic_contract_checks.

Refactor-Related Findings

Use refactor-related findings only for concrete actionable risk. Allowed risk categories:

• correctness
• security
• unsafe change surface
• branching/responsibility growth
• hidden dependency/ownership
• brittle testing
• poor extension seam
• readability/maintainability drag

Every refactor-related finding MUST state the risk category, affected surface, evidence from the review material, and the smallest durable fix that addresses the risk within the normal finding text.

Use concrete risk framing instead of generic convention, style, cleanliness, or improvement language. Request broad cleanup only when a smaller durable fix cannot remove the risk.

Principle and Readability Lens

The fresh Reviewer bundle includes references/review-principles.md for the SOLID, KISS, DRY, YAGNI, and readability lens. Report a principle finding only when evidence shows concrete correctness, coupling, duplication, complexity, or maintainability risk.

For each principle/readability finding, include the violated lens, affected surface, concrete evidence, risk category, and smallest durable fix. Do not report acronym-only findings such as "violates SOLID" without naming the observed behavior and the user-facing or maintainer risk.

Review Loop Routing

After entry fields are resolved, load references/review-loop.md before the first REVIEW step. It is the only immediately mandatory first-review reference in orchestrator context and owns REVIEW -> EVALUATE -> FIX -> VALIDATE, fresh bundle construction, drift/pivot handling, and the max 10 rounds limit. Principles, applicable checklist sections, and rubric guidance belong to the fresh Reviewer worker bundle created only when a review pass begins; they are not assistant-review entry dependencies.

Run QA only when qa_evaluation_mode=required. The loop routes to references/qa-evaluation-loop.md after build/test and code-review evidence exist; QA evaluates acceptance and scoped quality, and does not replace code review.

Exit: Present Final Result

After the loop completes, present one summary with rounds, CLEAN/ISSUES_FIXED/HAS_REMAINING_ITEMS, final rubric and score progression when applicable, fixed findings, remaining items, nits, verification, and residual risk. Canonical field ownership remains in contracts/output.yaml.

Rules

• Keep round results internal and present one final summary.
• Use a fresh Reviewer bundle each medium+ round; direct fallback uses the same isolated bundle without claiming a subagent dispatch.
• Previously fixed items are not re-reported; findings remain evidence-backed.
• A trivial clean review may exit after one round. Otherwise continue until clean, blocked, or round 10 is terminal; never start round 11.

Output

Return rounds/result, evidence-backed findings and fixes, verification, applicable bugfix/agentic/behavioral/semantic checks, required QA result, and residual risk. contracts/output.yaml owns the exact schema.

Stop Rules

• In audit mode, stop after the first review round and report findings without edits.
• In review-fix mode, stop only when clean, blocked, or max rounds is reached.
• Stop and report a blocker if required review material is unavailable or empty.

Drift detection (medium+ scope)

Compare rounds using references/score-tracking.md. GENUINE/SUSPICIOUS/NEUTRAL continue with recorded evidence; DRIFT resets fresh context once; REGRESSION and STAGNATION are investigated. Repeated DRIFT/REGRESSION or STAGNATION returns pivot_restart_signal; the orchestrator records pivot_restart_decision before another pass.

Review Finding Rule Distillation

At the end of review, load references/review-finding-permanent-rule.md for every blocker or must-fix finding. Classify each as one_off_fix, permanent_rule_candidate, or no_action. Promote only recurring process gaps, fake-pass eval gaps, missing contracts, missing checklists, or high-impact repeatable failure modes; do not promote style nits or one-off file-specific issues into broad rules.