Suede Code Review

Suede Code Review

Review code with full context: changed files, callers, contracts, deploy surface. Find real breakage. Rank by production impact. Every finding has a file, evidence, and a fix path. No findings without evidence. No volume without signal.

Model Routing

Default: Sonnet. Recommend Opus for auth, payments, and public API surface reviews.

Operating Stance

• Review current source, current diff, local docs, and relevant runtime behavior.
• Keep code generation and review separate by default. If you authored the code,

switch into review mode and look for what your implementation would miss.

• Preserve user and other-agent WIP. Do not stage, revert, or rewrite unrelated

files.

• Prefer high-signal findings over volume. Do not leave style nits when formatters

or local conventions already handle them.

• Every blocking finding needs evidence, impact, and a concrete fix path.

Review Contract

Before review, identify:

• target: repo, branch, PR, commit range, diff, route, API, or release build;
• intent: what the change claims to accomplish;
• risk lanes: frontend, backend, data, auth, payments, contracts, iOS, release,

public copy, analytics, secrets, deployment, and docs.

Context Graph

Build a lightweight graph before judging the diff:

1. Changed files and generated files.
2. Imports, callers, routes, API handlers, jobs, hooks, models, schemas, and

config/env dependencies touched by the change.

1. Tests, fixtures, migrations, release scripts, docs, and screenshots that

should move with the behavior.

1. Runtime surfaces: local route, live URL, API endpoint, simulator flow,

dashboard, App Store metadata, or deployment target.

1. Suede domain contracts: creator ownership, rights/provenance, registry-backed

media, royalty routing, agent commerce, wallet/payment flows, and public claim truth.

Flag beyond-the-diff risks when related files, defaults, docs, env, or deploy requirements no longer agree.

Run the Repo's Own Gates

Do not hand-review for what a tool already decides. Before manual analysis, run the gates the repo already ships and fold the output into findings. Detect what exists from package.json scripts, config files, and lockfiles — run only those. Never introduce a tool the repo does not use, and never fabricate a result you did not run.

• Type check: the repo's typecheck script, or the type checker directly. An error

on a changed line is at least P2; on a changed critical path, P1.

• Lint: the repo's configured linter on changed files only. Report violations the

change introduces; ignore pre-existing noise outside the diff.

• Tests: run the suite, or the changed-file subset. A failing test on changed

behavior is P1; a test that silently stopped running is P2.

• Dependency CVEs: the repo's dependency auditor — the package manager's audit

command, or a vulnerability scanner — when dependencies changed. A known-exploitable CVE reachable in a production path is P0/P1.

• Secret scan: a real entropy-based secret scanner over the diff when the repo

provides one — it catches keys the Commit Dirt Score patterns miss. A verified live secret is P0.

• Build: for release reviews, the production build must pass. A broken build is P0.

Gate Commands by Stack

The categories above are universal; the actual command differs by surface. Use this as a reference, not a checklist — detect which of these apply to the target repo, run only what exists, and never fabricate a result you did not run.

| Stack | Type check | Lint | Test | Other | |---|---|---|---|---| | Web / Node (TS/JS) | npx tsc --noEmit | npm run lint | npm run test | npm audit for dependency CVEs | | MCP server (Node) | node --check .mjs | repo's configured linter, if any | — | JSON-RPC smoke test: pipe a minimal request over stdio, e.g. echo '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}' \| node .mjs and confirm a well-formed response | | iOS / Swift (Xcode) | — (compiler check is the build) | SwiftLint, if configured | XCTest target, if present | xcodebuild -project X.xcodeproj -scheme X -destination 'platform=iOS Simulator,name=iPhone 16' build | | API / backend (generic) | language's own type/compile step, if any | repo's configured linter | contract or schema test — e.g. OpenAPI/schema validation against the live route, or the repo's own contract-test suite | — |

Cite the command, its exit status, and the file:line it implicates. If a gate cannot run (no script, missing deps, sandboxed), say so in Verification — never report a gate as passed that you did not execute.

Project Rules and Learnings

A review that fights the house style produces noise, not signal. Honor what the repo already encodes.

• Read the rules first: before judging, read CLAUDE.md, AGENTS.md,

CONTRIBUTING.md, .editorconfig, formatter/linter config, and any review-config file at the repo root or in the changed directories. A documented convention is binding — do not flag what a rule permits; do flag what it forbids.

• Most-specific rule wins: a rule in a subdirectory config or a nearest-ancestor

AGENTS.md overrides a repo-root rule for files under that path.

• Record learnings: when the user dismisses a finding as a false positive or a

