Schema Review

Schema / Data-Structures Review

A rigorous, multi-lens review engine for persistence schemas (Ecto, Postgres DDL, migrations, dbt models) and in-code data structures (Elixir structs/typespecs, TypeScript types, collection choices). It evaluates a target across seven lenses, scores findings by severity and confidence, and produces a draft findings report. It never posts anywhere on its own.

Context-hygiene contract (read first)

This skill is an orchestration layer. The main thread must stay lean.

• Do NOT grep, glob, read large files, or trace the target schema directly in the main context.
• DO delegate all reading/analysis to sub-agents. The main thread holds only: the mode decision, sub-agent result summaries, and the final synthesized report.
• The ONE allowed exception: re-reading a specific table/column/type in the main context to validate a CRITICAL or HIGH finding before it lands in the report — high-severity claims require main-context verification of the full failure/attack chain.

Input

Invoke as /schema-review <target>. The target may be:

• A migration file or directory (.exs Ecto migration, .sql DDL, dbt model .sql)
• An Ecto schema module, a Postgres \d/pg_dump schema, a dbt schema.yml
• A TypeScript/Elixir file or module defining domain types/structs
• A PR diff (<owner>/<repo>#<number> or a local branch — diff against the base)
• A design doc / proposal describing a schema or data model (review at design altitude)
• A directory or "the changes on this branch" (git diff vs base)

Derive a <slug> from the target (e.g. attachments-migration, orders-schema). Scratch work goes under /tmp/schema-review-<slug>/.

Step 1 — Classify the target, detect dialect, pick lenses

Identify what kind of artifact the target is, then select the applicable review lenses. Not every lens applies to every target — running irrelevant lenses wastes context.

1a. Detect the target SQL dialect (do this BEFORE dispatch)

Determine two things and carry both into every dispatch:

• Target dialect — the database the schema is meant for in production: Postgres, MySQL, SQLite, Snowflake, or dialect-agnostic (a design doc with no committed engine).
• Implementation dialect — what the code in front of you actually uses right now.

When implementation ≠ target dialect, that gap IS a primary review axis. A canonical case: a SQLite POC whose canonical target is Postgres. Reviewing the SQLite DDL as-if-Postgres produces false positives (SQLite INTEGER PK is 64-bit; Postgres int4 is not) — and missing the gap produces false negatives (the SQLite DDL will silently mistranslate TEXT-for-timestamp into a broken Postgres column). Lens agents MUST be told both dialects so they (a) review against the target dialect's rules and (b) flag every place the current implementation won't translate cleanly. Surface these as a consolidated "dialect-translation decide-now" list in the report.

The seven lenses (each backed by a reference file in references/):

1. relational-postgres — normalization, keys, constraints, indexing, Postgres types, classic relational anti-patterns. → references/relational-postgres.md
2. migration-safety — zero-downtime / online-DDL safety, lock hazards, expand-contract, backfills. → references/migration-safety.md
3. data-modeling — DDD aggregates/boundaries (transactional) + Kimball dimensional / dbt layering (analytics) + data contracts. → references/data-modeling.md
4. type-design — make-illegal-states-unrepresentable, parse-don't-validate, ADTs, primitive obsession, collection/Big-O choice (Elixir + TS). → references/type-design.md
5. pg-scale-sins — int4 PK exhaustion, MVCC bloat, autovacuum/xmin, txid wraparound, TOAST, hot-row contention, partition-too-late. → references/pg-scale-sins.md
6. simplification-taxonomy — over-complication, over-normalization, lookup/junction sprawl, tree/taxonomy modeling, redundancy/drift. → references/simplification-taxonomy.md
7. conventions-and-security — naming/consistency, PII/PHI, tenant isolation (IDOR), audit columns, mass-assignment, documentation. → references/conventions-and-security.md

Step 2 — Choose execution mode

Count the discrete review objects in the target (tables, migrations, schema modules, type definitions changed).

• Single-pass (default for small targets): the review surface is a few line items or fewer — roughly ≤ 3 discrete objects, a single focused migration, or one type module. One sub-agent loads the applicable reference files and walks every relevant checklist in one pass. Cheaper, lower latency, less context.
• Multi-agent fan-out (for larger targets): the review surface exceeds a few line items — > 3 objects, multiple tables, a broad PR diff, or a whole schema. Spawn one sub-agent per applicable lens, in parallel, each scoped to its reference file. Heavier but thorough; each agent stays expert and shallow-context.

When in doubt, state your object count and the resulting mode in one line, then proceed. If a single-pass review surfaces more complexity than expected, escalate to fan-out.

Single-pass dispatch

Spawn one general-purpose sub-agent (model: sonnet) with this shape:

You are a schema reviewer. Target: <target>. Target dialect: <dialect>. Implementation dialect: <impl-dialect> — review against the TARGET dialect's rules and flag every place the implementation won't translate cleanly. Read the target, then apply EVERY checklist in these reference files: <absolute paths to the applicable references/*.md> plus references/severity-and-output.md. For each violation, emit a finding in the exact format from severity-and-output.md (severity, confidence, location, problem, impact, fix, blocker). Do not invent findings — only flag conditions that actually match the checklists against the real target. Return the findings list as your final message, ordered by severity.

Multi-agent fan-out dispatch

Spawn the applicable lens agents in a single message, in parallel (model: sonnet each). Each agent gets:

You are the reviewer. Target: <target>. Target dialect: <dialect>. Implementation dialect: <impl-dialect> — review against the TARGET dialect and flag implementation-translation gaps. Your sole knowledge source is references/<lens>.md (read it) plus the finding format in references/severity-and-output.md. Apply only your lens's checklist + anti-patterns to the real target. Emit findings in the standard format. Flag only conditions that actually match — no speculative findings. Return your findings list ordered by severity as your final message.

Each agent returns a capped findings list (not file dumps). The main thread collects them.

Step 3 — Validate high-severity findings

Before any CRITICAL or HIGH finding enters the report, verify it in the main context (the allowed context-hygiene exception): re-read the specific table/column/migration/type and confirm the full failure or attack chain is real, not a checklist false-positive. Demote or drop anything that doesn't survive verification. This is mandatory — unverified high-severity findings are the most expensive kind of false positive.

Step 4 — Synthesize the report

4a. Cross-lens dedup pass (mandatory in fan-out mode)

Lenses overlap by design, so the same object will surface from multiple agents (e.g. an int PK flagged by both relational-postgres and pg-scale-sins; a missing tenant column flagged by conventions-and-security, relational-postgres, and data-modeling). Before scoring, collapse duplicates:

• One object/defect → one finding, citing every lens that flagged it (_(lens: a, b, c)_).
• Take the highest severity among the duplicates, but treat multi-lens corroboration as a confidence signal — a defect three lenses independently caught is rarely a false positive.
• Keep distinct angles on the same column separate (e.g. _raw_record as a redundancy/drift finding vs. _raw_record as a TOAST/bloat finding are two real findings, not a dup).

4b. Score

Compute a schema-health score: start at 100, subtract per the penalty table below.

Floor the score at 0. Lead the report with: score, mode used, lenses run, and a one-line verdict (e.g. HARD BLOCK if any CRITICAL, REVIEW if any HIGH, RISKY if only MEDIUM, SAFE otherwise).

4c. WIP / design-phase mode (suppress the punitive score)

If the target is a design doc, a pre-implementation proposal, or an explicitly work-in-progress POC — detect via: a design-doc/proposal target type, "not yet started" / "draft" / "WIP" language in the artifact, or an explicit user signal — the 0–100 score is misleading. A sound architecture mid-build will floor at 0 purely on decide-now items, which reads as "broken" when it isn't. In this mode:

• Suppress the numeric score (or render it — (WIP, suppressed)); the verdict becomes DESIGN-PHASE — decide-now items instead of HARD BLOCK.
• Lead with severity counts + the consolidated decide-now list (especially the dialect-translation table from Step 1a), not a grade.
• Add a one-line context note explaining why the score is suppressed, so the harshness isn't mistaken for a failing implementation.

This keeps the signal (what to fix before cutover) without the noise of a punitive grade on intentionally-unfinished work.

Write the report using templates/findings-report.md to:

• A PR diff target → /tmp/schema-review-<repo>-<pr>.md (draft PR comment; never auto-post)
• Any other target, if you want it durable → offer to save to ./schema-review-<slug>-<YYYY-MM-DD>.md. Otherwise print to terminal.

Output discipline

• Draft-only. Never auto-post to a PR, issue tracker, or chat. Surface the draft; the user decides.
• Findings state facts and impacts — never mock or blame prior authors, even casually.
• Recommendations are imperative and concrete ("Add CREATE INDEX CONCURRENTLY ..."), not hedged ("you might consider").
• No local file paths in anything destined for external comms — inline the DDL/snippet instead.

Memory persistence

After a substantive review, OFFER (don't auto-run) to store durable findings to the memory graph via /mem — recurring anti-patterns in a repo, a schema-design decision and its rationale, or a confirmed at-scale landmine. Don't store one-off cosmetic nits.

Failure modes to avoid

• Don't run all seven lenses blindly — classify the target first; irrelevant lenses produce noise and burn context.
• Don't trust the checklist over the code — a checklist match is a hypothesis; verify CRITICAL/HIGH against the real target before reporting.
• Don't escalate severity to compensate for low confidence — lower confidence lowers the severity floor. Label confidence explicitly.
• Don't review in the main context — delegate reading to sub-agents; only validation re-reads happen in main.
• Don't auto-post anything. Ever. Draft and surface.
• Don't recommend a rewrite when a targeted fix suffices — pragmatic, incremental corrections win; surface a structural pivot only when the schema is genuinely wrong for the requirement, and let the user choose.