add migration docs & tests, fix empty env var crash

- Expand migrate.py docstring with algorithm, protocol, and design decisions
- Add 20-test suite for migration framework (test_migrations.py)
- Fix: empty env vars (SECRET_KEY=) now fall back to defaults via _env() helper

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

CHANGELOG.md

@@ -6,7 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
 
+### Fixed
+- Empty env vars (e.g. `SECRET_KEY=`) now fall back to defaults instead of silently using `""` — fixes 500 on every request when `.env` has blank values
+
 ### Added
+- Comprehensive migration test suite (`tests/test_migrations.py` — 20 tests) covering fresh DB, existing DB, up-to-date DB, idempotent migration, version discovery, `_is_fresh_db`, migration 0001 correctness, and ordering
+- Expanded `migrate.py` module docstring documenting the 8-step algorithm, protocol for adding migrations, and design decisions
 - Sequential migration system (`migrations/migrate.py`) — tracks applied versions in `_migrations` table, auto-detects fresh vs existing DBs, runs pending migrations in order
 - `migrations/versions/0001_rename_ls_to_paddle.py` — first versioned migration (absorbed from `scripts/migrate_to_paddle.py`)
 - Server-side financial calculator (`planner/calculator.py`) — ported JS `calc()`, `pmt()`, `calcIRR()` to Python so the full financial model is no longer exposed in client-side JavaScript

padelnomics/src/padelnomics/core.py

@@ -17,14 +17,20 @@ from quart import g, request, session
 
 load_dotenv()
 
+
+def _env(key: str, default: str) -> str:
+    """Get env var, treating empty string same as unset."""
+    return os.getenv(key, "") or default
+
+
 # =============================================================================
 # Configuration
 # =============================================================================
 
 class Config:
-    APP_NAME: str = os.getenv("APP_NAME", "Padelnomics")
-    SECRET_KEY: str = os.getenv("SECRET_KEY", "change-me-in-production")
-    BASE_URL: str = os.getenv("BASE_URL", "http://localhost:5000")
+    APP_NAME: str = _env("APP_NAME", "Padelnomics")
+    SECRET_KEY: str = _env("SECRET_KEY", "change-me-in-production")
+    BASE_URL: str = _env("BASE_URL", "http://localhost:5000")
     DEBUG: bool = os.getenv("DEBUG", "false").lower() == "true"
     
     DATABASE_PATH: str = os.getenv("DATABASE_PATH", "data/app.db")
@@ -42,8 +48,8 @@ class Config:
     }
     
     RESEND_API_KEY: str = os.getenv("RESEND_API_KEY", "")
-    EMAIL_FROM: str = os.getenv("EMAIL_FROM", "hello@padelnomics.io")
-    ADMIN_EMAIL: str = os.getenv("ADMIN_EMAIL", "leads@padelnomics.io")
+    EMAIL_FROM: str = _env("EMAIL_FROM", "hello@padelnomics.io")
+    ADMIN_EMAIL: str = _env("ADMIN_EMAIL", "leads@padelnomics.io")
     
     RATE_LIMIT_REQUESTS: int = int(os.getenv("RATE_LIMIT_REQUESTS", "100"))
     RATE_LIMIT_WINDOW: int = int(os.getenv("RATE_LIMIT_WINDOW", "60"))

padelnomics/src/padelnomics/migrations/migrate.py

