--- name: code-review-and-quality-d2f4fb69b877 description: Multi-axis code review with optional strict maintainability mode. Use before merging any change—human, agent, or automation output. metadata: category: user-invoked disable-model-invocation: true --- Default output: return only the result, blockers, and required evidence. Omit preambles, process narration, repeated context, confidence scores, and follow-up offers. Use at most five bullets unless a required artifact or schema needs more. # Code Review and Quality Adapted from `addyosmani/agent-skills` (MIT), commit `82ceff41ed4d3c644e3dcca8a0514390b2911223`. ## Overview Review changes across five axes: correctness, readability, architecture, security, and performance. **Standard mode** asks whether the change improves the codebase without avoidable risk. **Strict maintainability mode** adds structural ambition: hunt for simplifications that delete complexity, not just rearrange it. ## When to Use - Before merging any change - After a feature, bug fix, or refactor - When validating whether tests and verification are sufficient - When the user or parent agent requests a deep maintainability audit (strict mode) ## Review modes | Mode | Goal | When | |------|------|------| | Standard | Correct, safe, readable change | Default for most PRs | | Strict maintainability | Ambitious structural quality; block spaghetti and unjustified sprawl | Large refactors, messy diffs, subagent thermo-style review | Strict mode baseline: *Rethink structure without changing behavior. Improve abstractions, modularity, and legibility. If a code-judo move could delete whole branches or layers, push for it.* ## The Five-Axis Review ### 1. Correctness - Does the change meet the task or spec? - Happy paths, edge cases, and errors handled? - Tests cover the real behavior change? ### 2. Readability and Simplicity - Clear names consistent with the codebase? - Control flow easy to follow? - Clever shortcuts that should be straightforward code? In **strict mode**, also ask: Is the implementation direct, or special-case spaghetti? Any thin wrappers or pass-through helpers that add indirection without clarity? ### 3. Architecture - Follows existing patterns? - Module boundaries still clean? - Abstraction level appropriate? In **strict mode**, also ask: - Logic in the canonical layer? Reusing existing helpers vs bespoke one-offs? - Feature logic leaking into shared paths? - File crossing **1000 lines** without strong justification—prefer extract/split first. - New ad-hoc conditionals bolted onto busy flows—prefer dedicated abstraction or module. - Sequential orchestration or non-atomic updates when parallel/atomic structure is obvious? ### 4. Security - Untrusted input validated at boundaries? - Secrets out of code and logs? - Auth, permissions, and external data handled safely? ### 5. Performance - N+1, repeated expensive work, unbounded operations? - Pagination, batching, caching, async boundaries where needed? ## Review Process ### Step 1: Understand intent What changed, why, and what proof should exist. ### Step 2: Review tests first - Regression tests for bug fixes? - Behavior verified, not private implementation? - Would tests fail if the bug returned? ### Step 3: Review implementation (five axes) Prefer concrete findings over style. In **strict mode**, prioritize: 1. Structural regressions and missed dramatic simplifications 2. Spaghetti / branching growth 3. Boundary, abstraction, and type-contract problems 4. File-size and decomposition 5. Modularity and legibility **Strict questions (when applicable):** - Code-judo move that removes concepts/branches/layers? - Refactor that moves complexity without deleting it? - Casts, `any`, `unknown`, or optionality hiding the real invariant? - Duplicate of a canonical helper or wrong package/layer? ### Step 4: Label findings - `Critical:` security, broken behavior, data loss - `Required:` must fix before merge - `Optional:` worthwhile, not blocking - `Nit:` cosmetic - `FYI:` context only Strict mode: do not soften structural issues into nits. Be direct; skip cosmetic floods when structural problems exist. ### Step 5: Verify the verification story Targeted tests, relevant suite, build/typecheck, manual checks for UI/ops. ## Findings priority (strict mode) 1. Structural regressions 2. Missed simplification / code-judo opportunities 3. Spaghetti and special-case branching 4. Boundary and type-contract problems 5. File size and decomposition 6. Modularity and legibility ## Approval bar **Standard:** no critical/required issues unresolved; relevant tests and build pass. **Strict:** behavior correct is not enough. Presumptive blockers unless clearly justified: - Plausible simplification path ignored; complexity preserved - File pushed from under to over 1000 lines - Ad-hoc branching tangling shared flows - Feature checks scattered across general modules - Unnecessary wrapper, cast-heavy contract, or wrong-layer logic - Duplicate of existing canonical helper ## Output template ```markdown ## Findings - Critical: ... - Required: ... - Optional: ... ## Verification - Tests: ... - Build: ... - Manual: ... ``` ## Red flags - No regression test for a bug fix - Large change without verification summary - Unlabeled severity in feedback - Security-sensitive changes reviewed only for style - Diffs that should have been split - (Strict) File-size jump, scattered conditionals, or refactor that only relocates complexity ## Anti-Rationalization | Excuse | Counter | |--------|---------| | "Tests pass, so it is fine" | Tests are necessary, not sufficient for architecture, security, or maintainability. | | "Too small for full review" | Small changes can have large effects. | | "I know this code" | Familiarity hides blind spots. | | "AI-generated code is probably okay" | AI output needs more scrutiny—it is often plausible and wrong. | | "We can clean up later" | Deferred cleanup rarely happens. | | (Strict) "It works, ship it" | Working code that worsens structure is a maintainability regression. | ## Verification checklist - [ ] Critical and required issues resolved or explicitly deferred with justification - [ ] Relevant tests pass; build succeeds - [ ] Review summary documents what was checked - [ ] (Strict) No presumptive blockers above without justification ## Subagent orchestration (strict mode) When invoked as a **Task subagent**, the parent supplies `### Git / diff output` and `### Changed file contents`. Apply this skill to that input only; trace cross-file impact at module boundaries. Do not spawn nested subagents unless asked. Typical parent flow: parallel shell + explore tasks for diff and file contents, then invoke `code-reviewer` in strict mode with labeled sections. --- Source attribution: v1truv1us Original project: https://github.com/v1truv1us/ai-eng-system/blob/eaff192b6e8bcb204a1f1b132b98f748a9b4572e/plugins/ai-eng-quality/skills/code-review-and-quality/SKILL.md License: MIT License information: https://opensource.org/license/mit Original upstream instructions; downloaded from Mega AI Hub. ## Upstream license notice MIT License Copyright (c) 2025 John Ferguson Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. # Mega AI Hub resource metadata Creator / publisher: v1truv1us Original project: https://github.com/v1truv1us/ai-eng-system/blob/eaff192b6e8bcb204a1f1b132b98f748a9b4572e/plugins/ai-eng-quality/skills/code-review-and-quality/SKILL.md Attribution basis: Source repository record Fields of work: Development & Engineering Roles: Backend Developer, DevOps / SRE, Frontend Developer, Full-Stack Developer, QA / Test Engineer Specialties: Backend, Frontend, Testing Tools: Tool not specified Country applicability: Country not specified Classification basis: Source catalog labels Instruction SHA-256: a9c2503401257c4170d5e82b38f76c5b7015c308825d66e9b75d809425a38bd5