# sql-review

A Claude Code skill that reviews SQL code against 10 rules and writes a fix plan to `query_tracker.md`. It never edits code and never runs queries.

Tuned for **SQL Server + pyodbc** (raw SQL strings in Python, `.sql` files, `CREATE INDEX` definitions). Other databases work too; the rules are applied in that dialect.

## Install

Copy this folder to `~/.claude/skills/sql-review/` and start a new Claude Code session.

## Use

In any project, type one of these:

- `/sql-review`
- `/sql-review src/superbill/repositories`
- "SQL check pannu" / "review the queries in run_repo.py"

## What it does

1. Finds the files that contain SQL. Tests and one-off migration scripts are skipped.
2. Hands the review to a **subagent**, so file dumps stay out of the main chat. With more than 10 files it runs 2-3 subagents in parallel.
3. Opens every HIGH finding at its `file:line` and confirms it. Findings it can't confirm are dropped.
4. Writes `query_tracker.md` at the project root.
5. Replies in chat with a short summary and a link to the tracker.

## The 10 rules

| # | Rule | Severity |
|---|---|---|
| 1 | Formatting: uppercase keywords, one clause per line | STYLE |
| 2 | No `SELECT *`; page unbounded lists with `TOP` / `OFFSET FETCH` | STYLE |
| 3 | CTEs instead of nested subqueries (readability only) | STYLE |
| 4 | Index-friendly WHERE: no function around a column | PERF |
| 5 | `DISTINCT` must not hide a bad join; use `EXISTS` | WRONG-RESULT |
| 6 | `COUNT(*)` for total rows, `COUNT(col)` only to skip NULLs | WRONG-RESULT |
| 7 | Parameterized queries (`?`): no f-string values | HIGH |
| 8 | Composite index column order matches the WHERE | PERF |
| 9 | Set-based writes: `executemany`, no per-row loops, no N+1 | PERF |
| 10 | `UNION ALL` when duplicates can't happen | STYLE |

## query_tracker.md

It opens with a 10-row summary table. After that comes one block per change, ordered HIGH, then WRONG-RESULT, then PERF, then STYLE:

````markdown
### Q1. SQL injection in orders_for_customer (rule 7, HIGH)
- [ ] Status: todo
- Where: `orders_repo.py:17`
- Problem: customer_id goes into the SQL through an f-string.
- Before:
  ```sql
  f"... WHERE CustomerId = {customer_id}"
  ```
- After:
  ```sql
  "... WHERE CustomerId = ?", customer_id
  ```
- Check: a valid id returns the same row count as before.
````

Running the skill again updates the tracker without overwriting it:
- Existing Q numbers and their `[x]` marks are kept.
- Items that are fixed are marked "resolved".
- New findings get the next free Q number.

To apply a fix, tell Claude which item, for example "Q1 fix pannu".

## Safety

- It is read-only: it doesn't change code and doesn't connect to any database.
- `query_tracker.md` is the only file it writes.
- It doesn't guess table sizes. Findings that only matter on large tables say so.

## Tested

On 2026-10-21 the skill ran against two sample repos:

| Sample | Result |
|---|---|
| A repo with one planted bug per rule | Found all 10, each with a correct `file:line` |
| A clean repo with traps: whitelisted f-string column names, `IN (?, ?)` placeholder lists, `COUNT(DISTINCT x)` | 0 false alarms |

The update-on-re-run behaviour has not been tested yet.