@@ -1,10 +1,47 @@
 """
-Sequential migration runner.
+Sequential migration runner for Padelnomics.
 
-- Runs schema.sql (idempotent CREATE IF NOT EXISTS for fresh DBs)
-- Scans versions/ for NNNN_*.py files and runs unapplied ones in order
-- Fresh DBs: marks all versions as applied without running them
-  (schema.sql already contains the final schema)
+Manages SQLite schema evolution with two paths: fresh databases get the
+full schema from schema.sql in one shot; existing databases get incremental
+migrations applied in order.
+
+Algorithm
+---------
+1. Connect to the SQLite database (create file if missing).
+2. Set WAL journal mode and enable foreign keys.
+3. Check whether the DB is fresh (no application tables at all).
+4. Execute schema.sql — all statements use CREATE IF NOT EXISTS, so this
+   is a no-op on existing databases that already have the tables.
+5. Discover version files in versions/ matching NNNN_*.py.
+6. Diff discovered versions against the _migrations tracking table.
+7. Choose a path:
+   - **Fresh DB**: record every version as applied *without* executing it,
+     because schema.sql already contains the final schema state.
+   - **Existing DB with pending versions**: import each pending module and
+     call its ``up(conn)`` function, then record it in _migrations.
+   - **Up-to-date DB**: no-op, print confirmation.
+8. Commit the transaction and print a summary with table names.
+
+Adding a new migration
+----------------------
+1. Create ``versions/NNNN_description.py`` with a single ``up(conn)``
+   function that receives an *uncommitted* ``sqlite3.Connection``.
+   The runner commits after all pending migrations succeed (batch
+   atomicity), so do NOT call ``conn.commit()`` inside ``up()``.
+2. Update ``schema.sql`` so it reflects the final state *after* the
+   migration. Fresh databases must end up identical to migrated ones.
+3. Keep both in sync — schema.sql is the single source of truth for
+   what a brand-new database looks like.
+
+Design decisions
+----------------
+- **schema.sql as source of truth**: Fresh deploys get the full schema
+  instantly without replaying every historical migration.
+- **Sync sqlite3, not aiosqlite**: Migrations run at startup *before*
+  the async event loop, so we use the stdlib sqlite3 module directly.
+- **up(conn) receives an uncommitted connection**: All pending migrations
+  share a single transaction. If any migration fails, the entire batch
+  rolls back, leaving the DB in its previous consistent state.
 """
 
 import importlib

padelnomics/tests/test_migrations.py

