Skip to content

Commit d7152bd

Browse files
committed
fix(db): make alembic upgrade head work against an empty database
`uv run alembic upgrade head` on a fresh database failed at the first `batch_op.drop_index(op.f("ix_fy_summaries_fiscal_year"))`: the migration that creates `fy_summaries` names its index `ix_fy_summary_year`, so the drop had nothing to drop and batch-mode reflection raised. Nothing caught it -- CI never ran migrations from scratch, prod only applies new revisions to a Neon database that already carries the history, and `init_db()`'s `create_all()` masks migration gaps at runtime. Fixing the reported drop exposed three more breaks in the same class, all from the same root cause (migrations were never the complete schema): - `account_classifications` was created by no migration at all, only by `create_all()`. Added guarded on reflection, following the existing `_create_transactions_table_if_missing` precedent. - af63e055055a used non-batch `create_foreign_key` / `create_unique_constraint`, which SQLite cannot do, and dropped four more create_all-named indexes. - The rollup backfill selected preference columns that only `create_all()` ever added. It now returns early when there are no users, since there is nothing to repair on a fresh database. Reaching head was not enough on its own: the resulting schema still lacked 12 columns the ORM requires, so the app would fail on `no such column: payday`. A reconciliation revision adds them, every add guarded on reflection so it is a no-op on deployed databases. Verified against a create_all-shaped database stamped at the old head: the incremental path applies one revision, drift goes to zero, and rows are preserved. No applied migration's effect is changed -- drops became conditional and constraint DDL moved into batch mode, both no-ops where the objects already exist as before. Regression coverage, whose absence is why this survived: an integration test upgrades a temp empty database and asserts it reaches head, that the schema has every ORM table and column, and that re-running is idempotent. Confirmed to fail with the original `No such index` error when the migration fixes are reverted. Wired the same from-scratch upgrade into ci.yml so every PR proves it.
1 parent f2af2e1 commit d7152bd

8 files changed

Lines changed: 388 additions & 16 deletions

.github/workflows/ci.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,17 @@ jobs:
5454
- name: Pytest
5555
run: uv run --no-sync --locked --no-build pytest tests/ -q
5656

57+
# Prove a fresh environment can build its database from migrations alone.
58+
# Nothing else covers this: migrate.yml only applies new revisions to a
59+
# Neon database that already carries the whole history, and init_db()'s
60+
# create_all() masks migration gaps at runtime. A broken chain here means
61+
# a new contributor cannot bootstrap at all.
62+
- name: Alembic upgrade from an empty database
63+
env:
64+
LEDGER_SYNC_DATABASE_URL: sqlite:///./ci_migration_check.db
65+
LEDGER_SYNC_JWT_SECRET_KEY: ci-secret-key-at-least-32-characters-long
66+
run: uv run --no-sync --locked --no-build alembic upgrade head
67+
5768
security:
5869
uses: Sagargupta16/shared-workflows/.github/workflows/security-scan.yml@main
5970
permissions:

