Feat/129 ci migration validation

[kelp-Shake]

Summary

There was no automated check that the Alembic migrations still apply cleanly or that the schema they build matches the SQLAlchemy models, so drift could reach main unnoticed — and it already had. This adds a validate-migrations CI job that applies every migration to a fresh, empty Postgres and then compares the result against the models, failing the PR when a migration is broken or has drifted. It also fixes the one piece of drift that was already present, so the new check passes on main.

Issue

Closes #129

Changes

• core/models/user.py — declare UniqueConstraint("email", name="uq_users_email") in __table_args__. Migration 001 has always created this constraint, but the model only set unique=True, index=True, which produces a unique index instead. The constraint existed in every migrated database while being absent from the model metadata, so alembic check reported drift. This changes no database state — the constraint already exists wherever the migrations have run. I chose this over a drop-constraint migration deliberately: it keeps a CI-scoped PR from becoming a schema change against live data.
• scripts/validate_migrations.sh (new) — runs alembic upgrade head then alembic check under set -euo pipefail. It requires DATABASE_URL to use the asyncpg driver, because alembic/env.py builds an async engine and a sync URL otherwise fails with an unhelpful traceback.
• .github/workflows/ci.yml — new validate-migrations job with a Postgres 16 service container (health check and setup copied from the existing test-integration job), pointed at its own empty pathreview_migrations database.
• tests/unit/test_migrations.py (new, 8 tests) — the parts of migration health that don't need a database.

Testing

• New/updated tests cover the changes
• Unit tests pass (make test-unit) — 53 failures, all pre-existing; see below
• Integration tests pass (make test-integration) — not run
• Linter passes (make lint) — 182 errors, all pre-existing; clean on both files I touched
• Type checker passes (make typecheck) — 99 errors, all pre-existing; none in files I touched

Verified locally against a docker compose Postgres, using a scratch database:

$ DATABASE_URL=postgresql+asyncpg://...  ./scripts/validate_migrations.sh
==> Applying all migrations to a fresh database
==> Comparing the migrated schema against the models
No new upgrade operations detected.
==> Migrations apply cleanly and match the models

I also confirmed the check goes red, not just green:

1. Without the model fix, it fails with remove_constraint uq_users_email — the original drift, so the fix is what makes it pass.
2. With drift added on purpose (an extra column on the Profile model and no migration), it fails with add_column ... and exits non-zero, which is what fails the job. Reverted afterwards.
3. Guard clauses: an unset DATABASE_URL and a sync postgresql:// URL both exit 1 with an explanatory message.

Pre-existing failures

This branch does not make make check or make test-unit green, because they aren't green on main. I recorded a baseline before my changes and compared after:

Check	main (unmodified)	This branch
ruff check .	182 errors	182 errors, identical per file
black --check .	52 files	52 files, identical
pytest tests/unit -m unit	53 failed, 375 passed	53 failed, 383 passed

The 53 failures are the same test ids before and after, and the 8 additional passes are the new tests. ruff and black are clean on both files I touched. These changes introduce no new failures.

These numbers line up with CI: the lint job reports the same 182 ruff errors, and test-unit reports the same 53 failed / 383 passed. None of the lint or typecheck output names core/models/user.py or tests/unit/test_migrations.py. The pinned pre-commit hooks (ruff, black, mypy) also pass on every commit in this branch.

CI status

I ran the workflow before opening this PR, via a dry-run PR into my own fork, and validate-migrations passes — service container healthy, pip install -e ".[dev]" fine on Python 3.11.15, the script executable in the checkout, both migrations applied and alembic check clean, 1m 1s total.

I also confirmed it fails on CI, not just locally. I pushed a commit adding a column to the Profile model with no migration behind it, and the job went red:

INFO  [alembic.autogenerate.compare.tables] Detected added column 'profiles.drift_probe'
ERROR [alembic.util.messaging] New upgrade operations detected:
  [('add_column', None, 'profiles', Column('drift_probe', String(length=50), table=<profiles>))]
##[error]Process completed with exit code 255.

Two things that run confirms: the migrations still applied cleanly first, so the failure came from alembic check rather than a broken migration — the job distinguishes the two. And lint, typecheck and test-unit returned identical numbers with that column present, as did the pre-commit hooks. A well-typed column with no migration behind it is invisible to ruff, black and mypy; only this check sees it. That commit is not part of this branch.

The other five jobs fail, all of them pre-existing:

Job	Result
validate-migrations	passes
lint	182 ruff errors, unchanged from main
typecheck	99 mypy errors across 25 files, none in files I touched
test-unit	53 failed / 383 passed — the same 53 ids as main, plus my 8 new passes
test-integration	exits 5, collected 0 items — no tests present to run
frontend	missing @testing-library/user-event, plus a ReviewSection assertion

Happy to open separate issues for any of these if that's useful — I left them alone to keep this PR scoped to #129.

Screenshots / Demo

N/A

Notes for Reviewers

• The uq_users_email fix is the judgement call here. Updating the model matches what deployed databases already contain; the alternative was a migration dropping the constraint. I went with the model change to avoid a schema change on live data in a CI-focused PR, but I'd happily switch if you'd rather the constraint go away — the column's unique=True already provides a unique index.
• alembic check doesn't catch everything. It reliably catches added/removed tables, columns, indexes and constraints, but can miss some server defaults and check constraints. It's a large improvement over no check at all, not a guarantee of perfect parity.
• The new tests read migration metadata with ast instead of importing the modules. The repo ships its own alembic/ package directory, which shadows the installed alembic distribution once the repo root is on sys.path — as it is under pytest — so from alembic import op raises at collection time. Importing the migrations would have been the natural approach; this is the workaround. Happy to revisit if you'd prefer that shadowing addressed separately.