Files
clawmates/skills/foundation/code-review-checklist.md
T
Omar SobhandClaude Opus 4.7 7b23f61632
ci / gates (push) Successful in 4s
ci / frontend (push) Successful in 25s
ci / rust (push) Failing after 3m41s
ci / e2e (push) Skipped
ci / publish (push) Skipped
slice 3.5c: seed 15 built-in skills across the 6 stacks
Hand-authored skill catalog anchored to real 2026-07 versions:
  - Rust 1.97.1 (stable), edition 2024
  - React 19.2.7, Server Components + Actions
  - TailwindCSS 4.3.3 (CSS-first config, Oxide engine)
  - three.js r185 (WebGPURenderer stable, BatchedMesh matured)
  - React Native 0.86 / Expo SDK 54+ (New Architecture default)
  - cargo-nextest 0.9.140, gitleaks 8.20+, cargo-audit 0.21+
  - Postgres 17 (18 in beta, don't rely on)
  - CUDA Blackwell, Metal Apple7+, ROCm CDNA3

Ships 15 skills across the categories:
  foundation/  workspace-repo-commit-protocol
               small-focused-commits
               tdd-red-green-refactor
               code-review-checklist
               int-xx-marker-protocol
               decompose-int-items
  rust/        write-rust-current-edition
               rust-error-handling
               cargo-test-driven-development
               rust-async-tokio-idioms
  backend/     postgres-migrations-forward-only
               postgres-index-selection
               api-pagination-day-1
  frontend/    react-19-server-components
               tailwind-v4-idioms
               component-4-state-model
  mobile/      expo-managed-vs-bare
               rn-flashlist-perf
  gpu/         gpu-coalescing-and-occupancy
               roofline-model
  threejs/     threejs-perf-and-teardown
  security/    cargo-audit-workflow
               secret-scanning-gitleaks

skills_loader.rs walks skills/**/*.md, parses YAML frontmatter
(name, description, when_to_use, tags), upserts via
skills_catalog::upsert_builtin. Idempotent per boot — bumps version
+ appends skill_versions row ONLY when body changes. Deterministic
sha256-derived ids so builtins are stable across boots.

Dockerfile copies skills/ to /etc/clawmates/skills. Server boot
task spawns loader alongside team_template_loader.

Follow-ups (Slice 3.5c continuation, future PRs):
  - 20-30 more skills (duckdb, shadcn composition, a11y, WebGPU
    migration, metal frame capture, rocprof, deep gitea forge
    integration, semgrep rulepacks)
  - Bind skills to team template roles (add [role.skills] refs to
    templates/teams/*.toml + wire template_role_skills population
    in team_template_loader)

Co-Authored-By: Claude Opus 4.7 <[email protected]>
2026-07-19 13:55:44 -07:00

2.4 KiB

name, description, when_to_use, tags
name description when_to_use tags
code-review-checklist A structured checklist for the reviewer role — correctness, safety, simplicity, testability, docs — with explicit approve/block markers. You are a reviewer role or performing a review pass on any code diff.
foundation
review

Code review checklist

Work top-down. A block on any earlier item stops the pass — don't score everything before opining.

1. Correctness (BLOCKS)

  • Does the change match the PLANNER's acceptance criteria for this INT-XX?
  • Do the tests exercise the behavior described in the plan, not just the code that was written?
  • Any edge cases the plan named but the code doesn't handle? (empty input, boundary values, timezones, unicode)
  • Any silent error paths (unwrap_or_default(), .ok()?) that hide real failures?

2. Safety (BLOCKS)

  • New unsafe blocks — is the justification comment concrete about the invariants?
  • New FFI — is the C-side contract checked at every call site?
  • SQL — parameterized every user input? No string concatenation into queries?
  • HTTP — validates auth, checks workspace scope on every read?

3. Simplicity (usually NIT, sometimes BLOCK)

  • Any code that a smaller function would replace? (Iterator::sum, .map, .filter)
  • Any abstraction added for one caller? BLOCK — introduce when second caller lands.
  • Any dead code, commented-out blocks, // TODO fix later without a ticket? BLOCK.
  • Files > 1500 LOC after the change? Split.

4. Performance (BLOCK on hot paths, NIT elsewhere)

  • Cold path (init, config load, admin endpoints): NIT.
  • Hot path (per-request handler, per-frame render, tight loop): BLOCK on obvious O(n²), unnecessary allocations, blocking sync in async context.

5. Testability / observability (BLOCK if downstream on-call would suffer)

  • Does the change add meaningful log lines at decision points? (Info at boundaries, warn on retries, error on give-up.)
  • Are new metrics wired? Prometheus counter/histogram naming convention followed?

6. Docs (NIT if module-private, BLOCK if public API)

  • New public function / struct / route → doc comment with example.
  • Non-obvious invariant → comment on the declaration.

Verdict markers

Emit exactly one of these on a line by itself when the pass is done:

REVIEW_APPROVE: INT-NN

or

REVIEW_BLOCK: INT-NN — <specific issue that must be fixed>

Multi-line commentary is fine, but the marker must appear literally.