@@ -0,0 +1,376 @@
+"""
+Tests for the sequential migration runner.
+
+Synchronous tests — migrate.py uses stdlib sqlite3, not aiosqlite.
+Uses tmp_path for isolated DB files and monkeypatch for DATABASE_PATH.
+"""
+
+import importlib
+import re
+import sqlite3
+from pathlib import Path
+from unittest.mock import patch
+
+import pytest
+
+from padelnomics.migrations.migrate import _discover_versions, _is_fresh_db, migrate
+
+SCHEMA_PATH = (
+    Path(__file__).parent.parent / "src" / "padelnomics" / "migrations" / "schema.sql"
+)
+VERSIONS_DIR = (
+    Path(__file__).parent.parent / "src" / "padelnomics" / "migrations" / "versions"
+)
+
+# ── Helpers ───────────────────────────────────────────────────
+
+
+def _old_schema_sql():
+    """Return schema.sql with paddle columns swapped back to lemonsqueezy."""
+    schema = SCHEMA_PATH.read_text()
+    schema = schema.replace("paddle_customer_id", "lemonsqueezy_customer_id")
+    schema = schema.replace("paddle_subscription_id", "lemonsqueezy_subscription_id")
+    return schema
+
+
+def _table_names(conn):
+    """Return sorted list of user-visible table names."""
+    rows = conn.execute(
+        "SELECT name FROM sqlite_master WHERE type='table'"
+        " AND name NOT LIKE 'sqlite_%' ORDER BY name"
+    ).fetchall()
+    return [r[0] for r in rows]
+
+
+def _column_names(conn, table):
+    return [r[1] for r in conn.execute(f"PRAGMA table_info({table})").fetchall()]
+
+
+# ── Fixtures ──────────────────────────────────────────────────
+
+
+@pytest.fixture
+def schema_sql():
+    return SCHEMA_PATH.read_text()
+
+
+@pytest.fixture
+def fresh_db_path(tmp_path):
+    """Path to a non-existent DB file."""
+    return str(tmp_path / "fresh.db")
+
+
+@pytest.fixture
+def existing_db(tmp_path):
+    """DB with old lemonsqueezy column names and no _migrations table."""
+    db_path = str(tmp_path / "existing.db")
+    schema = _old_schema_sql()
+    # Remove the _migrations table DDL so this DB has no tracking
+    schema = re.sub(
+        r"CREATE TABLE IF NOT EXISTS _migrations\s*\([^)]+\);",
+        "",
+        schema,
+    )
+    conn = sqlite3.connect(db_path)
+    conn.executescript(schema)
+    conn.commit()
+    conn.close()
+    return db_path
+
+
+@pytest.fixture
+def production_db(tmp_path, schema_sql):
+    """DB with current paddle columns but no _migrations records."""
+    db_path = str(tmp_path / "production.db")
+    # Remove the _migrations DDL so it simulates manual migration
+    schema = re.sub(
+        r"CREATE TABLE IF NOT EXISTS _migrations\s*\([^)]+\);",
+        "",
+        schema_sql,
+    )
+    conn = sqlite3.connect(db_path)
+    conn.executescript(schema)
+    conn.commit()
+    conn.close()
+    return db_path
+
+
+@pytest.fixture
+def up_to_date_db(tmp_path, schema_sql):
+    """DB with final schema and all migrations recorded."""
+    db_path = str(tmp_path / "uptodate.db")
+    conn = sqlite3.connect(db_path)
+    conn.executescript(schema_sql)
+    # Record all discovered versions as applied
+    for f in sorted(VERSIONS_DIR.iterdir()):
+        if re.match(r"^\d{4}_.+\.py$", f.name):
+            conn.execute(
+                "INSERT INTO _migrations (name) VALUES (?)", (f.stem,)
+            )
+    conn.commit()
+    conn.close()
+    return db_path
+
+
+@pytest.fixture
+def mock_versions_dir(tmp_path):
+    """Empty temp directory for version discovery tests."""
+    d = tmp_path / "versions"
+    d.mkdir()
+    return d
+
+
+# ── TestFreshDatabase ─────────────────────────────────────────
+
+
+class TestFreshDatabase:
+    def test_creates_all_tables(self, fresh_db_path, monkeypatch):
+        monkeypatch.setenv("DATABASE_PATH", fresh_db_path)
+        migrate()
+        conn = sqlite3.connect(fresh_db_path)
+        tables = _table_names(conn)
+        conn.close()
+        assert "_migrations" in tables
+        assert "users" in tables
+        assert "subscriptions" in tables
+        assert "scenarios" in tables
+
+    def test_records_all_versions_as_applied(self, fresh_db_path, monkeypatch):
+        monkeypatch.setenv("DATABASE_PATH", fresh_db_path)
+        migrate()
+        conn = sqlite3.connect(fresh_db_path)
+        applied = {
+            r[0] for r in conn.execute("SELECT name FROM _migrations").fetchall()
+        }
+        conn.close()
+        versions = _discover_versions()
+        assert applied == set(versions)
+
+    def test_does_not_call_import_module(self, fresh_db_path, monkeypatch):
+        monkeypatch.setenv("DATABASE_PATH", fresh_db_path)
+        with patch("padelnomics.migrations.migrate.importlib.import_module") as mock_imp:
+            migrate()
+            mock_imp.assert_not_called()
+
+    def test_uses_paddle_column_names(self, fresh_db_path, monkeypatch):
+        monkeypatch.setenv("DATABASE_PATH", fresh_db_path)
+        migrate()
+        conn = sqlite3.connect(fresh_db_path)
+        cols = _column_names(conn, "subscriptions")
+        conn.close()
+        assert "paddle_customer_id" in cols
+        assert "paddle_subscription_id" in cols
+        assert "lemonsqueezy_customer_id" not in cols
+
+
+# ── TestExistingDatabase ──────────────────────────────────────
+
+
+class TestExistingDatabase:
+    def test_applies_pending_migration(self, existing_db, monkeypatch):
+        monkeypatch.setenv("DATABASE_PATH", existing_db)
+        migrate()
+        conn = sqlite3.connect(existing_db)
+        cols = _column_names(conn, "subscriptions")
+        conn.close()
+        assert "paddle_customer_id" in cols
+        assert "paddle_subscription_id" in cols
+        assert "lemonsqueezy_customer_id" not in cols
+
+    def test_records_migration_with_timestamp(self, existing_db, monkeypatch):
+        monkeypatch.setenv("DATABASE_PATH", existing_db)
+        migrate()
+        conn = sqlite3.connect(existing_db)
+        row = conn.execute(
+            "SELECT name, applied_at FROM _migrations WHERE name LIKE '0001%'"
+        ).fetchone()
+        conn.close()
+        assert row is not None
+        assert row[0] == "0001_rename_ls_to_paddle"
+        assert row[1] is not None  # timestamp populated
+
+
+# ── TestUpToDateDatabase ──────────────────────────────────────
+
+
+class TestUpToDateDatabase:
+    def test_noop_when_all_applied(self, up_to_date_db, monkeypatch):
+        monkeypatch.setenv("DATABASE_PATH", up_to_date_db)
+        with patch("padelnomics.migrations.migrate.importlib.import_module") as mock_imp:
+            migrate()
+            mock_imp.assert_not_called()
+
+    def test_no_duplicate_entries_on_rerun(self, up_to_date_db, monkeypatch):
+        monkeypatch.setenv("DATABASE_PATH", up_to_date_db)
+        migrate()
+        migrate()
+        conn = sqlite3.connect(up_to_date_db)
+        count = conn.execute("SELECT COUNT(*) FROM _migrations").fetchone()[0]
+        conn.close()
+        assert count == len(_discover_versions())
+
+
+# ── TestIdempotentMigration ───────────────────────────────────
+
+
+class TestIdempotentMigration:
+    def test_production_db_paddle_cols_already_exist(
+        self, production_db, monkeypatch
+    ):
+        """Production scenario: paddle columns exist, no _migrations table.
+        0001 runs without error and gets recorded."""
+        monkeypatch.setenv("DATABASE_PATH", production_db)
+        migrate()
+        conn = sqlite3.connect(production_db)
+        cols = _column_names(conn, "subscriptions")
+        applied = {
+            r[0] for r in conn.execute("SELECT name FROM _migrations").fetchall()
+        }
+        conn.close()
+        assert "paddle_customer_id" in cols
+        assert "0001_rename_ls_to_paddle" in applied
+
+
+# ── TestDiscoverVersions ─────────────────────────────────────
+
+
+class TestDiscoverVersions:
+    def test_finds_and_sorts_version_files(self):
+        versions = _discover_versions()
+        assert len(versions) >= 1
+        assert versions[0] == "0001_rename_ls_to_paddle"
+
+    def test_ignores_non_matching_files(self, mock_versions_dir, monkeypatch):
+        (mock_versions_dir / "__init__.py").write_text("")
+        (mock_versions_dir / "readme.txt").write_text("")
+        (mock_versions_dir / "0001_real.py").write_text("")
+        monkeypatch.setattr(
+            "padelnomics.migrations.migrate.VERSIONS_DIR", mock_versions_dir
+        )
+        versions = _discover_versions()
+        assert versions == ["0001_real"]
+
+    def test_returns_empty_for_missing_directory(self, tmp_path, monkeypatch):
+        monkeypatch.setattr(
+            "padelnomics.migrations.migrate.VERSIONS_DIR",
+            tmp_path / "nonexistent",
+        )
+        assert _discover_versions() == []
+
+
+# ── TestIsFreshDb ─────────────────────────────────────────────
+
+
+class TestIsFreshDb:
+    def test_empty_db_is_fresh(self, tmp_path):
+        conn = sqlite3.connect(str(tmp_path / "empty.db"))
+        assert _is_fresh_db(conn) is True
+        conn.close()
+
+    def test_db_with_schema_is_not_fresh(self, tmp_path, schema_sql):
+        conn = sqlite3.connect(str(tmp_path / "full.db"))
+        conn.executescript(schema_sql)
+        assert _is_fresh_db(conn) is False
+        conn.close()
+
+    def test_db_with_single_table_is_not_fresh(self, tmp_path):
+        conn = sqlite3.connect(str(tmp_path / "one.db"))
+        conn.execute("CREATE TABLE foo (id INTEGER PRIMARY KEY)")
+        assert _is_fresh_db(conn) is False
+        conn.close()
+
+
+# ── TestMigration0001 ─────────────────────────────────────────
+
+
+class TestMigration0001:
+    @pytest.fixture
+    def mod_0001(self):
+        return importlib.import_module(
+            "padelnomics.migrations.versions.0001_rename_ls_to_paddle"
+        )
+
+    def test_renames_columns(self, tmp_path, mod_0001):
+        conn = sqlite3.connect(str(tmp_path / "rename.db"))
+        conn.executescript(_old_schema_sql())
+        mod_0001.up(conn)
+        cols = _column_names(conn, "subscriptions")
+        conn.close()
+        assert "paddle_customer_id" in cols
+        assert "paddle_subscription_id" in cols
+        assert "lemonsqueezy_customer_id" not in cols
+
+    def test_idempotent_when_already_renamed(self, tmp_path, schema_sql, mod_0001):
+        conn = sqlite3.connect(str(tmp_path / "idem.db"))
+        conn.executescript(schema_sql)
+        # Should not raise even though columns are already paddle_*
+        mod_0001.up(conn)
+        cols = _column_names(conn, "subscriptions")
+        conn.close()
+        assert "paddle_customer_id" in cols
+
+    def test_recreates_index(self, tmp_path, mod_0001):
+        conn = sqlite3.connect(str(tmp_path / "idx.db"))
+        conn.executescript(_old_schema_sql())
+        mod_0001.up(conn)
+        indexes = conn.execute(
+            "SELECT name FROM sqlite_master WHERE type='index'"
+            " AND name='idx_subscriptions_provider'"
+        ).fetchall()
+        conn.close()
+        assert len(indexes) == 1
+
+
+# ── TestMigrationOrdering ─────────────────────────────────────
+
+
+class TestMigrationOrdering:
+    def test_multiple_pending_run_in_order(self, tmp_path, monkeypatch):
+        """Mock two version files and verify they run in sorted order."""
+        db_path = str(tmp_path / "order.db")
+        # Create a DB with one arbitrary table so it's not "fresh"
+        conn = sqlite3.connect(db_path)
+        conn.execute("CREATE TABLE dummy (id INTEGER PRIMARY KEY)")
+        conn.close()
+
+        monkeypatch.setenv("DATABASE_PATH", db_path)
+
+        # Create fake version files in a temp versions dir
+        vdir = tmp_path / "vdir"
+        vdir.mkdir()
+        (vdir / "0001_first.py").write_text("")
+        (vdir / "0002_second.py").write_text("")
+        monkeypatch.setattr(
+            "padelnomics.migrations.migrate.VERSIONS_DIR", vdir
+        )
+
+        call_order = []
+
+        def fake_import(name):
+            class FakeMod:
+                @staticmethod
+                def up(conn):
+                    call_order.append(name)
+            return FakeMod()
+
+        with patch(
+            "padelnomics.migrations.migrate.importlib.import_module",
+            side_effect=fake_import,
+        ):
+            migrate()
+
+        assert call_order == [
+            "padelnomics.migrations.versions.0001_first",
+            "padelnomics.migrations.versions.0002_second",
+        ]
+
+    def test_migrations_table_created_on_existing_db(
+        self, existing_db, monkeypatch
+    ):
+        """An existing DB without _migrations gets the table after migrate()."""
+        monkeypatch.setenv("DATABASE_PATH", existing_db)
+        migrate()
+        conn = sqlite3.connect(existing_db)
+        tables = _table_names(conn)
+        conn.close()
+        assert "_migrations" in tables