Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/REVIEW.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Reserve 🔴 for things that would page someone or block a rollback. In this cod
- A Redis data shape used by both versions changes in place. New shapes need a new key namespace.
- A migration is not backward-compatible with the prior image.
- **Schema / migration safety.** Prisma migrations must be backward-compatible with the prior deploy. Adding NOT NULL without a default, dropping a column an old image still reads, renaming a column — all 🔴.
- **ClickHouse migration ordering + idempotency.** Goose runs in strict mode in the deploy pipeline and refuses to apply a missing version below the current version — slotting a new file in below the latest already-applied version blocks the deploy. New ClickHouse migration files MUST use the next available number (`max(files in internal-packages/clickhouse/schema/) + 1`); if main has added migrations while you've been on a branch, renumber yours. DDL must also be idempotent (`ADD COLUMN IF NOT EXISTS`, `DROP COLUMN IF EXISTS`, `CREATE TABLE IF NOT EXISTS`, `ADD INDEX IF NOT EXISTS`) so a partial / `--allow-missing` apply elsewhere doesn't fail on retry. Either fault is 🔴 — both break test/prod deploys. Rules live in `internal-packages/clickhouse/CLAUDE.md`.
- **Queue / concurrency correctness.** RunQueue, MarQS (V1, legacy), redis-worker — any change to enqueue / dequeue / locking semantics. Re-derive the invariant on paper before flagging or accepting.
- **Missing index on a hot table.** New Prisma queries against `TaskRun`, `TaskRunExecutionSnapshot`, `JobRun`, `Project`, etc. must use an existing index. Check `internal-packages/database/prisma/schema.prisma` for the relevant `@@index` lines — don't guess and don't propose `EXPLAIN`.
- **Recovery-path queries.** Any `TaskRun.findFirst` / `findMany` added to a schedule, run-recovery, or restart loop. Recovery fan-outs (Redis crash, restart storms) turn "rare indexed query" into a DB incident. 🔴 even if indexed.
Expand Down
34 changes: 31 additions & 3 deletions internal-packages/clickhouse/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,46 @@

## Migrations

Goose-format SQL migrations live in `schema/`. Create new numbered files:
Goose-format SQL migrations live in `schema/`. Two rules below are load-bearing — both can block a deploy.

### Rule 1: number to `max + 1`, never slot in

Goose runs in strict mode in the deploy pipeline. If a migration file numbered *below* the version currently recorded in `goose_db_version` ever shows up, goose refuses to apply it and the deploy fails:

```
goose run: error: found 1 missing migrations before current version 30:
version 29: 029_add_task_kind_to_task_runs_v2.sql
```
Comment thread
ericallam marked this conversation as resolved.

When adding a migration:

1. Look at `schema/` and take the largest existing number, call it `N`.
2. Name your file `0(N+1)_descriptive_name.sql`.
3. If you've been on a branch while main added migrations, **rebase and renumber** before opening the PR — a file numbered below the new max will block the next deploy after your PR merges.

### Rule 2: DDL must be idempotent

Migrations can be applied out of order in some environments (`goose up --allow-missing` for local recovery, manual fixups, etc.) and may be retried. Always use idempotent forms so a re-apply is a no-op:

```sql
-- +goose Up
ALTER TABLE trigger_dev.your_table
ADD COLUMN new_column String DEFAULT '';
ADD COLUMN IF NOT EXISTS new_column String DEFAULT '';

-- +goose Down
ALTER TABLE trigger_dev.your_table
DROP COLUMN new_column;
DROP COLUMN IF EXISTS new_column;
```

Equivalent forms for other DDL:

- `CREATE TABLE IF NOT EXISTS …`
- `DROP TABLE IF EXISTS …`
- `ADD INDEX IF NOT EXISTS …` / `DROP INDEX IF EXISTS …`
- `CREATE MATERIALIZED VIEW IF NOT EXISTS …` / `DROP VIEW IF EXISTS …`

ClickHouse supports `IF [NOT] EXISTS` on all of the above. Older migrations in this directory predate the rule and are not idempotent — leave them as-is unless you're explicitly hardening one.

## Naming Conventions

- `raw_` prefix for input tables (where data lands first)
Expand Down