deliberate house pattern, capture it in one line — pattern, why it is allowed, path scope — and do not re-raise that pattern this review or in later ones. Repeat nitpicks erode trust faster than a missed P3.

• No rule from silence: the absence of a convention is not license to impose a

personal preference. A style choice not governed by a rule, a formatter, or a real cost is not a finding.

Review Modes

• Fast diff review: small change, narrow blast radius, focused findings.
• Deep PR review: multi-file behavior, public surface, data/auth/payment,

release, or cross-repo risk.

• Plan review: implementation has not started; inspect scope, sequencing,

acceptance criteria, test mapping, and missing decisions.

• Fix verification: review after fixes; rescan changed files and confirm the

original finding is gone.

• Release review: validate build, secrets, env, public copy, screenshots,

metadata, deployment, and live/API readback.

Review Depth Levels

Add a --depth modifier to any review:

• --quick (~2 min): Pattern-based scan. Flag obvious bugs, hardcoded

secrets, missing null checks, SQL/command injection patterns, broken error handling. No cross-file analysis. Use for PRs with narrow blast radius.

• --standard (default, ~10 min): Per-file analysis: correctness on changed paths, language-specific traps (see checklists below), state handling, test coverage on changed behavior, call graph within changed files. Default for all PRs unless the user says otherwise.
• --deep (~25 min): Cross-file analysis including full import graph and

call chain tracing. Finds semantic bugs that only appear when you follow data across module boundaries. Use for auth changes, payment flows, data migrations, and public API changes.

State depth level at the top of every review output.

TypeScript Traps

Check these on every TypeScript file in the diff:

• any vs unknown: any disables type checking for everything downstream. If the type is truly unknown at the call site, use unknown and narrow with a type guard. Flag every any annotation that isn't in a third-party type shim.
• Non-null assertions (!): Each foo!.bar is a runtime crash waiting for the condition that makes foo null. Flag unless the null case is provably eliminated by a guard two lines above.
• Missing discriminated unions: When a function returns { type: 'a', ... } | { type: 'b', ... }, the switch/if must be exhaustive. Missing default: assertNever(x) is a silent future bug.
• Unsafe casts (as Foo): A cast without a preceding type guard means "trust me." Flag every as that isn't a DOM cast (as HTMLInputElement) or a narrow type refinement proven by the preceding condition.
• Object.keys() without keyof typeof: Object.keys(obj).forEach(k => obj[k]) fails type checking silently at runtime when keys are typed. Require (Object.keys(obj) as Array) or a typed Object.entries().
• Promise without await in an async function: Returns the Promise object instead of the resolved value. Flag any return someAsyncFn() inside an async that should return await someAsyncFn().

React Traps

Check these on every React component or hook in the diff:

• Missing useEffect dependency array: useEffect(fn) (no array) runs on every render. useEffect(fn, []) runs once. useEffect(fn, [dep]) runs when dep changes. Flag any effect where the deps array is absent or obviously incomplete (function references, object literals, values used inside the effect but not listed).
• Stale closures: An effect or callback captures a value at mount time and never re-captures it. Most common pattern: a setInterval inside useEffect(fn, []) that reads a stateful value. The fix is either adding the dep or using a ref.
• Missing key props: Any .map() returning JSX elements must have a stable, unique key. Using array index as key is a bug when the list can reorder or filter. Flag index-keyed lists where items have identity (id, slug, etc.).
• Unnecessary re-renders: Flag components that receive object or function props without useMemo/useCallback when those objects are created inline in the parent render. Flag components that could be wrapped in React.memo but aren't, when rendered in tight loops or high-frequency update paths.
• Prop drilling past 2 levels: If a prop is passed through 2+ components without being used at intermediate levels, flag as P3. The fix is context, Zustand, or component composition. Note which fits the existing pattern in this repo.
• State mutation without setter: arr.push(item) on state does not trigger a re-render. Flag any direct mutation of state variables.

Accessibility Traps

Check these on every React component or HTML-producing file in the diff:

