From 63c21d4a14c65c26c2d2fb481488307489a24cda Mon Sep 17 00:00:00 2001 From: Jim Lancaster <40281233+zoltan57@users.noreply.github.com> Date: Sat, 22 Aug 2026 11:21:18 -0500 Subject: [PATCH] v4.10 revision to remove "legacy compatibility" code --- README.md | 5 + docs/data_migration.md | 52 ++++ docs/schema.md | 4 +- src/transcription/api/print_api.py | 2 +- src/transcription/app.py | 4 +- src/transcription/db/__init__.py | 4 +- src/transcription/db/migration.py | 248 ++++++++++++++++++ src/transcription/db/operations.py | 149 +++++++---- src/transcription/services/registry.py | 10 - src/transcription/services/sources.py | 4 +- src/transcription/services/store.py | 15 +- src/transcription/services/workflows.py | 46 +--- src/transcription/ui/components/media_urls.py | 79 +----- src/transcription/ui/pages/settings_page.py | 8 +- tests/integration/test_pipeline_flow.py | 60 ++++- tests/services/test_settings_services.py | 6 +- tests/services/test_store.py | 7 +- tests/services/test_workflows_reliability.py | 18 +- tests/test_db.py | 118 ++++----- tests/tools/test_export_import_migration.py | 116 ++++++++ tests/ui/conftest.py | 2 +- tests/ui/test_media_urls.py | 55 ++-- tests/ui/test_people_page.py | 4 +- tests/ui/test_print_preview_page.py | 6 +- tools/export_import_migration.py | 119 +++++++++ 25 files changed, 840 insertions(+), 301 deletions(-) create mode 100644 docs/data_migration.md create mode 100644 src/transcription/db/migration.py create mode 100644 tests/tools/test_export_import_migration.py create mode 100644 tools/export_import_migration.py diff --git a/README.md b/README.md index d6a95a3..fd2ce20 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,11 @@ not a path. Each job snapshots the validated prompt text, SHA-256 hash, and samp The canonical MVP prompt is: - `prompts/transcribe_document.md` +## Database migration workflow + +Schema upgrades use an explicit export/import rebuild flow (no runtime legacy write compatibility). +See `docs/data_migration.md` for commands and cutover steps. + ## Destructive test procedure (with data backup) AI execution policy: before the first unit-test run in a test/fix cycle, create one backup of `./data`. Reuse that same backup for every subsequent test run in the cycle. After tests succeed, always pause and ask whether to restore now. diff --git a/docs/data_migration.md b/docs/data_migration.md new file mode 100644 index 0000000..25bfd7b --- /dev/null +++ b/docs/data_migration.md @@ -0,0 +1,52 @@ +# Database Rebuild Migration Workflow + +This project uses an explicit **export/import rebuild workflow** for schema migration. + +Policy: +- Do not add runtime legacy-compatibility write paths. +- Rebuild a fresh target database from current models. +- Export current data/media, then import into the fresh target. + +## Commands + +### 1) Export current DB + uploads into a bundle + +```bash +uv run python tools/export_import_migration.py export --bundle-dir .migration-bundle +``` + +Optional source overrides: +- `--source-db ` +- `--source-upload-dir ` + +### 2) Import bundle into a fresh DB + uploads root + +```bash +uv run python tools/export_import_migration.py import --bundle-dir .migration-bundle --target-db .\data\transcription-new.db --target-upload-dir .\data-new +``` + +### 3) One-shot export+import + +```bash +uv run python tools/export_import_migration.py migrate --bundle-dir .migration-bundle --target-db .\data\transcription-new.db --target-upload-dir .\data-new +``` + +## What gets migrated + +- Tables (in dependency order): `document_type`, `person_role`, `document`, `person`, `document_person`, `job`, `source`, `job_source`, `execution_attempt`. +- Media tree under `UPLOAD_DIR`. + +The bundle contains: +- `database.json` (row export) +- `uploads/` (copied media files) + +Path normalization during export/import: +- `source.file_path` is normalized to `documents/...` (upload-root-relative POSIX). +- `person.portrait_path` is normalized to `persons/...` (upload-root-relative POSIX). + +## Cutover + +After importing to a fresh target: +1. Stop the app. +2. Point `DATABASE__*` and `UPLOAD_DIR` to the new targets. +3. Start the app and run smoke checks (`/healthz`, create/upload/process one job). diff --git a/docs/schema.md b/docs/schema.md index 6d1ade0..738ff3b 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -104,7 +104,7 @@ erDiagram | `death_date_raw` | `str \| None` | optional | | `death_place` | `str \| None` | optional | | `biography` | `str \| None` | optional | -| `portrait_path` | `str \| None` | optional | +| `portrait_path` | `str \| None` | optional upload-root-relative POSIX path (`persons/...`) | | `family_search_id` | `str \| None` | nullable unique | | `metadata_` | `dict[str, JsonValue] \| None` | stored as DB column `metadata` (`JSONBCompat`) | | `created_at` | `datetime` | default now | @@ -156,7 +156,7 @@ Index: | `page_number` | `int` | default `1`, `ge=1` | | `upload_name` | `str` | required | | `filename` | `str` | required | -| `file_path` | `str` | required | +| `file_path` | `str` | required upload-root-relative POSIX path (`documents/...`) | | `file_hash` | `str` | required | | `file_size_bytes` | `int` | `BigInteger`, non-null | | `raw_transcription` | `str \| None` | projection field | diff --git a/src/transcription/api/print_api.py b/src/transcription/api/print_api.py index 5d7ccef..d52a483 100644 --- a/src/transcription/api/print_api.py +++ b/src/transcription/api/print_api.py @@ -39,8 +39,8 @@ async def read_document_source_media( if source.document_id != document_id: raise HTTPException(status_code=404, detail="Source not found for Document") - path = Path(source.file_path).resolve() upload_root = service.settings.upload_dir.resolve() + path = (upload_root / Path(source.file_path)).resolve() try: path.relative_to(upload_root) except ValueError as exc: diff --git a/src/transcription/app.py b/src/transcription/app.py index e1bbcfe..a018d23 100644 --- a/src/transcription/app.py +++ b/src/transcription/app.py @@ -24,7 +24,7 @@ from .config import get_settings from .db import create_all from .db import dispose_database_runtime from .db import initialize_database_runtime -from .db import normalize_legacy_status_spellings +from .db import reconcile_canonical_media_paths from .db import reconcile_legacy_job_source_columns from .services import ServiceBundle from .ui import register_pages @@ -45,7 +45,7 @@ async def _lifespan(app: FastAPI): if settings.should_bootstrap_schema: await create_all(engine=app.state.runtime.engine) await reconcile_legacy_job_source_columns(engine=app.state.runtime.engine) - await normalize_legacy_status_spellings(engine=app.state.runtime.engine) + await reconcile_canonical_media_paths(engine=app.state.runtime.engine) settings.upload_dir.mkdir(parents=True, exist_ok=True) settings.prompt_dir.mkdir(parents=True, exist_ok=True) diff --git a/src/transcription/db/__init__.py b/src/transcription/db/__init__.py index 0b7756c..05eb3f9 100644 --- a/src/transcription/db/__init__.py +++ b/src/transcription/db/__init__.py @@ -1,5 +1,5 @@ from .operations import create_all -from .operations import normalize_legacy_status_spellings +from .operations import reconcile_canonical_media_paths from .operations import reconcile_legacy_job_source_columns from .runtime import dispose_database_runtime from .runtime import initialize_database_runtime @@ -10,7 +10,7 @@ __all__ = [ "create_all", "dispose_database_runtime", "initialize_database_runtime", - "normalize_legacy_status_spellings", + "reconcile_canonical_media_paths", "reconcile_legacy_job_source_columns", "session_scope", "transaction_scope", diff --git a/src/transcription/db/migration.py b/src/transcription/db/migration.py new file mode 100644 index 0000000..93600e8 --- /dev/null +++ b/src/transcription/db/migration.py @@ -0,0 +1,248 @@ +from __future__ import annotations + +import base64 +import json +import shutil +from dataclasses import dataclass +from datetime import date +from datetime import datetime +from pathlib import Path +from typing import Any +from uuid import UUID + +from sqlalchemy import URL +from sqlalchemy import MetaData +from sqlalchemy import Table +from sqlalchemy import create_engine +from sqlalchemy import inspect as sqlalchemy_inspect +from sqlalchemy import select +from sqlalchemy.engine import Engine +from sqlmodel import SQLModel + +from transcription.config import Settings +from transcription.config import get_settings + +# Register table metadata. +from transcription.db import models as _models # noqa: F401 +from transcription.db.engine import get_database_url + +EXPORT_TABLE_ORDER = ( + "document_type", + "person_role", + "document", + "person", + "document_person", + "job", + "source", + "job_source", + "execution_attempt", +) + +BYTES_FIELDS = {"transport_body"} + + +@dataclass(frozen=True) +class MigrationPaths: + source_db_url: str + target_db_url: str + source_upload_dir: Path + target_upload_dir: Path + bundle_dir: Path + + +def export_bundle(*, source_db_url: str, source_upload_dir: Path, bundle_dir: Path) -> None: + bundle_dir.mkdir(parents=True, exist_ok=True) + export_json = bundle_dir / "database.json" + uploads_bundle_dir = bundle_dir / "uploads" + + payload: dict[str, Any] = { + "schema_name": "transcription.export-import", + "schema_version": "1", + "created_at": datetime.now().isoformat(), + "tables": {}, + } + + engine = create_engine(source_db_url) + try: + inspector = sqlalchemy_inspect(engine) + source_tables = set(inspector.get_table_names()) + metadata = MetaData() + metadata.reflect(bind=engine) + current_metadata = SQLModel.metadata + + with engine.connect() as connection: + for table_name in EXPORT_TABLE_ORDER: + if table_name not in source_tables: + payload["tables"][table_name] = [] + continue + + source_table = metadata.tables[table_name] + target_table = current_metadata.tables[table_name] + export_columns = [ + column.name for column in target_table.columns if column.name in source_table.columns + ] + rows = connection.execute(select(*(source_table.c[name] for name in export_columns))).mappings().all() + payload["tables"][table_name] = [ + _serialize_row(row, table_name=table_name, source_upload_dir=source_upload_dir) for row in rows + ] + finally: + engine.dispose() + + export_json.write_text(json.dumps(payload, indent=2), encoding="utf-8") + + if uploads_bundle_dir.exists(): + shutil.rmtree(uploads_bundle_dir) + if source_upload_dir.exists(): + shutil.copytree(source_upload_dir, uploads_bundle_dir) + else: + uploads_bundle_dir.mkdir(parents=True, exist_ok=True) + + +def import_bundle(*, target_db_url: str, target_upload_dir: Path, bundle_dir: Path) -> None: + export_json = bundle_dir / "database.json" + uploads_bundle_dir = bundle_dir / "uploads" + payload = json.loads(export_json.read_text(encoding="utf-8")) + + engine = create_engine(target_db_url) + try: + SQLModel.metadata.create_all(engine) + with engine.begin() as connection: + for table_name in reversed(EXPORT_TABLE_ORDER): + table = SQLModel.metadata.tables[table_name] + connection.execute(table.delete()) + + for table_name in EXPORT_TABLE_ORDER: + rows = payload.get("tables", {}).get(table_name, []) + if not rows: + continue + table = SQLModel.metadata.tables[table_name] + connection.execute(table.insert(), [_deserialize_row(row, table) for row in rows]) + finally: + engine.dispose() + + if target_upload_dir.exists(): + shutil.rmtree(target_upload_dir) + target_upload_dir.mkdir(parents=True, exist_ok=True) + if uploads_bundle_dir.exists(): + shutil.copytree(uploads_bundle_dir, target_upload_dir, dirs_exist_ok=True) + + +def migrate_via_bundle(paths: MigrationPaths) -> None: + export_bundle( + source_db_url=paths.source_db_url, + source_upload_dir=paths.source_upload_dir, + bundle_dir=paths.bundle_dir, + ) + import_bundle( + target_db_url=paths.target_db_url, + target_upload_dir=paths.target_upload_dir, + bundle_dir=paths.bundle_dir, + ) + + +def sqlite_url_from_path(path: Path) -> str: + return URL.create(drivername="sqlite", database=str(path)).render_as_string(hide_password=False) + + +def default_sync_db_url(settings: Settings | None = None) -> str: + runtime_settings = settings or get_settings() + return get_database_url(runtime_settings).replace("+aiosqlite", "").replace("+asyncpg", "").replace("+psycopg", "") + + +def _serialize_row(row: dict[str, Any], *, table_name: str, source_upload_dir: Path) -> dict[str, Any]: + serialized: dict[str, Any] = {} + for key, value in row.items(): + serialized_value = _serialize_value(key, value) + if table_name == "source" and key == "file_path" and isinstance(serialized_value, str): + serialized[key] = _canonical_media_relative_path( + serialized_value, + source_upload_dir=source_upload_dir, + preferred_prefix="documents/", + ) + continue + if table_name == "person" and key == "portrait_path" and isinstance(serialized_value, str): + serialized[key] = _canonical_media_relative_path( + serialized_value, + source_upload_dir=source_upload_dir, + preferred_prefix="persons/", + ) + continue + serialized[key] = serialized_value + return serialized + + +def _serialize_value(key: str, value: Any) -> Any: + if isinstance(value, UUID): + return str(value) + if isinstance(value, (datetime, date)): + return value.isoformat() + if isinstance(value, bytes): + return {"encoding": "base64", "data": base64.b64encode(value).decode("ascii")} + if isinstance(value, dict): + return {str(k): _serialize_value("", v) for k, v in value.items()} + if isinstance(value, list): + return [_serialize_value("", item) for item in value] + return value + + +def _deserialize_row(row: dict[str, Any], table: Table) -> dict[str, Any]: + deserialized: dict[str, Any] = {} + for key, value in row.items(): + if key in BYTES_FIELDS and isinstance(value, dict) and value.get("encoding") == "base64": + deserialized[key] = base64.b64decode(value["data"]) + continue + if key in table.columns: + try: + python_type: type[Any] = table.columns[key].type.python_type + except NotImplementedError: + deserialized[key] = value + continue + deserialized[key] = _deserialize_value(python_type, value) + return deserialized + + +def _deserialize_value(python_type: type[Any], value: Any) -> Any: + if value is None: + return None + if python_type is UUID and isinstance(value, str): + return UUID(value) + if python_type is datetime and isinstance(value, str): + return datetime.fromisoformat(value) + if python_type is date and isinstance(value, str): + return date.fromisoformat(value) + return value + + +def _canonical_media_relative_path(value: str, *, source_upload_dir: Path, preferred_prefix: str) -> str: + normalized = value.strip().replace("\\", "/") + lowered = normalized.casefold() + upload_root = source_upload_dir.resolve().as_posix().casefold().rstrip("/") + if lowered.startswith(upload_root + "/"): + normalized = normalized[len(source_upload_dir.resolve().as_posix()) + 1 :] + lowered = normalized.casefold() + + if lowered.startswith("/uploads/"): + normalized = normalized[len("/uploads/") :] + lowered = normalized.casefold() + elif lowered.startswith("uploads/"): + normalized = normalized[len("uploads/") :] + lowered = normalized.casefold() + elif lowered.startswith("data/"): + normalized = normalized[len("data/") :] + lowered = normalized.casefold() + + if preferred_prefix == "persons/" and lowered.startswith("portraits/"): + normalized = "persons/" + normalized[len("portraits/") :] + lowered = normalized.casefold() + + for prefix in ("documents/", "persons/"): + marker = f"/{prefix}" + index = lowered.find(marker) + if index >= 0: + normalized = normalized[index + 1 :] + lowered = normalized.casefold() + break + + if not lowered.startswith(preferred_prefix): + return normalized + return Path(normalized).as_posix() diff --git a/src/transcription/db/operations.py b/src/transcription/db/operations.py index e499c85..4b11a30 100644 --- a/src/transcription/db/operations.py +++ b/src/transcription/db/operations.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from pathlib import Path from sqlalchemy import inspect as sqlalchemy_inspect from sqlalchemy import text @@ -12,7 +13,6 @@ from sqlmodel.ext.asyncio.session import AsyncSession from .engine import resolve_engine from .models import DocumentType -from .models import JobSourceStatus from .models import PersonRole from .registries import BUILT_IN_DOCUMENT_TYPES from .registries import BUILT_IN_PERSON_ROLES @@ -32,56 +32,6 @@ async def create_all(*, engine: AsyncEngine | None = None) -> None: logger.debug("Database schema bootstrap complete for database_url=%s", active_engine.url) -async def normalize_legacy_status_spellings(*, engine: AsyncEngine | None = None) -> int: - """Normalize legacy enum-name spellings to canonical enum values. - - Historical databases may carry ``TRANSCRIBED``/``FAILED``-style enum *names* - in ``job_source.status`` or ``execution_attempt.status``. Runtime models - expect canonical lowercase values, so stale rows must be normalized before - ORM reads. - """ - active_engine = engine or resolve_engine() - if not hasattr(active_engine, "begin"): - return 0 - replacements = { - status.name: status.value - for status in JobSourceStatus - if status.name != status.value - } - - def _normalize(sync_connection) -> int: - inspector = sqlalchemy_inspect(sync_connection) - table_names = set(inspector.get_table_names()) - if not table_names: - return 0 - fixed = 0 - for table_name in ("job_source", "execution_attempt"): - if table_name not in table_names: - continue - for legacy, canonical in replacements.items(): - result = sync_connection.execute( - text(f'update "{table_name}" set status = :canonical where status = :legacy'), - {"canonical": canonical, "legacy": legacy}, - ) - fixed += result.rowcount or 0 - return fixed - - async with active_engine.begin() as connection: - fixed_rows = await connection.run_sync(_normalize) - if fixed_rows: - logger.warning("Normalized %s legacy status row(s) to canonical spellings", fixed_rows) - return fixed_rows - - -LEGACY_JOB_SOURCE_COLUMNS = ( - "raw_transcription", - "ai_metadata", - "raw_api_response", - "error_detail", - "executed_at", -) - - async def reconcile_legacy_job_source_columns(*, engine: AsyncEngine | None = None) -> int: """Remove stale V4.6 ``job_source`` evidence columns from existing databases. @@ -100,7 +50,13 @@ async def reconcile_legacy_job_source_columns(*, engine: AsyncEngine | None = No return 0 present_columns = {column["name"] for column in inspector.get_columns("job_source")} dropped = 0 - for column_name in LEGACY_JOB_SOURCE_COLUMNS: + for column_name in ( + "raw_transcription", + "ai_metadata", + "raw_api_response", + "error_detail", + "executed_at", + ): if column_name not in present_columns: continue sync_connection.execute(text(f'alter table "job_source" drop column "{column_name}"')) @@ -114,6 +70,95 @@ async def reconcile_legacy_job_source_columns(*, engine: AsyncEngine | None = No return dropped_columns +async def reconcile_canonical_media_paths(*, engine: AsyncEngine | None = None) -> int: + """Normalize stored media paths to upload-root-relative POSIX form.""" + active_engine = engine or resolve_engine() + if not hasattr(active_engine, "begin"): + return 0 + + def _reconcile(sync_connection) -> int: + rows_changed = 0 + inspector = sqlalchemy_inspect(sync_connection) + table_names = set(inspector.get_table_names()) + if "source" in table_names: + rows = sync_connection.execute(text('select id, file_path from "source" where file_path is not null')).mappings().all() + for row in rows: + original = str(row["file_path"]) + normalized = _canonical_relative_path(original, preferred_prefix="documents/") + if normalized is None or normalized == original: + continue + sync_connection.execute( + text('update "source" set file_path = :file_path where id = :id'), + {"id": row["id"], "file_path": normalized}, + ) + rows_changed += 1 + + if "person" in table_names: + rows = sync_connection.execute( + text('select id, portrait_path from "person" where portrait_path is not null') + ).mappings().all() + for row in rows: + original = str(row["portrait_path"]) + normalized = _canonical_relative_path(original, preferred_prefix="persons/") + if normalized is None or normalized == original: + continue + sync_connection.execute( + text('update "person" set portrait_path = :portrait_path where id = :id'), + {"id": row["id"], "portrait_path": normalized}, + ) + rows_changed += 1 + return rows_changed + + async with active_engine.begin() as connection: + rows_changed = await connection.run_sync(_reconcile) + if rows_changed: + logger.warning("Normalized %s media-path row(s) to canonical relative format", rows_changed) + return rows_changed + + +def _canonical_relative_path(value: str, *, preferred_prefix: str) -> str | None: + normalized = value.strip().replace("\\", "/") + if not normalized: + return None + + lowered = normalized.casefold() + if lowered.startswith("http://") or lowered.startswith("https://") or lowered.startswith("data:"): + return None + + if lowered.startswith("/uploads/"): + normalized = normalized[len("/uploads/") :] + lowered = normalized.casefold() + elif lowered.startswith("uploads/"): + normalized = normalized[len("uploads/") :] + lowered = normalized.casefold() + elif lowered.startswith("data/"): + normalized = normalized[len("data/") :] + lowered = normalized.casefold() + + for prefix in ("documents/", "persons/", "portraits/"): + marker = f"/{prefix}" + index = lowered.find(marker) + if index >= 0: + normalized = normalized[index + 1 :] + lowered = normalized.casefold() + break + if lowered.startswith(prefix): + break + + if preferred_prefix == "persons/" and lowered.startswith("portraits/"): + normalized = "persons/" + normalized[len("portraits/") :] + lowered = normalized.casefold() + + if not lowered.startswith(preferred_prefix): + return None + + # Collapse any accidental "." segments while preserving relative semantics. + collapsed = Path(normalized).as_posix() + if collapsed.startswith("../") or collapsed == "..": + return None + return collapsed + + async def seed_registry_defaults(*, engine: AsyncEngine | None = None) -> None: """Seed default registry rows for role and document type taxonomies.""" active_engine = engine or resolve_engine() diff --git a/src/transcription/services/registry.py b/src/transcription/services/registry.py index aecd603..8079d90 100644 --- a/src/transcription/services/registry.py +++ b/src/transcription/services/registry.py @@ -54,16 +54,6 @@ class RegistrySummary: is_built_in: bool reference_count: int - @property - def document_count(self) -> int: - """Backward-compatible alias for document-type settings consumers.""" - return self.reference_count - - @property - def link_count(self) -> int: - """Backward-compatible alias for person-role settings consumers.""" - return self.reference_count - class RegistryService[ModelT: RegistryEntry](ServiceBase): """Generic create/read/update/delete behavior for a registry table. diff --git a/src/transcription/services/sources.py b/src/transcription/services/sources.py index b50f7d7..9b909ad 100644 --- a/src/transcription/services/sources.py +++ b/src/transcription/services/sources.py @@ -95,14 +95,14 @@ class ProviderInput: media_type: str -def build_provider_input(source: Source) -> ProviderInput: +def build_provider_input(source: Source, *, upload_dir: Path) -> ProviderInput: """Describe the stored Source bytes that a provider request will carry. Stored pages are normalized upright at ingest, so the file on disk is the exact payload sent to the provider and ``file_hash`` already identifies it. """ return ProviderInput( - path=Path(source.file_path), + path=(upload_dir / Path(source.file_path)).resolve(), digest_sha256=source.file_hash.lower(), byte_size=source.file_size_bytes, media_type=source_mime_type(source.file_path), diff --git a/src/transcription/services/store.py b/src/transcription/services/store.py index 3158bda..4c5b885 100644 --- a/src/transcription/services/store.py +++ b/src/transcription/services/store.py @@ -115,6 +115,7 @@ async def create_document_job( stored_path=stored_path, file_hash=stored.file_hash, file_size_bytes=stored.file_size_bytes, + upload_dir=runtime_settings.upload_dir, prompt_execution=prompt_execution, ) except Exception as exc: @@ -191,6 +192,7 @@ async def create_job_for_document( stored_sources=stored_sources, provider=provider, model=model, + upload_dir=runtime_settings.upload_dir, prompt_execution=prompt_execution, ) except Exception as exc: @@ -219,6 +221,7 @@ async def _create_document_job_records( stored_path: Path, file_hash: str, file_size_bytes: int, + upload_dir: Path, prompt_execution, ) -> tuple[Document, Job]: document = Document( @@ -246,7 +249,7 @@ async def _create_document_job_records( page_number=1, upload_name=Path(original_filename).name, filename=stored_path.name, - file_path=str(stored_path), + file_path=_upload_relative_path(stored_path=stored_path, upload_dir=upload_dir), file_hash=file_hash, file_size_bytes=file_size_bytes, ) @@ -274,6 +277,7 @@ async def _create_job_for_document_records( stored_sources: Sequence[PendingStoredSource], provider: str | None, model: str | None, + upload_dir: Path, prompt_execution, ) -> tuple[Job, list[UUID]]: document = await session.get(Document, document_id) @@ -309,7 +313,10 @@ async def _create_job_for_document_records( page_number=next_page_number + page_offset, upload_name=Path(stored_source.original_filename).name, filename=stored_source.stored_path.name, - file_path=str(stored_source.stored_path), + file_path=_upload_relative_path( + stored_path=stored_source.stored_path, + upload_dir=upload_dir, + ), file_hash=stored_source.file_hash, file_size_bytes=stored_source.file_size_bytes, ) @@ -338,6 +345,10 @@ def _best_effort_delete(path: Path) -> None: logger.warning("Failed to clean up Source file after database error: %s", path) +def _upload_relative_path(*, stored_path: Path, upload_dir: Path) -> str: + return stored_path.resolve().relative_to(upload_dir.resolve()).as_posix() + + async def store_source_file( *, filename: str, diff --git a/src/transcription/services/workflows.py b/src/transcription/services/workflows.py index adf268e..11aaf60 100644 --- a/src/transcription/services/workflows.py +++ b/src/transcription/services/workflows.py @@ -1,5 +1,4 @@ import asyncio -import inspect import logging from dataclasses import dataclass from datetime import UTC @@ -24,7 +23,6 @@ from ..errors import format_error_detail from ..providers import ProviderError from ..providers import RequestManifest from ..providers import SourceEvidenceReference -from ..providers import TranscriptionProvider from ..providers import TranscriptionResult from ..providers import TransportEvidence from . import ServiceBundle @@ -225,7 +223,7 @@ async def process_queued_job( # noqa: PLR0915 provider_input = None page_outcome: _SuccessfulPage | _FailedPage try: - provider_input = build_provider_input(source) + provider_input = build_provider_input(source, upload_dir=runtime_settings.upload_dir) source_reference = SourceEvidenceReference( source_id=source.id, digest_sha256=provider_input.digest_sha256, @@ -238,9 +236,12 @@ async def process_queued_job( # noqa: PLR0915 # raised before this point, which would otherwise leave it unbound. monotonic_started_at = asyncio.get_running_loop().time() result = await asyncio.wait_for( - _call_transcriber( - input_path=provider_input.path, - prompt_execution=prompt_execution, + transcribe_document_image( + provider_input.path, + prompt_name=prompt_execution.prompt_name, + prompt_text=prompt_execution.user_prompt, + temperature=prompt_execution.temperature, + top_p=prompt_execution.top_p, settings=runtime_settings, provider=provider, source_reference=source_reference, @@ -697,36 +698,3 @@ def _find_provider_error(exc: BaseException) -> ProviderError | None: return current current = current.__cause__ or current.__context__ return None - - -async def _call_transcriber( - *, - input_path, - prompt_execution: PromptExecution, - settings: Settings, - provider: TranscriptionProvider, - source_reference: SourceEvidenceReference, - requested_model: str | None, -) -> TranscriptionResult: - """Call the current transcriber while supporting legacy injected test doubles.""" - if "source_reference" in inspect.signature(transcribe_document_image).parameters: - return await transcribe_document_image( - input_path, - prompt_name=prompt_execution.prompt_name, - prompt_text=prompt_execution.user_prompt, - temperature=prompt_execution.temperature, - top_p=prompt_execution.top_p, - settings=settings, - provider=provider, - source_reference=source_reference, - requested_model=requested_model, - ) - return await transcribe_document_image( - input_path, - prompt_name=prompt_execution.prompt_name, - prompt_text=prompt_execution.user_prompt, - temperature=prompt_execution.temperature, - top_p=prompt_execution.top_p, - settings=settings, - provider=provider, - ) diff --git a/src/transcription/ui/components/media_urls.py b/src/transcription/ui/components/media_urls.py index 0741777..3b4c2ae 100644 --- a/src/transcription/ui/components/media_urls.py +++ b/src/transcription/ui/components/media_urls.py @@ -4,10 +4,10 @@ from __future__ import annotations from pathlib import Path from urllib.parse import quote -from urllib.parse import unquote _ABSOLUTE_SCHEMES = ("http://", "https://", "data:") _UPLOAD_ROUTE_PREFIX = "/uploads/" +_CANONICAL_PREFIXES = ("documents/", "persons/") def absolute_upload_url(path: str, *, base_url: str) -> str: @@ -18,13 +18,7 @@ def absolute_upload_url(path: str, *, base_url: str) -> str: def resolve_media_url(path: str | None, *, upload_dir: Path, base_url: str) -> str | None: - """Map a stored media path onto a served upload URL. - - Stored paths have accumulated several shapes over the life of the schema: - absolute filesystem paths, paths relative to the working directory, paths - relative to the upload root, and paths that already carry an upload route. - All of them must still resolve, so each shape is tried in turn. - """ + """Map a canonical stored relative media path onto a served upload URL.""" candidate = (path or "").strip() if not candidate: return None @@ -34,56 +28,23 @@ def resolve_media_url(path: str | None, *, upload_dir: Path, base_url: str) -> s if lowered.startswith(_ABSOLUTE_SCHEMES): return normalized if normalized.startswith(_UPLOAD_ROUTE_PREFIX): - upload_relative = normalized.removeprefix(_UPLOAD_ROUTE_PREFIX) - return _resolve_upload_relative(upload_relative, upload_dir=upload_dir, base_url=base_url) - - resolved_upload_dir = upload_dir.resolve() - path_obj = Path(candidate) - - if path_obj.is_absolute(): - absolute_candidates = [path_obj.resolve()] - else: - absolute_candidates = [ - (Path.cwd() / path_obj).resolve(), - (resolved_upload_dir / path_obj).resolve(), - ] - - for absolute_candidate in absolute_candidates: - try: - relative = absolute_candidate.relative_to(resolved_upload_dir).as_posix() - except ValueError: - continue - if absolute_candidate.is_file(): - return absolute_upload_url(f"/uploads/{quote(relative)}", base_url=base_url) - - upload_name = resolved_upload_dir.name.casefold() - normalized_parts = Path(normalized).parts - lowered_parts = [part.casefold() for part in normalized_parts] - if upload_name in lowered_parts: - index = lowered_parts.index(upload_name) - relative = Path(*normalized_parts[index + 1 :]).as_posix() - if relative: - return _resolve_upload_relative(relative, upload_dir=upload_dir, base_url=base_url) - - if lowered.startswith("uploads/"): - upload_relative = normalized.split("/", 1)[1] if "/" in normalized else "" - return _resolve_upload_relative(upload_relative, upload_dir=upload_dir, base_url=base_url) - if lowered.startswith("data/"): - relative = normalized.split("/", 1)[1] if "/" in normalized else "" - return _resolve_upload_relative(relative, upload_dir=upload_dir, base_url=base_url) - if lowered.startswith(("documents/", "persons/")): + return _resolve_upload_relative(normalized.removeprefix(_UPLOAD_ROUTE_PREFIX), upload_dir=upload_dir, base_url=base_url) + if lowered.startswith(_CANONICAL_PREFIXES): return _resolve_upload_relative(normalized, upload_dir=upload_dir, base_url=base_url) return None def _resolve_upload_relative(relative: str, *, upload_dir: Path, base_url: str) -> str | None: - candidate = unquote(relative).strip().replace("\\", "/") + candidate = relative.strip().replace("\\", "/") if not candidate: return None resolved_upload_dir = upload_dir.resolve() try: - absolute_candidate = (resolved_upload_dir / Path(candidate)).resolve() + normalized_relative = Path(candidate).as_posix() + if not normalized_relative.casefold().startswith(_CANONICAL_PREFIXES): + return None + absolute_candidate = (resolved_upload_dir / Path(normalized_relative)).resolve() safe_relative = absolute_candidate.relative_to(resolved_upload_dir).as_posix() except ValueError: return None @@ -94,6 +55,7 @@ def _resolve_upload_relative(relative: str, *, upload_dir: Path, base_url: str) def public_media_path_label(path: str | None, *, upload_dir: Path) -> str: """Return a safe, non-local path label for UI metadata display.""" + _ = upload_dir candidate = (path or "").strip() if not candidate: return "unknown" @@ -103,23 +65,6 @@ def public_media_path_label(path: str | None, *, upload_dir: Path) -> str: if normalized.startswith(_UPLOAD_ROUTE_PREFIX): return normalized - if lowered.startswith("uploads/"): - return f"/{normalized}" - if lowered.startswith("data/"): - relative = normalized.split("/", 1)[1] if "/" in normalized else "" - return f"/uploads/{relative}" if relative else "/uploads" - if lowered.startswith(("documents/", "persons/")): + if lowered.startswith(_CANONICAL_PREFIXES): return f"/uploads/{normalized}" - - resolved_upload_dir = upload_dir.resolve() - path_obj = Path(candidate) - if path_obj.is_absolute(): - absolute_candidate = path_obj.resolve() - try: - relative = absolute_candidate.relative_to(resolved_upload_dir).as_posix() - return f"/uploads/{relative}" - except ValueError: - # Never expose non-managed absolute filesystem paths. - return path_obj.name or "unknown" - - return f"/uploads/{quote(path_obj.name)}" + return Path(normalized).name or "unknown" diff --git a/src/transcription/ui/pages/settings_page.py b/src/transcription/ui/pages/settings_page.py index adc2151..c85f2cf 100644 --- a/src/transcription/ui/pages/settings_page.py +++ b/src/transcription/ui/pages/settings_page.py @@ -62,7 +62,7 @@ def register_page(*, settings: Settings) -> None: # noqa: PLR0915 { "id": str(item.id), "label": item.label, - "document_count": item.document_count, + "reference_count": item.reference_count, "is_active": item.is_active, "is_built_in": item.is_built_in, } @@ -70,7 +70,7 @@ def register_page(*, settings: Settings) -> None: # noqa: PLR0915 ] table = render_registry_table( rows, - count_field="document_count", + count_field="reference_count", count_label="Documents", ) @@ -179,7 +179,7 @@ def register_page(*, settings: Settings) -> None: # noqa: PLR0915 { "id": str(role.id), "label": role.label, - "link_count": role.link_count, + "reference_count": role.reference_count, "is_active": role.is_active, "is_built_in": role.is_built_in, } @@ -187,7 +187,7 @@ def register_page(*, settings: Settings) -> None: # noqa: PLR0915 ] table = render_registry_table( rows, - count_field="link_count", + count_field="reference_count", count_label="Links", ) diff --git a/tests/integration/test_pipeline_flow.py b/tests/integration/test_pipeline_flow.py index 6d8ef7d..f2acaf3 100644 --- a/tests/integration/test_pipeline_flow.py +++ b/tests/integration/test_pipeline_flow.py @@ -99,8 +99,20 @@ class TestPipelineSuccessFlow: top_p=None, settings=None, provider=None, + source_reference=None, + requested_model=None, ) -> TranscriptionResult: - _ = (image_path, prompt_name, prompt_text, temperature, top_p, settings, provider) + _ = ( + image_path, + prompt_name, + prompt_text, + temperature, + top_p, + settings, + provider, + source_reference, + requested_model, + ) return TranscriptionResult( text="Pipeline transcript", provider="openrouter", @@ -181,9 +193,11 @@ class TestPipelineSuccessFlow: top_p=None, settings=None, provider=None, + source_reference=None, + requested_model=None, ) -> TranscriptionResult: page_name = Path(image_path).name - _ = (prompt_name, prompt_text, temperature, top_p, settings, provider) + _ = (prompt_name, prompt_text, temperature, top_p, settings, provider, source_reference, requested_model) return TranscriptionResult( text=f"Transcript for {page_name}", provider="openrouter", @@ -248,10 +262,22 @@ class TestPipelineSuccessFlow: top_p=None, settings=None, provider=None, + source_reference=None, + requested_model=None, ) -> TranscriptionResult: nonlocal call_count call_count += 1 - _ = (image_path, prompt_name, prompt_text, temperature, top_p, settings, provider) + _ = ( + image_path, + prompt_name, + prompt_text, + temperature, + top_p, + settings, + provider, + source_reference, + requested_model, + ) if call_count == 2: raise RuntimeError("simulated page failure") return TranscriptionResult( @@ -327,9 +353,21 @@ class TestPipelineSuccessFlow: top_p=None, settings=None, provider=None, + source_reference=None, + requested_model=None, ) -> TranscriptionResult: nonlocal call_count - _ = (image_path, prompt_name, prompt_text, temperature, top_p, settings, provider) + _ = ( + image_path, + prompt_name, + prompt_text, + temperature, + top_p, + settings, + provider, + source_reference, + requested_model, + ) call_count += 1 return TranscriptionResult( text="new transcript", @@ -383,8 +421,20 @@ class TestPipelineFailureFlow: top_p=None, settings=None, provider=None, + source_reference=None, + requested_model=None, ) -> TranscriptionResult: - _ = (image_path, prompt_name, prompt_text, temperature, top_p, settings, provider) + _ = ( + image_path, + prompt_name, + prompt_text, + temperature, + top_p, + settings, + provider, + source_reference, + requested_model, + ) raise RuntimeError("pipeline provider failure") monkeypatch.setattr( diff --git a/tests/services/test_settings_services.py b/tests/services/test_settings_services.py index 57203bf..3ceff41 100644 --- a/tests/services/test_settings_services.py +++ b/tests/services/test_settings_services.py @@ -34,7 +34,7 @@ async def test_document_type_maintenance_uses_alphabetical_labels(default_sessio summaries = await service.list_document_type_summaries() assert [item.label for item in summaries] == ["Archive", "Letter"] - assert [item.document_count for item in summaries] == [0, 0] + assert [item.reference_count for item in summaries] == [0, 0] @pytest.mark.asyncio @@ -56,7 +56,7 @@ async def test_document_type_delete_allows_unreferenced_and_blocks_referenced(de await service.create_document(Document(id=uuid4(), name="Typed document", document_type_id=referenced.id)) summaries = {item.id: item for item in await service.list_document_type_summaries()} - assert summaries[referenced.id].document_count == 1 + assert summaries[referenced.id].reference_count == 1 await service.delete_document_type(unused.id) with pytest.raises(DocumentTypeError) as caught: @@ -84,7 +84,7 @@ async def test_person_role_maintenance_orders_by_normalized_label(default_sessio assert [item.id for item in await service.list_person_roles(active_only=False)] == [first.id, second.id] assert [item.id for item in await service.list_person_roles()] == [first.id] summaries = {item.id: item for item in await service.list_person_role_summaries()} - assert summaries[second.id].link_count == 0 + assert summaries[second.id].reference_count == 0 assert summaries[second.id].is_built_in is False diff --git a/tests/services/test_store.py b/tests/services/test_store.py index 3cd460c..df6f918 100644 --- a/tests/services/test_store.py +++ b/tests/services/test_store.py @@ -73,7 +73,10 @@ async def test_create_job_for_document_sorts_sources_and_creates_links(async_ses assert all(source.filename.endswith(".pdf") for source in sources) assert all("A_page" not in source.filename and "b_page" not in source.filename for source in sources) assert all(Path(source.filename).stem == str(source.id) for source in sources) - assert all(Path(source.file_path).parent == (tmp_path / "documents" / str(document.id)) for source in sources) + assert all( + source.file_path == f"documents/{document.id}/{source.filename}" + for source in sources + ) assert [source.file_hash for source in sources] == [ "ca978112ca1bbdcafac231b39a23dc4da786eff8147c4e72b9807785afee48bb", "3e23e8160039594a33894f6564e1b1348bbd7a0088d42c4acb73eeaed59c009d", @@ -108,7 +111,7 @@ async def test_create_document_job_stores_source_under_document_id_directory(asy assert source is not None assert Path(source.filename).stem == str(source.id) assert result.stored_path.name == source.filename - assert Path(source.file_path).parent == expected_parent + assert source.file_path == f"documents/{result.document_id}/{source.filename}" assert source.file_hash == "2c8648d103e3dd7ad87660da0f126a1443b6d21ac1bd3ec000c5e24e2373a90c" assert source.file_size_bytes == len(b"image-bytes") diff --git a/tests/services/test_workflows_reliability.py b/tests/services/test_workflows_reliability.py index 59f65c7..2fb22ca 100644 --- a/tests/services/test_workflows_reliability.py +++ b/tests/services/test_workflows_reliability.py @@ -81,8 +81,20 @@ class TestWorkflowReliability: top_p=None, settings=None, provider=None, + source_reference=None, + requested_model=None, ): - _ = (image_path, prompt_name, prompt_text, temperature, top_p, settings, provider) + _ = ( + image_path, + prompt_name, + prompt_text, + temperature, + top_p, + settings, + provider, + source_reference, + requested_model, + ) raise TimeoutError("simulated provider timeout") monkeypatch.setattr("transcription.services.workflows.transcribe_document_image", _never_returns) @@ -146,9 +158,9 @@ class TestWorkflowReliability: budget_seconds = 0.20 real_build = workflows_module.build_provider_input - def _slow_build(source_arg): + def _slow_build(source_arg, **kwargs): time.sleep(setup_seconds) - return real_build(source_arg) + return real_build(source_arg, **kwargs) async def _never_returns(*args, **kwargs): _ = (args, kwargs) diff --git a/tests/test_db.py b/tests/test_db.py index 9a9803b..2890b2d 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -17,14 +17,10 @@ from transcription.config import SqliteSettings from transcription.db import create_all from transcription.db import dispose_database_runtime from transcription.db import initialize_database_runtime -from transcription.db import normalize_legacy_status_spellings +from transcription.db import reconcile_canonical_media_paths from transcription.db import reconcile_legacy_job_source_columns from transcription.db import session_scope -from transcription.db.models import Document from transcription.db.models import DocumentType -from transcription.db.models import Job -from transcription.db.models import JobSource -from transcription.db.models import JobSourceStatus from transcription.db.models import PersonRole from transcription.db.models import Source @@ -158,60 +154,6 @@ async def test_create_all_declares_hot_path_indexes(tmp_path): await dispose_database_runtime() -@pytest.mark.asyncio -async def test_normalize_legacy_status_spellings_repairs_job_source_status_rows(tmp_path): - settings = Settings( - openrouter_api_key="test-key", - database=SqliteSettings(path=str(tmp_path / "legacy-status.db")), - environment="test", - ) - runtime = initialize_database_runtime(settings=settings) - - try: - await create_all(engine=runtime.engine) - async with AsyncSession(runtime.engine, expire_on_commit=False) as session: - document = Document(name="legacy-status-doc") - session.add(document) - await session.flush() - job = Job(document_id=document.id) - session.add(job) - await session.flush() - source = Source( - document_id=document.id, - page_number=1, - upload_name="legacy.jpg", - filename="legacy.jpg", - file_path="uploads/legacy.jpg", - file_hash="a" * 64, - file_size_bytes=1, - ) - session.add(source) - await session.flush() - job_source = JobSource(job_id=job.id, source_id=source.id, status=JobSourceStatus.PENDING) - session.add(job_source) - await session.commit() - await session.refresh(job_source) - - async with runtime.engine.begin() as connection: - await connection.execute( - text('update "job_source" set status = :status where status = :expected'), - {"status": "TRANSCRIBED", "expected": JobSourceStatus.PENDING.value}, - ) - - fixed_rows = await normalize_legacy_status_spellings(engine=runtime.engine) - assert fixed_rows == 1 - - async with runtime.engine.connect() as connection: - status = ( - await connection.execute( - text('select status from "job_source"'), - ) - ).scalar_one() - assert status == "transcribed" - finally: - await dispose_database_runtime() - - @pytest.mark.asyncio async def test_reconcile_legacy_job_source_columns_drops_executed_at(tmp_path): settings = Settings( @@ -253,6 +195,64 @@ async def test_reconcile_legacy_job_source_columns_drops_executed_at(tmp_path): await dispose_database_runtime() +@pytest.mark.asyncio +async def test_reconcile_canonical_media_paths_normalizes_source_and_person_paths(tmp_path): + settings = Settings( + openrouter_api_key="test-key", + database=SqliteSettings(path=str(tmp_path / "canonical-paths.db")), + environment="test", + ) + runtime = initialize_database_runtime(settings=settings) + + try: + await create_all(engine=runtime.engine) + async with runtime.engine.begin() as connection: + await connection.execute( + text( + 'insert into "person" (id, full_name, portrait_path, created_at, updated_at) ' + 'values (:id, :full_name, :portrait_path, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)' + ), + {"id": "11" * 16, "full_name": "Portrait", "portrait_path": "portraits/person/seeded.png"}, + ) + await connection.execute( + text( + 'insert into "document" (id, name, created_at, updated_at) ' + 'values (:id, :name, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)' + ), + {"id": "22" * 16, "name": "Doc"}, + ) + await connection.execute( + text( + 'insert into "source" (id, document_id, page_number, upload_name, filename, file_path, file_hash, file_size_bytes, date_uploaded) ' + 'values (:id, :document_id, 1, :upload_name, :filename, :file_path, :file_hash, :file_size_bytes, CURRENT_TIMESTAMP)' + ), + { + "id": "33" * 16, + "document_id": "22" * 16, + "upload_name": "page.png", + "filename": "page.png", + "file_path": "data\\documents\\doc-1\\page.png", + "file_hash": "a" * 64, + "file_size_bytes": 1, + }, + ) + + changed = await reconcile_canonical_media_paths(engine=runtime.engine) + assert changed == 2 + + async with runtime.engine.connect() as connection: + source_path = ( + await connection.execute(text('select file_path from "source" where id = :id'), {"id": "33" * 16}) + ).scalar_one() + portrait_path = ( + await connection.execute(text('select portrait_path from "person" where id = :id'), {"id": "11" * 16}) + ).scalar_one() + assert source_path == "documents/doc-1/page.png" + assert portrait_path == "persons/person/seeded.png" + finally: + await dispose_database_runtime() + + def test_metadata_has_no_unresolvable_table_cycle(): """create_all must be able to order every table, including on PostgreSQL.""" with warnings.catch_warnings(): diff --git a/tests/tools/test_export_import_migration.py b/tests/tools/test_export_import_migration.py new file mode 100644 index 0000000..0f4b724 --- /dev/null +++ b/tests/tools/test_export_import_migration.py @@ -0,0 +1,116 @@ +from __future__ import annotations + +from datetime import UTC +from datetime import datetime +from pathlib import Path +from uuid import uuid4 + +from sqlalchemy import create_engine +from sqlalchemy import select +from sqlmodel import SQLModel + +from transcription.db.migration import export_bundle +from transcription.db.migration import import_bundle +from transcription.db.migration import sqlite_url_from_path + +# Register table metadata. +from transcription.db import models as _models # noqa: F401 + + +def test_export_import_migration_round_trips_db_and_uploads(tmp_path): + source_db_path = tmp_path / "source.db" + target_db_path = tmp_path / "target.db" + source_upload_dir = tmp_path / "source_uploads" + target_upload_dir = tmp_path / "target_uploads" + bundle_dir = tmp_path / "bundle" + + source_db_url = sqlite_url_from_path(source_db_path) + target_db_url = sqlite_url_from_path(target_db_path) + + document_id = uuid4() + job_id = uuid4() + source_id = uuid4() + job_source_id = uuid4() + attempt_id = uuid4() + filename = f"{source_id}.jpg" + media_path = source_upload_dir / "documents" / str(document_id) / filename + media_path.parent.mkdir(parents=True, exist_ok=True) + media_path.write_bytes(b"sample-image") + + engine = create_engine(source_db_url) + try: + SQLModel.metadata.create_all(engine) + with engine.begin() as connection: + connection.execute( + SQLModel.metadata.tables["document"].insert(), + [{"id": document_id, "name": "Export doc"}], + ) + connection.execute( + SQLModel.metadata.tables["job"].insert(), + [{"id": job_id, "document_id": document_id, "status": "queued"}], + ) + connection.execute( + SQLModel.metadata.tables["source"].insert(), + [ + { + "id": source_id, + "document_id": document_id, + "page_number": 1, + "upload_name": "upload.jpg", + "filename": filename, + "file_path": str(media_path), + "file_hash": "a" * 64, + "file_size_bytes": len(b"sample-image"), + } + ], + ) + connection.execute( + SQLModel.metadata.tables["job_source"].insert(), + [{"id": job_source_id, "job_id": job_id, "source_id": source_id, "status": "pending"}], + ) + connection.execute( + SQLModel.metadata.tables["execution_attempt"].insert(), + [ + { + "id": attempt_id, + "job_source_id": job_source_id, + "job_id": job_id, + "source_id": source_id, + "attempt_number": 1, + "status": "transcribed", + "provider": "fixture", + "model": "fixture-model", + "transport_body": b"body", + "raw_transcription": "hello", + "started_at": datetime.now(UTC), + "finished_at": datetime.now(UTC), + "duration_ms": 10, + } + ], + ) + finally: + engine.dispose() + + export_bundle(source_db_url=source_db_url, source_upload_dir=source_upload_dir, bundle_dir=bundle_dir) + import_bundle(target_db_url=target_db_url, target_upload_dir=target_upload_dir, bundle_dir=bundle_dir) + + target_engine = create_engine(target_db_url) + try: + with target_engine.connect() as connection: + source_row = connection.execute( + select(SQLModel.metadata.tables["source"].c.file_path).where( + SQLModel.metadata.tables["source"].c.id == source_id + ) + ).one() + attempt_row = connection.execute( + select(SQLModel.metadata.tables["execution_attempt"].c.transport_body).where( + SQLModel.metadata.tables["execution_attempt"].c.id == attempt_id + ) + ).one() + assert source_row[0] == f"documents/{document_id}/{filename}" + assert attempt_row[0] == b"body" + finally: + target_engine.dispose() + + copied_media_path = target_upload_dir / "documents" / str(document_id) / filename + assert copied_media_path.read_bytes() == b"sample-image" diff --git a/tests/ui/conftest.py b/tests/ui/conftest.py index 0b54e26..502e497 100644 --- a/tests/ui/conftest.py +++ b/tests/ui/conftest.py @@ -124,7 +124,7 @@ async def seed_job(app_client: tuple[FastAPI, TestClient]) -> Callable[..., Awai page_number=1, upload_name=filename, filename=filename, - file_path=str(stored_path), + file_path=stored_path.resolve().relative_to(app.state.settings.upload_dir.resolve()).as_posix(), file_hash="b" * 64, file_size_bytes=len(stored_path.read_bytes()), ) diff --git a/tests/ui/test_media_urls.py b/tests/ui/test_media_urls.py index 943a793..55f2584 100644 --- a/tests/ui/test_media_urls.py +++ b/tests/ui/test_media_urls.py @@ -4,24 +4,21 @@ from transcription.ui.components.media_urls import resolve_media_url def test_resolve_media_url_maps_managed_absolute_path_to_upload_route(tmp_path): upload_dir = tmp_path / "uploads" - managed_path = upload_dir / "documents" / "abc" / "page.jpg" - managed_path.parent.mkdir(parents=True, exist_ok=True) - managed_path.write_bytes(b"x") + canonical_path = upload_dir / "documents" / "abc" / "page.jpg" + canonical_path.parent.mkdir(parents=True, exist_ok=True) + canonical_path.write_bytes(b"x") - resolved = resolve_media_url(str(managed_path), upload_dir=upload_dir, base_url="http://localhost:8000") + resolved = resolve_media_url("documents/abc/page.jpg", upload_dir=upload_dir, base_url="http://localhost:8000") assert resolved == "http://localhost:8000/uploads/documents/abc/page.jpg" -def test_resolve_media_url_rejects_unmanaged_absolute_path(tmp_path): +def test_resolve_media_url_rejects_absolute_filesystem_path(tmp_path): upload_dir = tmp_path / "uploads" - managed_path = upload_dir / "documents" / "abc" / "page.jpg" - managed_path.parent.mkdir(parents=True, exist_ok=True) - managed_path.write_bytes(b"x") - unmanaged_path = tmp_path / "other-root" / "secret" / "page.jpg" - unmanaged_path.parent.mkdir(parents=True, exist_ok=True) - unmanaged_path.write_bytes(b"x") + absolute_path = upload_dir / "documents" / "abc" / "page.jpg" + absolute_path.parent.mkdir(parents=True, exist_ok=True) + absolute_path.write_bytes(b"x") - resolved = resolve_media_url(str(unmanaged_path), upload_dir=upload_dir, base_url="http://localhost:8000") + resolved = resolve_media_url(str(absolute_path), upload_dir=upload_dir, base_url="http://localhost:8000") assert resolved is None @@ -36,21 +33,9 @@ def test_resolve_media_url_rejects_stale_relative_path(tmp_path): assert resolved is None -def test_resolve_media_url_rejects_basename_collision_from_unmanaged_path(tmp_path): +def test_resolve_media_url_rejects_non_canonical_relative_path(tmp_path): upload_dir = tmp_path / "uploads" - managed_path = upload_dir / "documents" / "abc" / "shared-name.jpg" - managed_path.parent.mkdir(parents=True, exist_ok=True) - managed_path.write_bytes(b"managed") - - unmanaged_path = tmp_path / "scratch" / "shared-name.jpg" - unmanaged_path.parent.mkdir(parents=True, exist_ok=True) - unmanaged_path.write_bytes(b"unmanaged") - - resolved = resolve_media_url( - str(unmanaged_path), - upload_dir=upload_dir, - base_url="http://localhost:8000", - ) + resolved = resolve_media_url("shared-name.jpg", upload_dir=upload_dir, base_url="http://localhost:8000") assert resolved is None @@ -68,28 +53,18 @@ def test_resolve_media_url_accepts_existing_upload_relative_path(tmp_path): assert resolved == "http://localhost:8000/uploads/documents/abc/page.jpg" -def test_resolve_media_url_accepts_existing_portraits_relative_path(tmp_path): +def test_resolve_media_url_accepts_existing_persons_relative_path(tmp_path): upload_dir = tmp_path / "uploads" - managed_path = upload_dir / "portraits" / "person" / "seeded.png" + managed_path = upload_dir / "persons" / "person" / "seeded.png" managed_path.parent.mkdir(parents=True, exist_ok=True) managed_path.write_bytes(b"x") resolved = resolve_media_url( - "portraits/person/seeded.png", + "persons/person/seeded.png", upload_dir=upload_dir, base_url="http://localhost:8000", ) - assert resolved == "http://localhost:8000/uploads/portraits/person/seeded.png" - - -def test_public_media_path_label_maps_managed_absolute_path_to_upload_route(tmp_path): - upload_dir = tmp_path / "uploads" - managed_path = upload_dir / "documents" / "abc" / "page.jpg" - managed_path.parent.mkdir(parents=True, exist_ok=True) - managed_path.write_bytes(b"x") - - label = public_media_path_label(str(managed_path), upload_dir=upload_dir) - assert label == "/uploads/documents/abc/page.jpg" + assert resolved == "http://localhost:8000/uploads/persons/person/seeded.png" def test_public_media_path_label_hides_unmanaged_absolute_path(tmp_path): diff --git a/tests/ui/test_people_page.py b/tests/ui/test_people_page.py index d0f07b4..5180647 100644 --- a/tests/ui/test_people_page.py +++ b/tests/ui/test_people_page.py @@ -115,14 +115,14 @@ class TestPeoplePageRendering: app, client = app_client upload_dirs = {app.state.settings.upload_dir, get_settings().upload_dir} for upload_dir in upload_dirs: - portrait_file = upload_dir / "portraits" / "person" / "seeded.png" + portrait_file = upload_dir / "persons" / "person" / "seeded.png" portrait_file.parent.mkdir(parents=True, exist_ok=True) portrait_file.write_bytes(b"portrait") async with session_scope() as session: person = Person( full_name="Portrait Person", - portrait_path="portraits/person/seeded.png", + portrait_path="persons/person/seeded.png", ) session.add(person) await session.commit() diff --git a/tests/ui/test_print_preview_page.py b/tests/ui/test_print_preview_page.py index 8d66ead..86106c5 100644 --- a/tests/ui/test_print_preview_page.py +++ b/tests/ui/test_print_preview_page.py @@ -42,7 +42,7 @@ async def test_document_print_preview_and_safe_media_route(app_client): page_number=1, upload_name="print-page.png", filename="print-page.png", - file_path=str(media_path), + file_path="documents/print-page.png", file_hash="a" * 64, file_size_bytes=media_path.stat().st_size, raw_transcription="line one\nline two", @@ -54,7 +54,7 @@ async def test_document_print_preview_and_safe_media_route(app_client): page_number=2, upload_name="print-page.pdf", filename="print-page.pdf", - file_path=str(pdf_path), + file_path="documents/print-page.pdf", file_hash="c" * 64, file_size_bytes=pdf_path.stat().st_size, raw_transcription="PDF source", @@ -102,7 +102,7 @@ async def test_document_source_media_rejects_cross_document_access(app_client): page_number=1, upload_name="other.png", filename="other.png", - file_path=str(media_path), + file_path="documents/other.png", file_hash="b" * 64, file_size_bytes=media_path.stat().st_size, ) diff --git a/tools/export_import_migration.py b/tools/export_import_migration.py new file mode 100644 index 0000000..29847fa --- /dev/null +++ b/tools/export_import_migration.py @@ -0,0 +1,119 @@ +"""Formal export/import migration workflow for rebuilding to current schema. + +Usage examples: + +1) Export current DB + uploads to a bundle: + uv run python tools/export_import_migration.py export --bundle-dir .migration-bundle + +2) Import bundle into a fresh target DB + uploads root: + uv run python tools/export_import_migration.py import --bundle-dir .migration-bundle --target-db .\\data\\transcription-new.db --target-upload-dir .\\data-new + +3) One-shot rebuild flow: + uv run python tools/export_import_migration.py migrate --bundle-dir .migration-bundle --target-db .\\data\\transcription-new.db --target-upload-dir .\\data-new +""" + +from __future__ import annotations + +import argparse +from pathlib import Path +from typing import Final + +from transcription.config import get_settings +from transcription.db.migration import MigrationPaths +from transcription.db.migration import default_sync_db_url +from transcription.db.migration import export_bundle +from transcription.db.migration import import_bundle +from transcription.db.migration import migrate_via_bundle +from transcription.db.migration import sqlite_url_from_path + +DEFAULT_BUNDLE_DIR: Final[Path] = Path(".migration-bundle") + + +def _db_url(value: str | None, *, default_url: str) -> str: + if value is None or not value.strip(): + return default_url + candidate = value.strip() + if "://" in candidate: + return candidate + return sqlite_url_from_path(Path(candidate)) + + +def _path(value: str | None, *, default_path: Path) -> Path: + if value is None or not value.strip(): + return default_path + return Path(value) + + +def main() -> int: + settings = get_settings() + parser = argparse.ArgumentParser(description=__doc__) + subparsers = parser.add_subparsers(dest="command", required=True) + + common = argparse.ArgumentParser(add_help=False) + common.add_argument("--bundle-dir", default=str(DEFAULT_BUNDLE_DIR), help="Directory for export bundle artifacts") + common.add_argument( + "--source-db", + default=None, + help="Source database path or SQLAlchemy URL (default: current configured database)", + ) + common.add_argument( + "--source-upload-dir", + default=None, + help="Source upload directory (default: current UPLOAD_DIR setting)", + ) + common.add_argument( + "--target-db", + default=None, + help="Target database path or SQLAlchemy URL (required for import/migrate if not using current DB)", + ) + common.add_argument( + "--target-upload-dir", + default=None, + help="Target upload directory (required for import/migrate if not using current UPLOAD_DIR)", + ) + + subparsers.add_parser("export", parents=[common], help="Export DB rows and uploads into bundle") + subparsers.add_parser("import", parents=[common], help="Import bundle into target DB and uploads") + subparsers.add_parser("migrate", parents=[common], help="Run export then import in one command") + + args = parser.parse_args() + bundle_dir = Path(args.bundle_dir) + + source_db_url = _db_url(args.source_db, default_url=default_sync_db_url(settings)) + source_upload_dir = _path(args.source_upload_dir, default_path=settings.upload_dir) + target_db_url = _db_url(args.target_db, default_url=default_sync_db_url(settings)) + target_upload_dir = _path(args.target_upload_dir, default_path=settings.upload_dir) + + if args.command == "export": + export_bundle( + source_db_url=source_db_url, + source_upload_dir=source_upload_dir, + bundle_dir=bundle_dir, + ) + print(f"Export complete: {bundle_dir}") + return 0 + + if args.command == "import": + import_bundle( + target_db_url=target_db_url, + target_upload_dir=target_upload_dir, + bundle_dir=bundle_dir, + ) + print(f"Import complete: db={target_db_url} uploads={target_upload_dir}") + return 0 + + migrate_via_bundle( + MigrationPaths( + source_db_url=source_db_url, + target_db_url=target_db_url, + source_upload_dir=source_upload_dir, + target_upload_dir=target_upload_dir, + bundle_dir=bundle_dir, + ) + ) + print(f"Migration complete: bundle={bundle_dir} target_db={target_db_url} target_upload_dir={target_upload_dir}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main())