backend/pyproject.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,12 @@ select = [
134134
# Audited 2026-07-26, no real credential. Kept file-scoped on purpose so a
135135
# genuine hardcoded key added to any OTHER test fixture still fails CI.
136136
"tests/unit/test_auth.py" = ["S105", "S106"]
137+
# S603: this test shells out to `alembic upgrade head` to prove a from-scratch
138+
# database bootstrap works -- the argv is this interpreter plus string literals,
139+
# never user input. File-scoped rather than an inline noqa because ruff 0.15
140+
# (pinned in .pre-commit-config.yaml) raises S603 here while 0.16 does not, so
141+
# an inline directive fails one version or the other on RUF100.
142+
"tests/integration/test_migrations_from_scratch.py" = ["S101", "S603"]
137143
# Migration SQL is static DDL built from module-level constants and hardcoded
138144
# table lists, never from user input. The remaining S608 is a multi-line
139145
# f-string INSERT of default preference values whose diagnostic anchors to the

backend/src/ledger_sync/db/migrations/versions/20260206_0837_cc9fa860116e_add_user_id_to_aggregation_tables.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,20 @@
1818
depends_on: str | Sequence[str] | None = None
1919

2020

21+
def _existing_indexes(table: str) -> set[str]:
22+
"""Reflect the index names currently on ``table``.
23+
24+
The autogenerated drop/recreate pairs below assume the index names that
25+
``create_all()`` produces (``ix_<table>_<column>``), but the migration that
26+
created these tables named them differently (``ix_fy_summary_year``,
27+
``ix_merchant_name``, ``ix_monthly_summary_period``). On a database built
28+
from migrations those names never exist, so the drops have to be
29+
conditional or batch-mode reflection raises ``No such index``.
30+
"""
31+
inspector = sa.inspect(op.get_bind())
32+
return {idx["name"] for idx in inspector.get_indexes(table) if idx.get("name")}
33+
34+
2135
def upgrade() -> None:
2236
# ### commands auto generated by Alembic - please adjust! ###
2337

@@ -69,16 +83,20 @@ def upgrade() -> None:
6983
batch_op.create_index(op.f("ix_financial_goals_user_id"), ["user_id"], unique=False)
7084
batch_op.create_foreign_key("fk_financial_goals_user_id", "users", ["user_id"], ["id"])
7185

86+
fy_indexes = _existing_indexes("fy_summaries")
7287
with op.batch_alter_table("fy_summaries") as batch_op:
7388
batch_op.add_column(sa.Column("user_id", sa.Integer(), nullable=False))
74-
batch_op.drop_index(op.f("ix_fy_summaries_fiscal_year"))
89+
if "ix_fy_summaries_fiscal_year" in fy_indexes:
90+
batch_op.drop_index(op.f("ix_fy_summaries_fiscal_year"))
7591
batch_op.create_index(op.f("ix_fy_summaries_fiscal_year"), ["fiscal_year"], unique=False)
7692
batch_op.create_index(op.f("ix_fy_summaries_user_id"), ["user_id"], unique=False)
7793
batch_op.create_foreign_key("fk_fy_summaries_user_id", "users", ["user_id"], ["id"])
7894

95+
merchant_indexes = _existing_indexes("merchant_intelligence")
7996
with op.batch_alter_table("merchant_intelligence") as batch_op:
8097
batch_op.add_column(sa.Column("user_id", sa.Integer(), nullable=False))
81-
batch_op.drop_index(op.f("ix_merchant_intelligence_merchant_name"))
98+
if "ix_merchant_intelligence_merchant_name" in merchant_indexes:
99+
batch_op.drop_index(op.f("ix_merchant_intelligence_merchant_name"))
82100
batch_op.create_index(
83101
op.f("ix_merchant_intelligence_merchant_name"), ["merchant_name"], unique=False
84102
)
@@ -87,9 +105,11 @@ def upgrade() -> None:
87105
"fk_merchant_intelligence_user_id", "users", ["user_id"], ["id"]
88106
)
89107

