Skip to content

Make m013_usage migration idempotent to recover interrupted migrations#1520

Open
AnayGarodia wants to merge 1 commit into
simonw:mainfrom
AnayGarodia:fix-m013-migration-idempotent
Open

Make m013_usage migration idempotent to recover interrupted migrations#1520
AnayGarodia wants to merge 1 commit into
simonw:mainfrom
AnayGarodia:fix-m013-migration-idempotent

Conversation

@AnayGarodia

Copy link
Copy Markdown

Refs #1036

The bug

migrate() applies each migration and records it in _llm_migrations as two separate auto-committed steps. m013_usage issues three ALTER TABLE statements, each committed individually. If the process is interrupted after the first ALTER but before the migration is recorded (Ctrl-C, terminal closed, crash elsewhere on a slow first run), the database is left with the input_tokens column present but m013_usage unrecorded.

Every subsequent invocation then re-runs m013_usage and dies with:

sqlite3.OperationalError: duplicate column name: input_tokens

which permanently wedges llm until the user deletes logs.db — exactly the workaround reported in #1036.

The fix

Make m013_usage idempotent by checking columns_dict before each add_column, following the same pattern m001_initial already uses. Re-running after an interruption now completes the migration and records it.

Test

test_migrate_recovers_from_interrupted_m013 simulates the interrupted state (columns added, migration not recorded) by deleting the m013_usage row from _llm_migrations after a full migrate, then re-running migrate().

Fails before the fix with duplicate column name: input_tokens, passes after. The other migration tests pass unchanged (test_migrations_for_embeddings and test_backfill_content_hash fail on my machine both with and without this change — local SQLite version issue, appears related to the same family as #1511).

Note: other multi-statement migrations (e.g. m014_schemas) can wedge the same way. Happy to guard those too in a follow-up if you'd like this pattern applied across the board, or alternatively wrap apply+record in a single transaction in migrate() itself.

Copilot AI review requested due to automatic review settings July 8, 2026 02:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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