Code Review and Quality

Code Review and Quality is a merge-gate skill: it tells an agent how to review a change across five dimensions, how to label findings by severity, and when to approve. It is part of addyosmani/agent-skills, a 25-skill engineering library maintained by Addy Osmani.

The skill is not a linter and not a security audit. It is a procedure for reading a diff like a senior reviewer, with explicit standards for what blocks a merge and what is merely a suggestion.

Source and revision

What the skill actually does

The review has five axes:

1. Correctness. Does the change match the spec, handle edge cases and error paths, pass tests that actually test the behavior, and avoid off-by-one, race, or state bugs?
2. Readability and simplicity. Are names descriptive, control flow direct, abstractions earned, dead artifacts removed? The skill treats a new conditional bolted onto an unrelated flow as a design smell, not a nit.
3. Architecture. Does the change fit existing patterns, keep module boundaries clean, avoid circular dependencies, and reduce complexity rather than relocate it? It asks whether a refactor makes whole branches disappear or just moves the same concepts around.
4. Security. Input validation, secrets, auth checks, parameterized SQL, output encoding, dependency trust, and external data treated as untrusted. The detailed checklist is in the repo-level references/security-checklist.md.
5. Performance. N+1 queries, unbounded loops, synchronous work that should be async, unnecessary re-renders, missing pagination, and large objects in hot paths. The detailed checklist is in references/performance-checklist.md.

The approval standard is deliberately not "perfect." Approve when the change definitely improves overall code health and follows project conventions, even if it is not how you would have written it.

Review mechanics that make it useful

Severity labels. Every comment gets a prefix: no prefix means required before merge; Critical: blocks merge; Nit: is optional; Optional: or Consider: is a suggestion; FYI needs no action. This prevents an author from treating every comment as mandatory.

Lead with leverage. Findings are ordered correctness and security first, then structural regressions and missed simplifications, then cosmetics. One structural problem outranks ten nits.

Test the tests. The skill asks the reviewer to invert one condition the change adds, run the suite, and restore the file. A mutation that stays green is a finding: the test that should have caught it is missing. This is a concrete review action, not a coverage-number ritual.

Change sizing. Around 100 changed lines is easy to review; around 300 is acceptable for one logical change; around 1,000 is too large. The skill also watches total file size: a small diff that pushes a file past roughly 1,000 lines should trigger decomposition first.

Dependency upgrades. Read the changelog, upgrade one dependency per change, require a green suite before and after, review the lockfile diff, and never hand-edit the lockfile. A bulk "bump deps" pull request hides which package broke the build.

Dead code hygiene. After a refactor, list orphaned code and ask before deleting it. The skill does not want silent deletion of things the reviewer is unsure about.

Illustrative review

This is a constructed diff and expected review shape, not a run against a repository.

// Change adds a customer list endpoint
export async function listCustomers(orgId: string) {
  const customers = await db.customer.findMany({ where: { orgId } });
  const enriched = [];
  for (const customer of customers) {
    const orders = await db.order.findMany({ where: { customerId: customer.id } });
    enriched.push({ ...customer, orders });
  }
  return enriched;
}

Expected findings:

• Critical: the loop issues one query per customer. This is an N+1 pattern; use a single relation query or a batched lookup. The comment should quantify the expected cost if the reviewer can.
• Required: orgId comes from a request and is used directly. Confirm the caller authorizes the organization, not just filters by it.
• Required: the return type is inferred and the enriched array is untyped. Make the shape explicit so the client contract is reviewable.
• Nit: enriched is vague; customersWithOrders says what it is.

The review does not stop at "tests pass." It asks what the tests cover, whether a regression mutation would be caught, and whether the change is the right size to review at all.

When to use it

• Before merging any pull request, including one written by an agent.
• After a feature or bug fix, especially when the fix includes a regression test.
• When reviewing a dependency upgrade or a refactor that claims to simplify code.
• When you need a shared severity vocabulary so authors know what is blocking.

When not to use it

• You need a dedicated security audit or threat model. The skill hands security depth to security-and-hardening and its checklist.
• You need language-specific lint rules or formatting. Use the project's linter and formatter.
• The change is a complete file deletion or an automated refactor where line-by-line review is not the right control. The skill names those as exceptions.
• You want a rubber stamp. The skill explicitly calls out "LGTM without evidence" as a failure.

Install

npx skills add addyosmani/agent-skills --skill code-review-and-quality -a claude-code

Prerequisites and footprint

The skill is text-only, but it directs the agent to run tests and potentially mutate a source file during review. That is an execution instruction, not inert prose. Run it in a working copy, not against uncommitted work you cannot restore.

Limitations

• Not a security guarantee. The five-axis pass is broad, but the skill defers deep security to a separate checklist and skill.
• Severity is judgment. The labels are clear; deciding whether a finding is Critical or Required still depends on context.
• Mutation testing needs a runnable suite. If the project has no reliable tests, the mutation step cannot produce a meaningful result.
• Shared references break on partial installs. This is a real packaging limitation, not a hypothetical.
• Language-agnostic advice. The examples lean TypeScript and npm; substitute the repository's own commands.
• The review is only as good as the diff context. A reviewer who cannot see the spec or the callers will miss architecture problems.

Verify it yourself

1. Clone at the pinned commit and read the full skill, including "Common Rationalizations" and "Red Flags."
2. Confirm the shared checklist paths exist at the repo root, not inside the skill folder.
3. Check whether your install method copied those checklists. If not, either install the whole repo or copy them into a local references/ directory.
4. Run the skill on a known-small diff and see whether it follows the severity prefixes. If it produces an undifferentiated list, the host or model is not following the procedure.
5. For the mutation check, use a disposable branch and confirm the file is restored.

Review transparency

This is an original guide. It does not reproduce the full skill text.

Related

• Debugging and Error Recovery — the companion skill for "why is this broken"
• Browser Testing with DevTools — runtime verification for UI changes
• Code Review Agent Blueprint — a runnable agent, not a skill
• Code Review with ChatGPT — prompt-level review patterns
• How to Vet an Agent Skill — review the reviewer