108+
monthly_indexes = _existing_indexes("monthly_summaries")
90109
with op.batch_alter_table("monthly_summaries") as batch_op:
91110
batch_op.add_column(sa.Column("user_id", sa.Integer(), nullable=False))
92-
batch_op.drop_index(op.f("ix_monthly_summaries_period_key"))
111+
if "ix_monthly_summaries_period_key" in monthly_indexes:
112+
batch_op.drop_index(op.f("ix_monthly_summaries_period_key"))
93113
batch_op.create_index(op.f("ix_monthly_summaries_period_key"), ["period_key"], unique=False)
94114
batch_op.create_index(op.f("ix_monthly_summaries_user_id"), ["user_id"], unique=False)
95115
batch_op.create_index(

backend/src/ledger_sync/db/migrations/versions/20260207_1000_add_user_id_to_account_classifications.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,49 @@
1818
depends_on: str | Sequence[str] | None = None
1919

2020

21+
ACCOUNT_TYPE_VALUES = (
22+
"CASH",
23+
"BANK_ACCOUNTS",
24+
"CREDIT_CARDS",
25+
"INVESTMENTS",
26+
"LOANS",
27+
"OTHER_WALLETS",
28+
)
29+
30+
31+
def _create_account_classifications_if_missing(inspector: sa.Inspector) -> None:
32+
"""Create ``account_classifications`` when no earlier migration has.
33+
34+
No migration ever created this table -- every deployed database got it from
35+
``init_db()``'s ``create_all()``, so the reference below (and in later
36+
revisions) only resolved by luck. Creating it here, guarded on reflection,
37+
lets a fresh database bootstrap from migrations alone while staying a no-op
38+
everywhere the table already exists. Columns match what the ORM expected at
39+
this revision; ``is_closed`` / ``closed_date`` arrive in closed_accounts_2026.
40+
"""
41+
if "account_classifications" in inspector.get_table_names():
42+
return
43+
44+
op.create_table(
45+
"account_classifications",
46+
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
47+
sa.Column("account_name", sa.String(length=255), nullable=False),
48+
sa.Column(
49+
"account_type",
50+
sa.Enum(*ACCOUNT_TYPE_VALUES, name="accounttype"),
51+
nullable=False,
52+
),
53+
sa.Column("created_at", sa.DateTime(), nullable=False),
54+
sa.Column("updated_at", sa.DateTime(), nullable=False),
55+
sa.PrimaryKeyConstraint("id"),
56+
)
57+
op.create_index(
58+
"ix_account_classifications_account_name",
59+
"account_classifications",
60+
["account_name"],
61+
)
62+
63+
2164
def upgrade() -> None:
2265
"""Add user_id to account_classifications, new indexes to transactions."""
2366
# --- account_classifications: add user_id column ---
@@ -27,6 +70,8 @@ def upgrade() -> None:
2770
# Check if user_id column already exists (idempotent)
2871
conn = op.get_bind()
2972
inspector = sa.inspect(conn)
73+
_create_account_classifications_if_missing(inspector)
74+
inspector.clear_cache()
3075
existing_columns = [col["name"] for col in inspector.get_columns("account_classifications")]
3176

3277
if "user_id" not in existing_columns:

backend/src/ledger_sync/db/migrations/versions/20260221_1837_af63e055055a_add_created_at_updated_at_to_.py

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@
1818
depends_on: str | Sequence[str] | None = None
1919

2020

21+
def _existing_indexes(table: str) -> set[str]:
22+
"""Reflect the index names currently on ``table``."""
23+
inspector = sa.inspect(op.get_bind())
24+
return {idx["name"] for idx in inspector.get_indexes(table) if idx.get("name")}
25+
26+
2127
def upgrade() -> None:
2228
# ### commands auto generated by Alembic - please adjust! ###
2329
op.create_table(
@@ -80,32 +86,47 @@ def upgrade() -> None:
8086
op.create_index(
8187
"ix_scheduled_user_active", "scheduled_transactions", ["user_id", "is_active"], unique=False
8288
)
83-
op.drop_index(
84-
op.f("ix_account_classifications_account_name"), table_name="account_classifications"
85-
)
89+
# The index/constraint names below are the ones create_all() produces. A
90+
# database built from migrations names them differently (or not at all), so
91+
# every drop is guarded on reflection and every constraint add goes through
92+
# batch mode -- SQLite has no ALTER TABLE ... ADD CONSTRAINT.
93+
account_indexes = _existing_indexes("account_classifications")
94+
if "ix_account_classifications_account_name" in account_indexes:
95+
op.drop_index(
96+
op.f("ix_account_classifications_account_name"), table_name="account_classifications"
97+
)
8698
op.create_index(
8799
op.f("ix_account_classifications_account_name"),
88100
"account_classifications",
89101
["account_name"],
90102
unique=False,
91103
)
92-
op.create_foreign_key(None, "account_classifications", "users", ["user_id"], ["id"])
93-
op.drop_index(op.f("ix_net_worth_date"), table_name="net_worth_snapshots")
94-
op.drop_index(op.f("ix_net_worth_user"), table_name="net_worth_snapshots")
104+
with op.batch_alter_table("account_classifications") as batch_op:
105+
batch_op.create_foreign_key(
106+
"fk_account_classifications_user_id", "users", ["user_id"], ["id"]
107+
)
108+
109+
net_worth_indexes = _existing_indexes("net_worth_snapshots")
110+
for name in ("ix_net_worth_date", "ix_net_worth_user"):
111+
if name in net_worth_indexes:
112+
op.drop_index(op.f(name), table_name="net_worth_snapshots")
95113
op.create_index(
96114
"ix_net_worth_user_date", "net_worth_snapshots", ["user_id", "snapshot_date"], unique=False
97115
)
98-
op.create_unique_constraint(
99-
"uq_net_worth_user_date", "net_worth_snapshots", ["user_id", "snapshot_date"]
100-
)
116+
with op.batch_alter_table("net_worth_snapshots") as batch_op:
117+
batch_op.create_unique_constraint("uq_net_worth_user_date", ["user_id", "snapshot_date"])
118+
101119
op.add_column("tax_records", sa.Column("user_id", sa.Integer(), nullable=False))
102-
op.drop_index(op.f("ix_tax_records_financial_year"), table_name="tax_records")
103-
op.drop_index(op.f("ix_tax_records_fy"), table_name="tax_records")
120+
tax_indexes = _existing_indexes("tax_records")
121+
for name in ("ix_tax_records_financial_year", "ix_tax_records_fy"):
122+
if name in tax_indexes:
123+
op.drop_index(op.f(name), table_name="tax_records")
104124
op.create_index(
105125
"ix_tax_records_user_fy", "tax_records", ["user_id", "financial_year"], unique=False
106126
)
107127
op.create_index(op.f("ix_tax_records_user_id"), "tax_records", ["user_id"], unique=False)
108-
op.create_foreign_key(None, "tax_records", "users", ["user_id"], ["id"])
128+
with op.batch_alter_table("tax_records") as batch_op:
129+
batch_op.create_foreign_key("fk_tax_records_user_id", "users", ["user_id"], ["id"])
109130
op.add_column("transactions", sa.Column("created_at", sa.DateTime(), nullable=False))
110131
op.add_column("transactions", sa.Column("updated_at", sa.DateTime(), nullable=False))
111132
# ### end Alembic commands ###

backend/src/ledger_sync/db/migrations/versions/20260727_1100_backfill_rollup_preference_splits.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,14 @@ def _repair_fiscal(
422422
def upgrade() -> None:
423423
bind = op.get_bind()
424424

425+
user_ids = [row[0] for row in bind.execute(sa.text("SELECT id FROM users")).fetchall()]
426+
if not user_ids:
427+
# Nothing to repair. Checked before the preferences SELECT below, which
428+
# names columns that only ``create_all()`` ever added -- on a database
429+
# bootstrapped from migrations alone they do not exist until the
430+
# reconciliation revision that follows this one.
431+
return
432+
425433
prefs_by_user: dict[int, dict[str, Any]] = {
426434
row["user_id"]: dict(row)
427435
for row in bind.execute(
@@ -434,7 +442,7 @@ def upgrade() -> None:
434442
).mappings()
435443
}
436444

437-
for (user_id,) in bind.execute(sa.text("SELECT id FROM users")).fetchall():
445+
for user_id in user_ids:
438446
prefs = prefs_by_user.get(user_id)
439447
# A missing preferences row falls back exactly like an empty one, so it
440448
# is in the victim class for both settings.
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
"""add the columns that only ``create_all()`` ever created
2+
3+
Revision ID: reconcile_create_all_2026
4+
Revises: capital_loss_categories_2026
5+
Create Date: 2026-07-27 13:00:00.000000
6+
7+
``init_db()`` calls ``Base.metadata.create_all()`` on every startup, so a
8+
database that has ever booted the app has the full ORM schema whether or not a
9+
migration declared it. That masked a slow drift: twelve columns were added to the
10+
models without a matching migration, and every environment got them from
11+
``create_all`` instead. ``alembic upgrade head`` against an EMPTY database
12+
therefore reached head with a schema the app cannot query -- the first request
13+
touching preferences would fail on ``no such column: payday``.
14+
15+
The columns, all reachable from live code paths:
16+
17+
* ``user_preferences`` -- ``excluded_accounts``, ``fixed_expense_categories``,
18+
``monthly_investment_target``, ``notify_anomalies``, ``notify_budget_alerts``,
19+
``notify_days_ahead``, ``notify_upcoming_bills``, ``payday``,
20+
``preferred_tax_regime``, ``savings_goal_percent``
21+
* ``users`` -- ``last_login``
22+
* ``import_logs`` -- ``user_id`` (plus its index and CASCADE FK; the import
23+
idempotency check is user-scoped through it)
24+
25+
Every add is guarded on reflection, so this is a no-op on every already-deployed
26+
database and only does work on a from-scratch bootstrap. ``server_default``
27+
values mirror the ORM defaults so pre-existing rows would be valid either way.
28+
29+
``import_logs.user_id`` is NOT NULL in the model. It is added nullable, then
30+
backfilled to the first user and tightened only when that is safe -- on a fresh
31+
database the table is empty, and on an existing one the column is already there
32+
and this whole block is skipped.
33+
34+
Follows the repo convention of an empty ``downgrade()`` (restore from a
35+
database backup to roll back).
36+
"""
37+
38+
import sqlalchemy as sa
39+
from alembic import op
40+
41+
revision: str = "reconcile_create_all_2026"
42+
down_revision: str | None = "capital_loss_categories_2026"
43+
branch_labels: str | None = None
44+
depends_on: str | None = None
45+
46+
47+
# (table, column spec) -- server_default mirrors the ORM-side default.
48+
_MISSING_COLUMNS: list[tuple[str, sa.Column]] = [
49+
(
50+
"user_preferences",
51+
sa.Column("excluded_accounts", sa.Text(), nullable=False, server_default="[]"),
52+
),
53+
(
54+
"user_preferences",
55+
sa.Column("fixed_expense_categories", sa.Text(), nullable=False, server_default="[]"),
56+
),
57+
(
58+
"user_preferences",
59+
sa.Column("monthly_investment_target", sa.Float(), nullable=False, server_default="0"),
60+
),
61+
(
62+
"user_preferences",
63+
sa.Column("notify_anomalies", sa.Boolean(), nullable=False, server_default=sa.true()),
64+
),
65+
(
66+
"user_preferences",
67+
sa.Column("notify_budget_alerts", sa.Boolean(), nullable=False, server_default=sa.true()),
68+
),
69+
(
70+
"user_preferences",
71+
sa.Column("notify_days_ahead", sa.Integer(), nullable=False, server_default="7"),
72+
),
73+
(
74+
"user_preferences",
75+
sa.Column("notify_upcoming_bills", sa.Boolean(), nullable=False, server_default=sa.true()),
76+
),
77+
("user_preferences", sa.Column("payday", sa.Integer(), nullable=False, server_default="1")),
78+
(
79+
"user_preferences",
80+
sa.Column(
81+
"preferred_tax_regime", sa.String(length=10), nullable=False, server_default="new"
82+
),
83+
),
84+
(
85+
"user_preferences",
86+
sa.Column("savings_goal_percent", sa.Float(), nullable=False, server_default="20"),
87+
),
88+
("users", sa.Column("last_login", sa.DateTime(), nullable=True)),
89+
]
90+
91+
92+
def _columns(table: str) -> set[str]:
93+
inspector = sa.inspect(op.get_bind())
94+
return {col["name"] for col in inspector.get_columns(table)}
95+
96+
97+
def _indexes(table: str) -> set[str]:
98+
inspector = sa.inspect(op.get_bind())
99+
return {idx["name"] for idx in inspector.get_indexes(table) if idx.get("name")}
100+
101+
102+
def _add_import_logs_user_id() -> None:
103+
"""Add the user-scoping FK column to ``import_logs``."""
104+
bind = op.get_bind()
105+
106+
if "user_id" not in _columns("import_logs"):
107+
op.add_column("import_logs", sa.Column("user_id", sa.Integer(), nullable=True))
108+
first_user = bind.execute(sa.text("SELECT id FROM users ORDER BY id LIMIT 1")).scalar()
109+
if first_user is not None:
110+
bind.execute(
111+
sa.text("UPDATE import_logs SET user_id = :uid WHERE user_id IS NULL"),
112+
{"uid": first_user},
113+
)
114+
# Only tighten to NOT NULL once no row can violate it. A database with
115+
# import history but no users cannot happen (the FK is the owner link),
116+
# but leaving the column nullable beats failing the upgrade.
117+
orphans = bind.execute(
118+
sa.text("SELECT COUNT(*) FROM import_logs WHERE user_id IS NULL"),
119+
).scalar()
120+
if not orphans:
121+
with op.batch_alter_table("import_logs") as batch_op:
122+
batch_op.alter_column("user_id", existing_type=sa.Integer(), nullable=False)
123+
batch_op.create_foreign_key(
124+
"fk_import_logs_user_id_cascade",
125+
"users",
126+
["user_id"],
127+
["id"],
128+
ondelete="CASCADE",
129+
)
130+
131+
if "ix_import_logs_user_id" not in _indexes("import_logs"):
132+
op.create_index("ix_import_logs_user_id", "import_logs", ["user_id"], unique=False)
133+
134+
135+
def upgrade() -> None:
136+
for table, column in _MISSING_COLUMNS:
137+
if column.name not in _columns(table):
138+
op.add_column(table, column)
139+
140+
_add_import_logs_user_id()
141+
142+
143+
def downgrade() -> None:
144+
"""No downgrade -- restore from a database backup."""

0 commit comments

Comments
 (0)