• Missing alt text: ` without alt, or alt=""` on non-decorative images. Empty alt skips the element for screen readers; flag when the image conveys content.
• Interactive elements without accessible names: `, , with no visible text, no aria-label, and no aria-labelledby`. An icon-only button with no label is invisible to assistive tech.
• Wrong element semantics: ` used as a button; used as a link; heading levels skipped (h1 → h3) or used for visual styling instead of document hierarchy. Use the correct element or add role=` with keyboard handlers.
• Missing focus management: modals, drawers, toasts, and route transitions that don't trap or restore focus. When a modal opens, focus must move inside it; when it closes, focus must return to the trigger.
• Keyboard inaccessibility: custom components (dropdowns, date pickers, carousels) that handle onClick but not onKeyDown (Enter, Space, arrow keys). Flag interactive elements that can't be reached or operated by keyboard alone.
• Color contrast: inline styles or Tailwind classes that set foreground/background color combinations. Flag likely failures: text-gray-400 on white, text-white on light-colored buttons, any low-contrast pairing below ~4.5:1 for body text or ~3:1 for large text.
• Missing form labels: ` or without an associated (via htmlFor/id) or aria-label`. Placeholder text is not a label.
• ARIA misuse: aria-hidden="true" on interactive elements (traps keyboard users); role="presentation" on semantically meaningful elements; ARIA roles that contradict the native element's semantics.

Next.js Traps

Check these on every Next.js file in the diff:

• Server/client boundary: Any file with 'use client' cannot import server-only modules (DB clients, fs, crypto, server-side env vars). Any file without 'use client' that uses useState, useEffect, browser globals (window, document), or event handlers is a runtime crash. Flag the mismatch, not just the symptom.
• Missing Suspense boundary: Async server components that fetch data and are rendered inside a client component tree need a `` wrapper. Missing boundaries cause the entire parent tree to suspend without a fallback.
• Missing error.tsx / loading.tsx: Any new route segment that fetches data or can throw should have both. Flag their absence as P2 when the route is user-facing.
• Server-only secrets in client bundle: process.env.SECRET_KEY in a 'use client' file or in a prop passed from server to client component is exposed in the browser bundle. Only NEXT_PUBLIC_* vars are safe client-side. Flag any non-NEXT_PUBLIC_ env var referenced in client code.
• Unguarded generateMetadata / getServerSideProps fetches: These run on every request. An uncached external fetch here is a latency and cost bomb. Flag missing { next: { revalidate: N } } or unstable_cache wrapping.
• useRouter from next/router in App Router: App Router uses next/navigation. Importing from next/router in an App Router project silently fails or returns stale data. Flag the wrong import.

SEO Impact

Check on any file that touches routes, layouts, metadata, or public-facing content:

• Metadata regression: changes to generateMetadata, `, , description, og:title, og:description, og:image, twitter:card. Any removal or blanking of previously populated fields is a regression. Flag if generateMetadata` now returns fewer keys than before the change.
• Canonical drift: canonical URL changed, removed, or now pointing to a different domain. Flag any change to canonical logic — canonical changes can consolidate or fragment link equity unintentionally.
• Robots / noindex added unintentionally: noindex, nofollow, or X-Robots-Tag: noindex appearing on pages that were previously indexable. Flag any new robots metadata that restricts crawling on a route that wasn't restricted before.
• Sitemap impact: new routes not added to sitemap; removed routes not pruned; sitemap.ts / sitemap.xml not updated when routes change. Flag route additions or removals without a corresponding sitemap change.
• OG image regression: og:image URLs broken, pointing to localhost, missing dimension params, or removed from previously covered pages. Flag layout-level changes that remove OG image generation entirely.
• Structured data / JSON-LD drift: schema.org markup changed, fields removed, or @type changed. Flag removals of @type, name, url, or description from any existing structured data block.
• URL structure changes without redirects: route renames, slug changes, or path restructuring without a corresponding 301. A renamed route without a redirect is a hard 404 for crawlers and any existing backlinks.
• llms.txt / AI discoverability: if the repo includes llms.txt or llms-full.txt, flag any changes that expand or restrict what AI crawlers are permitted to see.

Database Traps (Drizzle / Prisma)

Check these on any file that touches DB queries:

• N+1 queries: A loop that issues a query per iteration. The fix is a single query with WHERE id IN (...) or a join. Flag any .map() or for loop that calls db.query(), prisma.find*(), or db.select() inside the body.
• Missing index on filtered/sorted columns: Any WHERE, ORDER BY, GROUP BY, or join condition on a column that isn't indexed is a full-table scan. Flag new query predicates on columns with no corresponding index in the migration/schema.
• Multi-table writes without transactions: Two or more INSERT/UPDATE/DELETE calls that must succeed or fail together. Flag any multi-table write that isn't wrapped in db.transaction() / prisma.$transaction().
• **Missing uni

…

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: JasonColapietro
• Source: JasonColapietro/suede-creator-skills
• License: MIT
• Homepage: https://jasoncolapietro.github.io/suede-creator-skills/

Install and usage instructions live in the source repository linked above.