generated from john/python-template
V4.6 Phase 2: schema re-level in a single atomic pass
These changes all regenerate the same schema, so they land together and revert
together. A partially applied schema pass is not a valid state.
Remove hand-rolled migrations [HIGH-05]
- Delete upgrade_schema and the _upgrade_person_family_search_id /
_upgrade_v42_evidence_tables / _upgrade_v45_selection_columns chain, plus the
two tests that exercised them. The DDL was SQLite-shaped raw SQL that would
not have run on PostgreSQL. create_all now derives everything from metadata
and remains gated by Settings.should_bootstrap_schema. No raw ALTER TABLE or
CREATE INDEX string remains in src.
Break the foreign key cycle [HIGH-08]
- Declare Source.preferred_execution_attempt_id with use_alter=True and an
explicit constraint name. source / job_source / execution_attempt formed an
unresolvable cycle that made metadata.sorted_tables emit an SAWarning and
order execution_attempt before source, which would have been a hard
create_all failure on PostgreSQL and was invisible on SQLite.
- As a side effect the column is now a dialect-aware Uuid rather than the
hardcoded CHAR(32) the raw upgrade DDL produced, so it emits native UUID on
PostgreSQL.
Index the hot filters [HIGH-04]
- Add composite Index("ix_job_status_date_created", "status", "date_created")
for the worker poll, and index the foreign keys the worker and detail pages
filter on: job.document_id, source.document_id, job_source.job_id,
job_source.source_id, document.document_type_id, and the three
document_person foreign keys.
Stop preloading by default [CRIT-02]
- Flip 16 relationships from lazy="selectin" to lazy="raise". The bidirectional
selectin defaults meant loading one Job pulled a large connected subgraph.
- Three further relationships (ExecutionAttempt.job_source,
ProcessingArtifact.execution_attempt, ProcessingArtifact.source) declared no
lazy at all and defaulted to "select", which raises MissingGreenlet under
async. These are now "raise" as well.
- Only 5 of 262 tests failed under the flip; the service layer already carried
explicit eager loads. Fixes went into the service queries, never back into
the models:
- PeopleService._finalize_link refreshes document, person, and role_ref so
the DocumentPerson write endpoints can still project them.
- JobService.update_job_state loads job_sources -> source so the Job it
returns still answers .error_detail and .filename.
- Two tests that bypassed the service layer now load explicitly.
- Audited every UI relationship access against its feeding service method; all
resolve to *_detail / list_*_detail variants with complete eager loads.
Tests
- Assert the composite and hot foreign key indexes exist in a fresh schema.
- Assert metadata.sorted_tables raises no SAWarning and orders source before
execution_attempt.
- Assert preferred_execution_attempt_id is a Uuid that compiles to UUID on
PostgreSQL and that its foreign key carries use_alter.
- Guard CRIT-02 from regression: no mapped relationship may declare a lazy
strategy outside {raise, noload}.
The development database was rebuilt from metadata rather than upgraded; the
previous file is retained out of tree as the Phase 8 migration source.
Verified: 266 passed, 4 skipped; ruff check clean.
Co-authored-by: Copilot App <[email protected]>
This commit is contained in:
zoltan57
Copilot App
+44
-71
@@ -1,8 +1,12 @@
|
||||
"""Tests for the database runtime and V2 schema bootstrap behavior."""
|
||||
|
||||
import warnings
|
||||
|
||||
import pytest
|
||||
import sqlalchemy as sa
|
||||
from sqlalchemy import inspect
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.dialects import postgresql
|
||||
from sqlmodel import SQLModel
|
||||
from sqlmodel import select
|
||||
from sqlmodel.ext.asyncio.session import AsyncSession
|
||||
|
||||
@@ -12,9 +16,9 @@ from transcription.db import create_all
|
||||
from transcription.db import dispose_database_runtime
|
||||
from transcription.db import initialize_database_runtime
|
||||
from transcription.db import session_scope
|
||||
from transcription.db import upgrade_schema
|
||||
from transcription.db.models import DocumentType
|
||||
from transcription.db.models import PersonRole
|
||||
from transcription.db.models import Source
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -106,92 +110,61 @@ async def test_create_all_seeds_default_registry_rows(tmp_path):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_all_upgrades_existing_person_table_for_family_search(tmp_path):
|
||||
async def test_create_all_declares_hot_path_indexes(tmp_path):
|
||||
"""Worker and detail-page filters must be index-backed in a freshly created schema."""
|
||||
settings = Settings(
|
||||
openrouter_api_key="test-key",
|
||||
database=SqliteSettings(path=str(tmp_path / "upgrade.db")),
|
||||
database=SqliteSettings(path=str(tmp_path / "indexes.db")),
|
||||
environment="test",
|
||||
)
|
||||
runtime = initialize_database_runtime(settings=settings)
|
||||
|
||||
try:
|
||||
async with runtime.engine.begin() as connection:
|
||||
await connection.execute(
|
||||
text("CREATE TABLE person (id CHAR(32) PRIMARY KEY NOT NULL, full_name VARCHAR NOT NULL)")
|
||||
)
|
||||
|
||||
await create_all(engine=runtime.engine)
|
||||
async with runtime.engine.connect() as connection:
|
||||
columns, indexes = await connection.run_sync(
|
||||
lambda sync_connection: (
|
||||
{column["name"] for column in inspect(sync_connection).get_columns("person")},
|
||||
inspect(sync_connection).get_indexes("person"),
|
||||
)
|
||||
)
|
||||
|
||||
assert "family_search_id" in columns
|
||||
assert any(index["column_names"] == ["family_search_id"] and index["unique"] for index in indexes)
|
||||
def collect(sync_connection) -> dict[str, list[list[str]]]:
|
||||
database = inspect(sync_connection)
|
||||
return {
|
||||
table: [index["column_names"] for index in database.get_indexes(table)]
|
||||
for table in ("job", "source", "job_source", "document", "document_person")
|
||||
}
|
||||
|
||||
indexes = await connection.run_sync(collect)
|
||||
|
||||
assert ["status", "date_created"] in indexes["job"]
|
||||
assert ["document_id"] in indexes["job"]
|
||||
assert ["document_id"] in indexes["source"]
|
||||
assert ["preferred_execution_attempt_id"] in indexes["source"]
|
||||
assert ["job_id"] in indexes["job_source"]
|
||||
assert ["source_id"] in indexes["job_source"]
|
||||
assert ["document_type_id"] in indexes["document"]
|
||||
for column in ("document_id", "person_id", "role_id"):
|
||||
assert [column] in indexes["document_person"]
|
||||
finally:
|
||||
await dispose_database_runtime()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_v42_upgrade_adds_evidence_tables_without_rewriting_legacy_snapshot(tmp_path):
|
||||
settings = Settings(
|
||||
openrouter_api_key="test-key",
|
||||
database=SqliteSettings(path=str(tmp_path / "v42-upgrade.db")),
|
||||
environment="test",
|
||||
)
|
||||
runtime = initialize_database_runtime(settings=settings)
|
||||
def test_metadata_has_no_unresolvable_table_cycle():
|
||||
"""create_all must be able to order every table, including on PostgreSQL."""
|
||||
with warnings.catch_warnings():
|
||||
warnings.simplefilter("error", sa.exc.SAWarning)
|
||||
ordered = [table.name for table in SQLModel.metadata.sorted_tables]
|
||||
|
||||
try:
|
||||
async with runtime.engine.begin() as connection:
|
||||
await connection.execute(text("CREATE TABLE job (id CHAR(32) PRIMARY KEY NOT NULL)"))
|
||||
await connection.execute(text("CREATE TABLE source (id CHAR(32) PRIMARY KEY NOT NULL)"))
|
||||
await connection.execute(
|
||||
text(
|
||||
"CREATE TABLE job_source ("
|
||||
"id CHAR(32) PRIMARY KEY NOT NULL, "
|
||||
"job_id CHAR(32) NOT NULL, "
|
||||
"source_id CHAR(32) NOT NULL, "
|
||||
"raw_api_response JSON"
|
||||
")"
|
||||
)
|
||||
)
|
||||
await connection.execute(text("INSERT INTO job (id) VALUES ('job-1')"))
|
||||
await connection.execute(text("INSERT INTO source (id) VALUES ('source-1')"))
|
||||
await connection.execute(
|
||||
text(
|
||||
"INSERT INTO job_source (id, job_id, source_id, raw_api_response) "
|
||||
"VALUES ('link-1', 'job-1', 'source-1', :snapshot)"
|
||||
),
|
||||
{"snapshot": '{"legacy":true}'},
|
||||
)
|
||||
assert ordered.index("source") < ordered.index("execution_attempt")
|
||||
|
||||
await upgrade_schema(engine=runtime.engine)
|
||||
await upgrade_schema(engine=runtime.engine)
|
||||
async with runtime.engine.connect() as connection:
|
||||
table_names = set(await connection.run_sync(lambda c: inspect(c).get_table_names()))
|
||||
job_columns = set(
|
||||
await connection.run_sync(
|
||||
lambda c: tuple(column["name"] for column in inspect(c).get_columns("job"))
|
||||
)
|
||||
)
|
||||
source_columns = set(
|
||||
await connection.run_sync(
|
||||
lambda c: tuple(column["name"] for column in inspect(c).get_columns("source"))
|
||||
)
|
||||
)
|
||||
legacy_snapshot = (
|
||||
await connection.execute(text("SELECT raw_api_response FROM job_source WHERE id = 'link-1'"))
|
||||
).scalar_one()
|
||||
|
||||
assert {"execution_attempt", "processing_artifact"}.issubset(table_names)
|
||||
assert "purpose" in job_columns
|
||||
assert "preferred_execution_attempt_id" in source_columns
|
||||
assert "legacy" in legacy_snapshot
|
||||
finally:
|
||||
await dispose_database_runtime()
|
||||
def test_preferred_execution_attempt_id_column_matches_model_declaration():
|
||||
"""The self-referential provenance column is a real UUID, not an opaque CHAR(32)."""
|
||||
column = SQLModel.metadata.tables["source"].c["preferred_execution_attempt_id"]
|
||||
|
||||
assert isinstance(column.type, sa.Uuid)
|
||||
assert column.type.compile(dialect=postgresql.dialect()) == "UUID"
|
||||
assert Source.model_fields["preferred_execution_attempt_id"].annotation is not None
|
||||
|
||||
foreign_key = next(iter(column.foreign_keys))
|
||||
assert foreign_key.use_alter is True
|
||||
assert foreign_key.column is SQLModel.metadata.tables["execution_attempt"].c["id"]
|
||||
|
||||
|
||||
def test_bootstrap_policy_production_defaults_false():
|
||||
|
||||
Reference in New Issue
Block a user