deer-flow/backend/tests/test_persistence_bootstrap_regression.py
AnoobFeng debb0fd161
feat(persistence): wire alembic migrations, bootstrap schema on startup (#3706)
* feat(persistence): wire alembic migrations + bootstrap schema on startup

Closes #3682. Pre-#3658 DBs lack the `runs.token_usage_by_model` column
because alembic was never wired up — startup only ran `create_all`,
which never ALTERs existing tables.

Adds a hybrid bootstrap in FastAPI lifespan (replaces bare `create_all`):
- empty DB     → create_all + stamp head
- legacy DB    → stamp 0001_baseline + upgrade head
- versioned DB → upgrade head

Concurrency: Postgres `pg_advisory_lock` (cross-process); SQLite
per-engine `asyncio.Lock` + 30s `PRAGMA busy_timeout` on both prod and
alembic engines. Column revisions use `safe_add_column` /
`safe_drop_column` idempotent helpers as fallback.

Other bits:
- 0001 baseline (chain root) + 0002 add `runs.token_usage_by_model`
- `include_object` filter so alembic ignores LangGraph checkpointer tables
- `make migrate-rev MSG="..."` for authoring new revisions
  (no migrate/stamp targets — startup is the only execution path)
- Tests: three-branch decision, concurrency, #3682 regression, env
  filter, blocking-IO gate anchor
- CLAUDE.md: new "Schema migrations" section

* fix(style): fix lint error

* perf(persistence): address review feedback on alembic bootstrap

Behavioural fixes
- _SQLITE_LOCKS now keyed via WeakKeyDictionary so id-reuse after GC
  cannot return a stale, loop-bound lock and the cache cannot leak one
  entry per disposed engine.
- safe_add_column compares nullable / server_default against the desired
  column when the name already exists and emits a warning on drift,
  surfacing manual-ALTER workarounds instead of silently no-op'ing.
- _postgres_lock issues SET LOCAL idle_in_transaction_session_timeout=0
  before pg_advisory_lock, so managed Postgres cannot kill the idle
  lock-holding session mid-upgrade and silently release the advisory
  lock.
- legacy branch now backfills missing baseline tables via a restricted
  create_all (Base.metadata.create_all scoped to _BASELINE_TABLE_NAMES).
  Restores pre-#1930 upgraders whose channel_* tables were never
  provisioned, without pre-empting future create_table revisions for
  newly-added models.

Schema parity
- runs.token_usage_by_model gains server_default=text("'{}'") in both
  the ORM model and the 0001_baseline create_table, matching what 0002
  adds via ALTER. create_all and alembic-upgrade paths now produce
  identical column definitions.
- New parity test compares Base.metadata.create_all output against a
  pure alembic upgrade base->head, asserting column-set, nullable, and
  server_default agree across all tables (normalized through the same
  helper safe_add_column's drift check uses).

Guards
- test_baseline_table_names_constant_matches_0001 pins
  _BASELINE_TABLE_NAMES to 0001_baseline.upgrade()'s actual output --
  the constant cannot drift silently when someone edits 0001.
- test_legacy_backfill_skips_non_baseline_tables verifies the restricted
  backfill does not create a phantom table on Base.metadata, modelling
  a future revision that would otherwise collide on op.create_table.

Doc residuals
- Three-branch decision table is now consistent across bootstrap.py
  top docstring, engine.py comment, test module docstring, and
  CLAUDE.md.
- Stale test anchor in blocking_io/test_persistence_engine_sqlite.py
  docstring now points at the real file.

* fix(style): fix lint error

* fix(persistence): close drift detection holes

- _check_column_drift compares column type via a family equivalence
  allowlist ({JSON, JSONB}). Catches the wrong-type workaround
  `TEXT NOT NULL DEFAULT '{}'` that previously slipped through silently,
  while keeping Postgres JSON/JSONB dialect reflection quiet. Reflected
  and desired type are also echoed in every drift warning's payload for
  operator triage.

- Extract _escape_url_for_alembic so bootstrap._alembic_safe_url and
  scripts/_autogen_revision share the ConfigParser % escape rule
  instead of duplicating it.

- backend/README.md: add `make migrate-rev MSG=...` to Commands and a
  Schema Migrations section per the repo's README/CLAUDE.md sync policy.

- test_base_to_dict.py: scope the test ORM class to an isolated MetaData
  so the create_all-vs-alembic parity test (added in the previous
  commit) is not polluted by the phantom table on the full pytest
  session.
2026-06-24 13:57:12 +08:00

122 lines
4.8 KiB
Python

"""Regression test for GitHub issue #3682.
End-to-end shape:
1. Hand-build a SQLite DB that mirrors a real pre-#3658 deployment -- the
``runs`` table is missing the ``token_usage_by_model`` column, mirroring
what every existing user's DB looked like after the upgrade that triggered
the issue.
2. Run ``init_engine`` (the entry point used by the FastAPI Gateway
lifespan), which now routes through ``bootstrap_schema``.
3. Confirm a real ``SELECT`` against the column succeeds, demonstrating the
500 from the original issue is gone.
The pre-fix codepath would have raised
``sqlalchemy.exc.OperationalError: no such column: runs.token_usage_by_model``
on step 3.
"""
from __future__ import annotations
import sqlite3
from pathlib import Path
from uuid import uuid4
import pytest
import sqlalchemy as sa
import deerflow.persistence.models # noqa: F401 -- registers ORM models
from deerflow.persistence.base import Base
from deerflow.persistence.engine import close_engine, get_session_factory, init_engine
from deerflow.persistence.run import RunRepository
pytestmark = pytest.mark.asyncio
def _seed_pre_3658_database(db_path: Path) -> None:
"""Build a DB that looks like a pre-PR-#3658 deployment.
Uses the synchronous ``sqlite3`` driver so the seed is independent of the
async engine under test.
"""
db_path.parent.mkdir(parents=True, exist_ok=True)
# Easiest way to get the legacy shape exactly right: create_all then
# ALTER away the new column.
sync_url = f"sqlite:///{db_path.as_posix()}"
sync_engine = sa.create_engine(sync_url)
try:
Base.metadata.create_all(sync_engine)
with sync_engine.begin() as conn:
conn.execute(sa.text("ALTER TABLE runs DROP COLUMN token_usage_by_model"))
finally:
sync_engine.dispose()
async def test_legacy_database_recovers_token_usage_column(tmp_path: Path) -> None:
db_path = tmp_path / "legacy.db"
_seed_pre_3658_database(db_path)
# Sanity: confirm we did indeed land in the buggy pre-fix shape before
# init_engine touches the file.
with sqlite3.connect(db_path) as raw:
cols = {row[1] for row in raw.execute("PRAGMA table_info(runs)").fetchall()}
assert "run_id" in cols
assert "token_usage_by_model" not in cols
version_table_count = raw.execute("SELECT count(*) FROM sqlite_master WHERE type='table' AND name='alembic_version'").fetchone()[0]
assert version_table_count == 0
# Run the same init_engine path FastAPI lifespan uses on startup.
url = f"sqlite+aiosqlite:///{db_path.as_posix()}"
await init_engine(backend="sqlite", url=url, sqlite_dir=str(tmp_path))
try:
# The column must now be present.
with sqlite3.connect(db_path) as raw:
cols = {row[1] for row in raw.execute("PRAGMA table_info(runs)").fetchall()}
assert "token_usage_by_model" in cols
version_row = raw.execute("SELECT version_num FROM alembic_version").fetchone()
assert version_row[0] == "0002_runs_token_usage"
# And the read path that originally 500'd must now succeed.
sf = get_session_factory()
assert sf is not None
repo = RunRepository(sf)
# No rows yet -- the point is just that the SELECT does not raise
# ``no such column: runs.token_usage_by_model``.
result = await repo.aggregate_tokens_by_thread(thread_id=str(uuid4()))
assert result["total_tokens"] == 0
assert result["by_model"] == {}
finally:
await close_engine()
async def test_legacy_database_with_manual_alter_still_bootstraps(tmp_path: Path) -> None:
"""User-side workaround scenario: someone already applied the manual
``ALTER TABLE runs ADD COLUMN token_usage_by_model JSON`` from the issue
write-up. The hybrid bootstrap must just stamp head, not double-add the
column, and not error.
"""
db_path = tmp_path / "manual_altered.db"
db_path.parent.mkdir(parents=True, exist_ok=True)
sync_engine = sa.create_engine(f"sqlite:///{db_path.as_posix()}")
try:
Base.metadata.create_all(sync_engine)
# Don't strip the column -- this is the "user already ran the
# workaround" case where create_all already produced it.
finally:
sync_engine.dispose()
url = f"sqlite+aiosqlite:///{db_path.as_posix()}"
await init_engine(backend="sqlite", url=url, sqlite_dir=str(tmp_path))
try:
with sqlite3.connect(db_path) as raw:
cols = [row[1] for row in raw.execute("PRAGMA table_info(runs)").fetchall()]
# No duplicate column -- list, not set, to catch dupes.
assert cols.count("token_usage_by_model") == 1
version_row = raw.execute("SELECT version_num FROM alembic_version").fetchone()
assert version_row[0] == "0002_runs_token_usage"
finally:
await close_engine()