Enforce SQLite foreign keys on application connections

app/database/engine.py

@@ -1,18 +1,71 @@
 """Database engine and session factory construction."""
 
-from sqlalchemy import create_engine
-from sqlalchemy.engine import Engine
+import sqlite3
+
+from sqlalchemy import create_engine, event
+from sqlalchemy.engine import Engine, make_url
+from sqlalchemy.engine.interfaces import DBAPIConnection
 from sqlalchemy.orm import Session, sessionmaker
+from sqlalchemy.pool import ConnectionPoolEntry
 
 
 def build_engine(database_url: str) -> Engine:
-    """Create a SQLAlchemy engine for the given database URL."""
+    """Create a SQLAlchemy engine for the given database URL.
+
+    SQLite engines enable foreign-key enforcement on every new connection.
+    """
+    is_sqlite = make_url(database_url).get_backend_name() == "sqlite"
     connect_args: dict[str, object] = {}
-    if database_url.startswith("sqlite"):
+    if is_sqlite:
         connect_args["check_same_thread"] = False
-    return create_engine(database_url, connect_args=connect_args)
+    engine = create_engine(database_url, connect_args=connect_args)
+    if is_sqlite:
+        event.listen(engine, "connect", _enable_sqlite_foreign_keys)
+    return engine
 
 
 def build_session_factory(engine: Engine) -> sessionmaker[Session]:
     """Create a session factory bound to ``engine``."""
     return sessionmaker(bind=engine, autocommit=False, autoflush=False)
+
+
+class SQLiteForeignKeyEnforcementError(RuntimeError):
+    """A SQLite connection could not be configured to enforce foreign keys."""
+
+
+def _enable_sqlite_foreign_keys(
+    dbapi_connection: DBAPIConnection,
+    connection_record: ConnectionPoolEntry,
+) -> None:
+    """Turn on enforcement of the schema's declared foreign keys.
+
+    SQLite parses ``FOREIGN KEY`` clauses but ignores them unless each
+    connection sets ``PRAGMA foreign_keys = ON``, and silently ignores that
+    pragma inside an open transaction. Following SQLAlchemy's SQLite dialect
+    guidance, the pragma runs with ``sqlite3`` autocommit temporarily on, so
+    it never depends on the driver's transaction-control mode. The setting is
+    read back so a connection that cannot enforce foreign keys fails loudly.
+    """
+    if not isinstance(dbapi_connection, sqlite3.Connection):
+        raise SQLiteForeignKeyEnforcementError(
+            "Expected a sqlite3 connection for a SQLite database URL, got "
+            f"{type(dbapi_connection).__name__}"
+        )
+
+    previous_autocommit = dbapi_connection.autocommit
+    dbapi_connection.autocommit = True
+    try:
+        cursor = dbapi_connection.cursor()
+        try:
+            cursor.execute("PRAGMA foreign_keys = ON")
+            enabled = cursor.execute("PRAGMA foreign_keys").fetchone()
+        finally:
+            cursor.close()
+    finally:
+        dbapi_connection.autocommit = previous_autocommit
+
+    if enabled != (1,):
+        raise SQLiteForeignKeyEnforcementError(
+            "SQLite did not enable foreign-key enforcement "
+            f"(PRAGMA foreign_keys returned {enabled!r})"
+        )

app/database/repositories.py

@@ -636,13 +636,20 @@ def upsert_player(
 ) -> PlayerPersistenceOutcome:
     """Insert, update, or leave unchanged the one row for a player's identity.
 
-    Does not commit or roll back.
+    Does not commit or roll back. A newly inserted player is flushed
+    immediately, so the row exists inside the caller's open transaction before
+    any ``player_seasons`` or ``player_season_hitting`` row references it.
     """
     record = _load_player(session, identity.player_id)
     now = datetime.now(UTC).replace(tzinfo=None)
 
     if record is None:
         session.add(PlayerRecord.from_domain(identity, created_at=now, updated_at=now))
+        # The models declare ForeignKeys but no relationship(), so the unit of
+        # work has no parent/child dependency between these mappers and orders
+        # their INSERTs by class name. Flushing here makes the parent row
+        # visible to SQLite's foreign-key checks without relying on that.
+        session.flush()
         return PlayerPersistenceOutcome.INSERTED
 
     if record.to_domain() == identity:

docs/architecture-deep-dive.md

@@ -155,6 +155,41 @@ Migrations are Alembic (`alembic/versions/`); three so far, matching the
 project's milestones (create batting lines table → add league ingestion
 state → add batting strikeouts column).
 
+**Foreign keys (added in issue #63).** SQLite parses `FOREIGN KEY` clauses
+but ignores them unless each connection runs `PRAGMA foreign_keys = ON`.
+`build_engine()` (`app/database/engine.py`) registers an engine-scoped
+`connect` listener for SQLite URLs only, so the web app, import scripts, and
+tests all reject orphan `player_seasons` and `player_season_hitting` rows.
+SQLite silently ignores the pragma inside an open transaction, so the
+listener follows SQLAlchemy's SQLite dialect guidance: it sets the `sqlite3`
+connection's `autocommit` to `True` while running the pragma, restores the
+previous value, and reads the pragma back, raising
+`SQLiteForeignKeyEnforcementError` if enforcement did not turn on.
+Non-SQLite URLs get no listener.
+
+**Parent-before-child writes.** The Player models declare `ForeignKey`s but
+no `relationship()`s. SQLAlchemy's unit of work only orders INSERTs across
+mappers when a relationship links them; otherwise it orders mapper batches
+by module and class name. `PlayerRecord` happens to sort before
+`PlayerSeasonCatalogRecord` and `PlayerSeasonHittingRecord`, but that is
+coincidence, not a guarantee. `upsert_player()` therefore flushes
+immediately after adding a new player, so the `players` row exists in the
+caller's open transaction before membership or hitting rows reference it.
+The flush does not commit: single-player and catalog ingestion still run in
+one `session.begin()` block and roll back together on failure. Updates and
+unchanged players do not flush.
+
+Alembic builds its own engine in `alembic/env.py` and does not go through
+`build_engine()`, so migrations run with SQLite's default (enforcement off).
+That is deliberate: SQLite's `batch_alter_table` table rebuilds are safest
+with enforcement off, and changing it is outside issue #63. The automated
+test (`tests/test_database_foreign_keys.py`) upgrades a fresh database
+through that normal Alembic path, then opens it with an enforcing
+application engine and checks `PRAGMA foreign_key_check` reports no
+violations. Migrations are **not** exercised with enforcement on.
+Enforcement also applies only to new writes. Rows committed earlier are not
+re-checked; `PRAGMA foreign_key_check` lists any orphans.
+
 ## 3. Analytics — `app/analytics/`
 
 Pure functions: given `Sequence[TeamGameBattingLine]` in, a typed