Skip to content

Fix M7 clip-columns migration failing against a populated database - #27

Merged
davior merged 1 commit into
mainfrom
claude/relaxed-ride-20boyt
Sep 18, 2026
Merged

davior merged 1 commit into
mainfrom
claude/relaxed-ride-20boyt

Conversation

@davior

@davior davior commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Summary

M7 (davior/gam#26) took production down on deploy: alembic upgrade head failed inside the new add_clip_columns_to_asset migration with sqlite3.IntegrityError: FOREIGN KEY constraint failed on DROP TABLE asset, and since migrations run before uvicorn binds a port (entrypoint.sh), the backend container never came up — crash-looping under restart: unless-stopped.

Root cause: batch_alter_table's SQLite "recreate the table" strategy (needed to add a real foreign key to an already-existing table) drops the original asset table and renames a rebuilt copy into place. SQLite enforces foreign keys on DROP TABLE too — an implicit "as if every row were deleted" check — and every connection here runs with PRAGMA foreign_keys=ON (app/database.py's connect listener, which alembic/env.py inherits since it reuses the app's own engine). The drop was refused the instant another table (assettag, suggestion) held a real row referencing an asset row.

No existing test caught this because every migration test runs against a freshly-created, empty database — there's nothing yet in assettag/suggestion to violate at the moment schema migrations run. Any populated database — i.e. every real deployment — hits this every time.

Changes

  • backend/alembic/versions/20260918_0900_add_clip_columns_to_asset.py: disable PRAGMA foreign_keys for the duration of the table rebuild, in both upgrade() and downgrade() (downgrade recreates asset too).
  • backend/tests/test_migrations.py: new regression test that seeds a real asset → assettag cross-reference before running this migration (mirroring what any populated database actually has), then asserts the upgrade succeeds and PRAGMA foreign_key_check reports zero violations afterward.

Verification

  • Reproduced the exact production failure locally against a seeded SQLite database (identical traceback: FOREIGN KEY constraint failed on DROP TABLE asset).
  • Confirmed the fix resolves it, with the asset row and its tag reference both intact afterward and PRAGMA foreign_key_check clean.
  • Confirmed downgrade() also works cleanly against the same populated database, and that upgrade is repeatable.
  • cd backend && pytest -q — 830 passed.
  • cd frontend && npm test -- --run — 264 passed (no frontend files touched by this change).

Deploy notes for this environment

Production (/opt/gam on debian-flight-tracker-01) currently has an orphaned _alembic_tmp_asset table left over from the failed migration attempts, and the backend container is crash-looping. Recovery, once this PR is merged and pulled:

docker compose stop backend
docker compose run --rm --entrypoint python backend -c "
import sqlite3
conn = sqlite3.connect('/app/data/db/gam.db')
conn.execute('DROP TABLE IF EXISTS _alembic_tmp_asset')
conn.commit()
"
git pull origin main
docker compose up --build -d
docker compose ps
curl -f http://localhost:18082/api/health

The real asset table itself was never touched by the failed attempts (confirmed: 15 rows, all present) — only the leftover temp copy needs clearing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KzCKjg6yBtAwMFwZmezuw1


Generated by Claude Code

batch_alter_table's SQLite "recreate the table" strategy drops the original
`asset` table, and SQLite enforces foreign keys on DROP TABLE too (an implicit
"as if every row were deleted" check). Every connection here runs with
PRAGMA foreign_keys=ON, so the drop was refused the moment any other table
(assettag, suggestion) held a real row referencing an asset — which any
populated database has and an empty one never does, so no existing test
caught it. Disable the pragma for just this table rebuild, in both
directions since downgrade() recreates asset too.

Verified against a seeded SQLite db reproducing the exact production
failure (FOREIGN KEY constraint failed on DROP TABLE asset), confirmed
the fix resolves it with no data loss and an intact foreign_key_check,
and added a regression test that seeds a real cross-reference before
migrating so this can't silently regress.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzCKjg6yBtAwMFwZmezuw1
@davior
davior marked this pull request as ready for review September 18, 2026 06:35
@davior
davior merged commit 9fbcb33 into main Sep 18, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants