Continue GC code review and cleanup

This commit is contained in:
Jim Lancaster
2026-08-11 16:42:09 -05:00
parent 8d5aec4301
commit b8be27f0c9
16 changed files with 363 additions and 161 deletions
+13 -4
View File
@@ -145,14 +145,15 @@ The canonical MVP prompt is:
## Destructive test procedure (with data backup) ## Destructive test procedure (with data backup)
AI execution policy: before running any unit tests, create a backup of `./data` first. After tests succeed, always pause and ask whether to restore now. 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.
Use the cross-platform Python wrapper below whenever an AI agent runs tests against this repository. Use the cross-platform Python wrapper below whenever an AI agent runs tests against this repository.
1. Create backup of `./data`. 1. Create one backup of `./data` and mark it as the active test-cycle backup.
2. Run your test command. 2. Run your test command.
3. On success, always prompt whether to restore now (do not auto-restore unless explicitly approved). 3. On failure, fix the errors and run the wrapper again; it reuses the active backup and never backs up post-test data.
4. On failure, keep backup and current state for inspection. 4. On success, always prompt whether to restore now (do not auto-restore unless explicitly approved).
5. Close the cycle only by restoring the active backup or explicitly accepting the current data.
Preflight behavior: Preflight behavior:
@@ -183,10 +184,18 @@ uv run python tools/run_destructive_tests.py --skip-restore-prompt -- pytest
This keeps both the current post-test state and the backup, so restore can be decided explicitly later. This keeps both the current post-test state and the backup, so restore can be decided explicitly later.
Repeated wrapper invocations reuse the backup recorded in `.test-backups/.active-backup`. If that backup is missing, the wrapper stops rather than creating a replacement from potentially destructive post-test data.
### Restore later from a saved backup ### Restore later from a saved backup
```bash ```bash
uv run python tools/run_destructive_tests.py --restore-from data-backup-YYYYMMDD-HHMMSS uv run python tools/run_destructive_tests.py --restore-from data-backup-YYYYMMDD-HHMMSS
``` ```
To keep the current data and close the active cycle without restoring:
```bash
uv run python tools/run_destructive_tests.py --accept-current-data
```
Backups are stored in `.test-backups/` and ignored by git. Backups are stored in `.test-backups/` and ignored by git.
+11 -6
View File
@@ -24,7 +24,10 @@ from .db import create_all
from .db import dispose_database_runtime from .db import dispose_database_runtime
from .db import initialize_database_runtime from .db import initialize_database_runtime
from .services import ServiceBundle from .services import ServiceBundle
from .services.documents import DocumentService
from .services.jobs import JobService from .services.jobs import JobService
from .services.people import PeopleService
from .services.sources import SourceService
from .ui import register_pages from .ui import register_pages
from .worker import worker_consumer_lifespan from .worker import worker_consumer_lifespan
@@ -36,8 +39,14 @@ async def _lifespan(app: FastAPI):
settings = getattr(app.state, "settings", None) or get_settings() settings = getattr(app.state, "settings", None) or get_settings()
configure_logging(settings) configure_logging(settings)
app.state.settings = settings app.state.settings = settings
app.state.services = ServiceBundle()
app.state.runtime = initialize_database_runtime(settings=settings) app.state.runtime = initialize_database_runtime(settings=settings)
session_factory = app.state.runtime.session_factory
app.state.services = ServiceBundle(
documents=DocumentService(session_factory=session_factory, settings=settings),
sources=SourceService(session_factory=session_factory, settings=settings),
jobs=JobService(session_factory=session_factory, settings=settings),
people=PeopleService(session_factory=session_factory, settings=settings),
)
if settings.should_bootstrap_schema: if settings.should_bootstrap_schema:
await create_all(engine=app.state.runtime.engine) await create_all(engine=app.state.runtime.engine)
@@ -93,12 +102,8 @@ def create_app(settings: Settings | None = None) -> FastAPI:
async def ui_redirect() -> RedirectResponse: async def ui_redirect() -> RedirectResponse:
return RedirectResponse(url="/ui/homepage", status_code=status.HTTP_307_TEMPORARY_REDIRECT) return RedirectResponse(url="/ui/homepage", status_code=status.HTTP_307_TEMPORARY_REDIRECT)
@app.get("/healthz")
def health() -> dict[str, str]:
return {"status": "ok"}
register_error_handlers(app) register_error_handlers(app)
register_pages(app)
app.include_router(health_router) app.include_router(health_router)
app.include_router(v4_documents_router) app.include_router(v4_documents_router)
register_pages(app)
return app return app
+12 -6
View File
@@ -1,5 +1,4 @@
import logging import logging
from contextvars import ContextVar
from dataclasses import dataclass from dataclasses import dataclass
from sqlalchemy.ext.asyncio import AsyncEngine from sqlalchemy.ext.asyncio import AsyncEngine
@@ -23,21 +22,28 @@ class DatabaseRuntime:
session_factory: async_sessionmaker[AsyncSession] session_factory: async_sessionmaker[AsyncSession]
_runtime: ContextVar[DatabaseRuntime | None] = ContextVar("database_runtime", default=None) _runtime: DatabaseRuntime | None = None
def get_database_runtime() -> DatabaseRuntime | None:
"""Return the process-owned database runtime."""
return _runtime
async def dispose_database_runtime() -> None: async def dispose_database_runtime() -> None:
"""Dispose lifespan-owned async database resources.""" """Dispose lifespan-owned async database resources."""
runtime = _runtime.get() global _runtime
runtime = _runtime
if runtime is None: if runtime is None:
return return
await runtime.engine.dispose() await runtime.engine.dispose()
_runtime.set(None) _runtime = None
def initialize_database_runtime(*, settings: Settings | None = None) -> DatabaseRuntime: def initialize_database_runtime(*, settings: Settings | None = None) -> DatabaseRuntime:
"""Initialize lifespan-owned async DB resources once per process.""" """Initialize lifespan-owned async DB resources once per process."""
runtime = _runtime.get() global _runtime
runtime = _runtime
if runtime is not None: if runtime is not None:
return runtime return runtime
@@ -46,6 +52,6 @@ def initialize_database_runtime(*, settings: Settings | None = None) -> Database
engine = get_engine(database_url) engine = get_engine(database_url)
session_factory = get_session_factory(database_url) session_factory = get_session_factory(database_url)
runtime = DatabaseRuntime(engine=engine, session_factory=session_factory) runtime = DatabaseRuntime(engine=engine, session_factory=session_factory)
_runtime.set(runtime) _runtime = runtime
logger.debug("Initialized async database runtime for database_url=%s", engine.url) logger.debug("Initialized async database runtime for database_url=%s", engine.url)
return runtime return runtime
+6
View File
@@ -33,6 +33,12 @@ def resolve_session_factory(
) -> SessionFactory: ) -> SessionFactory:
if database_url is not None: if database_url is not None:
return get_session_factory(database_url) return get_session_factory(database_url)
if settings is None:
from .runtime import get_database_runtime
runtime = get_database_runtime()
if runtime is not None:
return runtime.session_factory
return get_session_factory(get_database_url(settings or get_settings())) return get_session_factory(get_database_url(settings or get_settings()))
+1 -1
View File
@@ -23,8 +23,8 @@ class TranscriptionResult:
text: str text: str
provider: str provider: str
prompt_name: str
model: str model: str
prompt_name: str | None = None
prompt_hash: str | None = None prompt_hash: str | None = None
system_prompt: str | None = None system_prompt: str | None = None
user_prompt: str | None = None user_prompt: str | None = None
+18 -3
View File
@@ -95,7 +95,7 @@ class OpenRouterTranscriptionProvider:
return TranscriptionResult( return TranscriptionResult(
text=text, text=text,
provider="openrouter", provider="openrouter",
prompt_name="", prompt_name=None,
prompt_hash=None, prompt_hash=None,
system_prompt=None, system_prompt=None,
user_prompt=prompt_text, user_prompt=prompt_text,
@@ -158,7 +158,8 @@ class OpenRouterTranscriptionProvider:
if callable(serializer): if callable(serializer):
try: try:
return self._to_json_compatible(serializer()) return self._to_json_compatible(serializer())
except Exception: # noqa: BLE001 except Exception as exc: # noqa: BLE001
logger.debug("OpenRouter response serializer %s failed: %s", method_name, exc)
continue continue
object_dict = getattr(value, "__dict__", None) object_dict = getattr(value, "__dict__", None)
@@ -182,13 +183,27 @@ class OpenRouterTranscriptionProvider:
) -> OpenRouterRequest: ) -> OpenRouterRequest:
image_b64 = base64.b64encode(image_bytes).decode("ascii") image_b64 = base64.b64encode(image_bytes).decode("ascii")
data_url = f"data:{mime_type};base64,{image_b64}" data_url = f"data:{mime_type};base64,{image_b64}"
media_content: dict[str, Any]
if mime_type == "application/pdf":
media_content = {
"type": "file",
"file": {
"filename": "source.pdf",
"file_data": data_url,
},
}
else:
media_content = {
"type": "image_url",
"image_url": {"url": data_url},
}
messages: list[dict[str, Any]] = [ messages: list[dict[str, Any]] = [
{ {
"role": "user", "role": "user",
"content": [ "content": [
{"type": "text", "text": prompt_text}, {"type": "text", "text": prompt_text},
{"type": "image_url", "image_url": {"url": data_url}}, media_content,
], ],
} }
] ]
+3 -2
View File
@@ -23,9 +23,10 @@ class ServiceBase(ABC):
self, self,
session_factory: async_sessionmaker[AsyncSession] | None = None, session_factory: async_sessionmaker[AsyncSession] | None = None,
queue: asyncio.Queue | None = None, queue: asyncio.Queue | None = None,
settings: Settings | None = None,
): ):
self.settings = get_settings() self.settings = settings or get_settings()
self.session_factory = session_factory or resolve_session_factory() self.session_factory = session_factory or resolve_session_factory(settings=self.settings)
self.queue = queue or asyncio.Queue() self.queue = queue or asyncio.Queue()
@asynccontextmanager @asynccontextmanager
+19 -7
View File
@@ -7,13 +7,13 @@ from sqlalchemy.orm import selectinload
from sqlmodel import select from sqlmodel import select
from sqlmodel.ext.asyncio.session import AsyncSession from sqlmodel.ext.asyncio.session import AsyncSession
from ..errors import AppError
from ..errors import ErrorCategory
from ..db.models import Job from ..db.models import Job
from ..db.models import JobSource from ..db.models import JobSource
from ..db.models import JobSourceStatus from ..db.models import JobSourceStatus
from ..db.models import JobStatus from ..db.models import JobStatus
from ..db.models import Source from ..db.models import Source
from ..errors import AppError
from ..errors import ErrorCategory
from .base import ServiceBase from .base import ServiceBase
@@ -29,6 +29,10 @@ class JobResubmitBlockedError(AppError):
"""Raised when a job resubmit operation is blocked by lifecycle policy.""" """Raised when a job resubmit operation is blocked by lifecycle policy."""
class JobNotFoundError(AppError):
"""Raised when a requested Job does not exist."""
class JobService(ServiceBase): class JobService(ServiceBase):
"""Thin service class for managing jobs in the database.""" """Thin service class for managing jobs in the database."""
@@ -61,7 +65,7 @@ class JobService(ServiceBase):
) )
job = (await _session.exec(query)).first() job = (await _session.exec(query)).first()
if job is None: if job is None:
raise ValueError(f"Job with id {job_id} not found") raise self._not_found(job_id)
return job return job
async def update_job(self, job: Job, session: AsyncSession | None = None) -> Job: async def update_job(self, job: Job, session: AsyncSession | None = None) -> Job:
@@ -148,7 +152,7 @@ class JobService(ServiceBase):
) )
job = (await _session.exec(query)).first() job = (await _session.exec(query)).first()
if job is None: if job is None:
raise ValueError(f"Job with id {job_id} not found") raise self._not_found(job_id)
job.status = status job.status = status
if retry_count_increment: if retry_count_increment:
job.retry_count += retry_count_increment job.retry_count += retry_count_increment
@@ -216,7 +220,7 @@ class JobService(ServiceBase):
) )
job = (await _session.exec(query)).first() job = (await _session.exec(query)).first()
if job is None: if job is None:
raise ValueError(f"Job with id {job_id} not found") raise self._not_found(job_id)
if job.status == JobStatus.PROCESSING: if job.status == JobStatus.PROCESSING:
raise JobDeleteBlockedError( raise JobDeleteBlockedError(
@@ -244,7 +248,7 @@ class JobService(ServiceBase):
) )
job = (await _session.exec(query)).first() job = (await _session.exec(query)).first()
if job is None: if job is None:
raise ValueError(f"Job with id {job_id} not found") raise self._not_found(job_id)
if job.status in {JobStatus.TRANSCRIBED, JobStatus.COMPLETED}: if job.status in {JobStatus.TRANSCRIBED, JobStatus.COMPLETED}:
raise JobCancelBlockedError( raise JobCancelBlockedError(
@@ -283,7 +287,7 @@ class JobService(ServiceBase):
) )
job = (await _session.exec(query)).first() job = (await _session.exec(query)).first()
if job is None: if job is None:
raise ValueError(f"Job with id {job_id} not found") raise self._not_found(job_id)
if job.status == JobStatus.PROCESSING: if job.status == JobStatus.PROCESSING:
raise JobResubmitBlockedError( raise JobResubmitBlockedError(
@@ -314,3 +318,11 @@ class JobService(ServiceBase):
await self._finalize(session=_session, caller_session=session, refresh=(job,)) await self._finalize(session=_session, caller_session=session, refresh=(job,))
return len(candidates) return len(candidates)
@staticmethod
def _not_found(job_id: UUID) -> JobNotFoundError:
return JobNotFoundError(
f"Job with id {job_id} not found",
category=ErrorCategory.NOT_FOUND,
suggestion="Verify the Job id and retry.",
)
+6 -2
View File
@@ -81,8 +81,12 @@ class SourceService(ServiceBase):
provider: TranscriptionProvider provider: TranscriptionProvider
def __init__(self, session_factory: async_sessionmaker[AsyncSession] | None = None): def __init__(
super().__init__(session_factory=session_factory) self,
session_factory: async_sessionmaker[AsyncSession] | None = None,
settings: Settings | None = None,
):
super().__init__(session_factory=session_factory, settings=settings)
self.provider = get_transcription_provider(settings=self.settings) self.provider = get_transcription_provider(settings=self.settings)
async def create_source(self, source: Source, *, session: AsyncSession | None = None) -> Source: async def create_source(self, source: Source, *, session: AsyncSession | None = None) -> Source:
-22
View File
@@ -9,7 +9,6 @@ from contextlib import asynccontextmanager
from contextlib import contextmanager from contextlib import contextmanager
from contextlib import suppress from contextlib import suppress
from typing import Protocol from typing import Protocol
from uuid import UUID
from sqlalchemy.ext.asyncio import async_sessionmaker from sqlalchemy.ext.asyncio import async_sessionmaker
from sqlmodel.ext.asyncio.session import AsyncSession from sqlmodel.ext.asyncio.session import AsyncSession
@@ -23,7 +22,6 @@ from .services.documents import DocumentService
from .services.jobs import JobService from .services.jobs import JobService
from .services.people import PeopleService from .services.people import PeopleService
from .services.sources import SourceService from .services.sources import SourceService
from .services.workflows import advance_job
from .services.workflows import process_next_queued_job as process_next_queued_job_workflow from .services.workflows import process_next_queued_job as process_next_queued_job_workflow
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@@ -96,19 +94,6 @@ async def worker_consumer_lifespan(
await worker_task await worker_task
async def queue_consumer_loop(queue: asyncio.Queue[UUID], stop_event: asyncio.Event):
"""Main worker loop that consumes jobs from the queue and processes them.
The queue contains Job UUIDs whose Document and Source records already exist.
"""
service = JobService()
while not stop_event.is_set():
with handle_worker_exceptions():
async with _get_queue_item(queue) as job_id:
job = await service.read_job(job_id)
asyncio.create_task(advance_job(job=job, services=ServiceBundle()))
@contextmanager @contextmanager
def handle_worker_exceptions(operation: str = "worker.loop"): def handle_worker_exceptions(operation: str = "worker.loop"):
"""Context manager to log and suppress exceptions in the worker loop.""" """Context manager to log and suppress exceptions in the worker loop."""
@@ -123,13 +108,6 @@ def handle_worker_exceptions(operation: str = "worker.loop"):
) )
@asynccontextmanager
async def _get_queue_item(queue: asyncio.Queue[UUID]) -> AsyncGenerator[UUID]:
"""Context manager to enqueue a job and ensure it is marked done."""
yield await queue.get()
queue.task_done()
async def run_worker_loop( async def run_worker_loop(
*, *,
session_factory: async_sessionmaker[AsyncSession] | None = None, session_factory: async_sessionmaker[AsyncSession] | None = None,
+22
View File
@@ -120,6 +120,7 @@ class TestOpenRouterProviderTranscribe:
assert result.text == "Line 1\nLine 2" assert result.text == "Line 1\nLine 2"
assert result.provider == "openrouter" assert result.provider == "openrouter"
assert result.prompt_name is None
assert result.model == "vendor/model-b" assert result.model == "vendor/model-b"
assert result.ai_metadata == { assert result.ai_metadata == {
"finish_reason": "stop", "finish_reason": "stop",
@@ -142,6 +143,27 @@ class TestOpenRouterProviderTranscribe:
mime_type="image/png", mime_type="image/png",
) )
@pytest.mark.asyncio
async def test_sends_pdf_as_file_content(self):
"""PDF payloads use OpenRouter's file content contract."""
response = {"model": "vendor/model-a", "choices": [{"message": {"content": "Transcript text"}}]}
client = _FakeClient(response=response)
provider = OpenRouterTranscriptionProvider(
settings=Settings(openrouter_api_key="test-key"),
client=client,
)
await provider.transcribe(
prompt_text="Prompt body",
image_bytes=b"pdf-bytes",
mime_type="application/pdf",
)
content = client.chat.calls[0]["messages"][0]["content"]
assert content[1]["type"] == "file"
assert content[1]["file"]["filename"] == "source.pdf"
assert content[1]["file"]["file_data"].startswith("data:application/pdf;base64,")
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_raises_on_empty_or_invalid_response(self): async def test_raises_on_empty_or_invalid_response(self):
"""Transcribe raises ProviderResponseError for missing completion text.""" """Transcribe raises ProviderResponseError for missing completion text."""
+10 -5
View File
@@ -1,7 +1,7 @@
from uuid import uuid4
from datetime import UTC from datetime import UTC
from datetime import datetime from datetime import datetime
from datetime import timedelta from datetime import timedelta
from uuid import uuid4
import pytest import pytest
@@ -12,8 +12,9 @@ from transcription.db.models import JobSourceStatus
from transcription.db.models import JobStatus from transcription.db.models import JobStatus
from transcription.db.models import Source from transcription.db.models import Source
from transcription.services.documents import DocumentService from transcription.services.documents import DocumentService
from transcription.services.jobs import JobDeleteBlockedError
from transcription.services.jobs import JobCancelBlockedError from transcription.services.jobs import JobCancelBlockedError
from transcription.services.jobs import JobDeleteBlockedError
from transcription.services.jobs import JobNotFoundError
from transcription.services.jobs import JobResubmitBlockedError from transcription.services.jobs import JobResubmitBlockedError
from transcription.services.jobs import JobService from transcription.services.jobs import JobService
@@ -228,7 +229,7 @@ class TestJobService:
await job_service.delete_job_with_guardrails(job_id=job.id) await job_service.delete_job_with_guardrails(job_id=job.id)
with pytest.raises(ValueError): with pytest.raises(JobNotFoundError):
await job_service.read_job(job_id=job.id) await job_service.read_job(job_id=job.id)
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -355,8 +356,12 @@ class TestJobService:
refreshed = await job_service.read_job(job_id=job.id) refreshed = await job_service.read_job(job_id=job.id)
assert refreshed.status == JobStatus.QUEUED assert refreshed.status == JobStatus.QUEUED
failed_entry = next(item for item in refreshed.job_sources if item.source is not None and item.source.page_number == 1) failed_entry = next(
transcribed_entry = next(item for item in refreshed.job_sources if item.source is not None and item.source.page_number == 2) item for item in refreshed.job_sources if item.source is not None and item.source.page_number == 1
)
transcribed_entry = next(
item for item in refreshed.job_sources if item.source is not None and item.source.page_number == 2
)
assert failed_entry.status == JobSourceStatus.PENDING assert failed_entry.status == JobSourceStatus.PENDING
assert failed_entry.error_detail is None assert failed_entry.error_detail is None
assert failed_entry.source is not None assert failed_entry.source is not None
+23 -16
View File
@@ -7,18 +7,19 @@ from fastapi import FastAPI
from fastapi.testclient import TestClient from fastapi.testclient import TestClient
from transcription.app import create_app from transcription.app import create_app
from transcription.config import Settings
@pytest.mark.unit @pytest.mark.unit
class TestAppFactory: class TestAppFactory:
"""Verify FastAPI app factory wiring.""" """Verify FastAPI app factory wiring."""
def test_create_app_returns_fastapi_instance(self): def test_create_app_returns_fastapi_instance(self, monkeypatch):
"""create_app returns a FastAPI application instance.""" """create_app returns a FastAPI application instance."""
monkeypatch.setattr("transcription.app.register_pages", lambda _app: None)
app = create_app() app = create_app()
assert isinstance(app, FastAPI) assert isinstance(app, FastAPI)
@pytest.mark.integration @pytest.mark.integration
class TestAppLifespan: class TestAppLifespan:
"""Verify startup and shutdown lifecycle behavior.""" """Verify startup and shutdown lifecycle behavior."""
@@ -28,6 +29,7 @@ class TestAppLifespan:
calls = [] calls = []
monkeypatch.setattr("transcription.app.configure_logging", lambda _settings: calls.append("logging")) monkeypatch.setattr("transcription.app.configure_logging", lambda _settings: calls.append("logging"))
monkeypatch.setattr("transcription.app.register_pages", lambda _app: None)
async def _create_all(**_kwargs): async def _create_all(**_kwargs):
calls.append("schema") calls.append("schema")
@@ -56,12 +58,14 @@ class TestAppLifespan:
monkeypatch.setattr("transcription.app.worker_consumer_lifespan", _worker_lifespan) monkeypatch.setattr("transcription.app.worker_consumer_lifespan", _worker_lifespan)
class _Settings: settings = Settings(
should_bootstrap_schema = True openrouter_api_key="test-key",
upload_dir = tmp_path / "uploads" environment="test",
prompt_dir = tmp_path / "prompts" bootstrap_schema_on_startup=True,
upload_dir=tmp_path / "uploads",
monkeypatch.setattr("transcription.app.get_settings", lambda: _Settings()) prompt_dir=tmp_path / "prompts",
)
monkeypatch.setattr("transcription.app.get_settings", lambda: settings)
app = create_app() app = create_app()
with TestClient(app): with TestClient(app):
@@ -73,14 +77,15 @@ class TestAppLifespan:
assert "worker_start" in calls assert "worker_start" in calls
assert "worker_stop" in calls assert "worker_stop" in calls
assert "dispose_db" in calls assert "dispose_db" in calls
assert _Settings.upload_dir.exists() assert settings.upload_dir.exists()
assert _Settings.prompt_dir.exists() assert settings.prompt_dir.exists()
def test_shutdown_stops_worker_resources(self, monkeypatch, tmp_path): def test_shutdown_stops_worker_resources(self, monkeypatch, tmp_path):
"""Shutdown signals and stops worker resources cleanly.""" """Shutdown signals and stops worker resources cleanly."""
calls = [] calls = []
monkeypatch.setattr("transcription.app.configure_logging", lambda _settings: calls.append("logging")) monkeypatch.setattr("transcription.app.configure_logging", lambda _settings: calls.append("logging"))
monkeypatch.setattr("transcription.app.register_pages", lambda _app: None)
async def _create_all(**_kwargs): async def _create_all(**_kwargs):
calls.append("schema") calls.append("schema")
@@ -109,12 +114,14 @@ class TestAppLifespan:
monkeypatch.setattr("transcription.app.worker_consumer_lifespan", _worker_lifespan) monkeypatch.setattr("transcription.app.worker_consumer_lifespan", _worker_lifespan)
class _Settings: settings = Settings(
should_bootstrap_schema = True openrouter_api_key="test-key",
upload_dir = tmp_path / "uploads" environment="test",
prompt_dir = tmp_path / "prompts" bootstrap_schema_on_startup=True,
upload_dir=tmp_path / "uploads",
monkeypatch.setattr("transcription.app.get_settings", lambda: _Settings()) prompt_dir=tmp_path / "prompts",
)
monkeypatch.setattr("transcription.app.get_settings", lambda: settings)
app = create_app() app = create_app()
with TestClient(app): with TestClient(app):
+50
View File
@@ -0,0 +1,50 @@
from pathlib import Path
import pytest
from tools import run_destructive_tests
def test_reuses_initial_backup_across_test_attempts(tmp_path: Path) -> None:
data_path = tmp_path / "data"
backup_root = tmp_path / ".test-backups"
data_path.mkdir()
backup_root.mkdir()
(data_path / "transcription.db").write_text("original", encoding="utf-8")
initial_backup = run_destructive_tests.create_or_reuse_backup(data_path, backup_root)
(data_path / "transcription.db").write_text("overwritten", encoding="utf-8")
reused_backup = run_destructive_tests.create_or_reuse_backup(data_path, backup_root)
assert reused_backup == initial_backup
assert (reused_backup / "transcription.db").read_text(encoding="utf-8") == "original"
assert [path for path in backup_root.iterdir() if path.is_dir()] == [initial_backup]
def test_missing_active_backup_stops_instead_of_replacing_it(tmp_path: Path) -> None:
data_path = tmp_path / "data"
backup_root = tmp_path / ".test-backups"
data_path.mkdir()
backup_root.mkdir()
(data_path / "transcription.db").write_text("post-test", encoding="utf-8")
(backup_root / run_destructive_tests.ACTIVE_BACKUP_FILENAME).write_text("data-backup-missing", encoding="utf-8")
with pytest.raises(FileNotFoundError, match="Active backup is missing"):
run_destructive_tests.create_or_reuse_backup(data_path, backup_root)
assert not any(path.is_dir() for path in backup_root.iterdir())
def test_closing_cycle_preserves_backup(tmp_path: Path) -> None:
data_path = tmp_path / "data"
backup_root = tmp_path / ".test-backups"
data_path.mkdir()
backup_root.mkdir()
(data_path / "transcription.db").write_text("original", encoding="utf-8")
backup_path = run_destructive_tests.create_or_reuse_backup(data_path, backup_root)
closed_backup = run_destructive_tests.close_active_backup_cycle(backup_root)
assert closed_backup == backup_path
assert backup_path.is_dir()
assert not (backup_root / run_destructive_tests.ACTIVE_BACKUP_FILENAME).exists()
+32 -26
View File
@@ -2,11 +2,12 @@
from __future__ import annotations from __future__ import annotations
import asyncio from collections.abc import AsyncGenerator
from collections.abc import AsyncGenerator, Callable from collections.abc import Awaitable
from datetime import UTC, datetime from collections.abc import Callable
from datetime import UTC
from datetime import datetime
from pathlib import Path from pathlib import Path
from typing import Awaitable
from uuid import UUID from uuid import UUID
import pytest import pytest
@@ -16,45 +17,52 @@ from fastapi.testclient import TestClient
from sqlmodel import delete from sqlmodel import delete
from transcription.app import create_app from transcription.app import create_app
from transcription.config import Settings, SqliteSettings from transcription.config import Settings
from transcription.db import create_all, initialize_database_runtime, session_scope from transcription.config import SqliteSettings
from transcription.db.models import ( from transcription.db import session as db_session_module
Document, from transcription.db import session_scope
DocumentPerson, from transcription.db.models import Document
Job, from transcription.db.models import DocumentPerson
JobSource, from transcription.db.models import Job
JobSourceStatus, from transcription.db.models import JobSource
JobStatus, from transcription.db.models import JobSourceStatus
Person, from transcription.db.models import JobStatus
Source, from transcription.db.models import Person
) from transcription.db.models import Source
@pytest.fixture(scope="session") @pytest.fixture(scope="session")
def app_client(tmp_path_factory: pytest.TempPathFactory) -> AsyncGenerator[tuple[FastAPI, TestClient], None]: def app_client(tmp_path_factory: pytest.TempPathFactory) -> AsyncGenerator[tuple[FastAPI, TestClient]]:
"""Provide a real application and test client backed by in-memory SQLite.""" """Provide a real application and test client backed by in-memory SQLite."""
tmp_path = tmp_path_factory.mktemp("ui") tmp_path = tmp_path_factory.mktemp("ui")
settings = Settings( settings = Settings(
openrouter_api_key="test-key", openrouter_api_key="test-key",
database=SqliteSettings(path=":memory:"), database=SqliteSettings(path=str(tmp_path / "ui-tests.db")),
environment="test", environment="test",
bootstrap_schema_on_startup=True, bootstrap_schema_on_startup=True,
upload_dir=tmp_path / "uploads", upload_dir=tmp_path / "uploads",
prompt_dir=tmp_path / "prompts", prompt_dir=tmp_path / "prompts",
) )
app = create_app() app = create_app(settings=settings)
app.state.runtime = initialize_database_runtime(settings=settings)
asyncio.run(create_all(engine=app.state.runtime.engine))
with TestClient(app) as client: with TestClient(app) as client:
yield app, client yield app, client
@pytest_asyncio.fixture(autouse=True) @pytest_asyncio.fixture(autouse=True)
async def clear_ui_database(app_client: tuple[FastAPI, TestClient]) -> None: async def clear_ui_database(
app_client: tuple[FastAPI, TestClient],
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""Reset UI-facing tables asynchronously before each test for isolation.""" """Reset UI-facing tables asynchronously before each test for isolation."""
async with session_scope() as session: app, _ = app_client
monkeypatch.setattr(
db_session_module,
"resolve_session_factory",
lambda *_args, **_kwargs: app.state.runtime.session_factory,
)
async with session_scope(session_factory=app.state.runtime.session_factory) as session:
await session.exec(delete(JobSource)) await session.exec(delete(JobSource))
await session.exec(delete(DocumentPerson)) await session.exec(delete(DocumentPerson))
await session.exec(delete(Source)) await session.exec(delete(Source))
@@ -118,9 +126,7 @@ async def seed_job(app_client: tuple[FastAPI, TestClient]) -> Callable[..., Awai
job_id=job.id, job_id=job.id,
source_id=source.id, source_id=source.id,
status=( status=(
JobSourceStatus.TRANSCRIBED JobSourceStatus.TRANSCRIBED if transcription_text is not None else JobSourceStatus.FAILED
if transcription_text is not None
else JobSourceStatus.FAILED
), ),
raw_transcription=transcription_text, raw_transcription=transcription_text,
error_detail=error_detail, error_detail=error_detail,
+135 -59
View File
@@ -3,16 +3,16 @@ from __future__ import annotations
import argparse import argparse
import ctypes import ctypes
import os import os
from ctypes import wintypes
from datetime import datetime
from pathlib import Path
import shlex import shlex
import shutil import shutil
import subprocess import subprocess
import sys import sys
from ctypes import wintypes
from datetime import datetime
from pathlib import Path
RETRY_CANCEL_CHOICES = {"n", "no", "c", "cancel", "a", "abort", "q", "quit"} RETRY_CANCEL_CHOICES = {"n", "no", "c", "cancel", "a", "abort", "q", "quit"}
ACTIVE_BACKUP_FILENAME = ".active-backup"
def show_phase(title: str) -> None: def show_phase(title: str) -> None:
@@ -88,11 +88,11 @@ def wait_for_restore_preflight(db_file_path: Path) -> bool:
while not test_file_unlocked(db_file_path): while not test_file_unlocked(db_file_path):
show_phase("Restore Preflight") show_phase("Restore Preflight")
print(f"WARNING: Restore preflight blocked: database appears to be in use: {db_file_path}") print(f"WARNING: Restore preflight blocked: database appears to be in use: {db_file_path}")
print("Close conflicting applications (for example DB Browser for SQLite, uvicorn, or any process using this DB).") print(
"Close conflicting applications (for example DB Browser for SQLite, uvicorn, or any process using this DB)."
)
print("Restore is paused and waiting for your input.") print("Restore is paused and waiting for your input.")
answer = input( answer = input(f"Attempt {attempt}: type 'retry' to check again, or 'cancel' to skip restore: ").strip()
f"Attempt {attempt}: type 'retry' to check again, or 'cancel' to skip restore: "
).strip()
print(f"Input received: '{answer}'") print(f"Input received: '{answer}'")
if answer.lower() in RETRY_CANCEL_CHOICES: if answer.lower() in RETRY_CANCEL_CHOICES:
return False return False
@@ -115,10 +115,61 @@ def restore_backup(backup_path: Path, data_path: Path) -> None:
print(f"Restored data from backup: {backup_path}") print(f"Restored data from backup: {backup_path}")
def active_backup_path(backup_root: Path) -> Path | None:
marker_path = backup_root / ACTIVE_BACKUP_FILENAME
if not marker_path.exists():
return None
backup_name = marker_path.read_text(encoding="utf-8").strip()
if not backup_name or Path(backup_name).name != backup_name:
raise RuntimeError(f"Invalid active backup marker: {marker_path}")
backup_path = backup_root / backup_name
if not backup_path.is_dir():
raise FileNotFoundError(
f"Active backup is missing: {backup_path}. "
f"Restore it or remove {marker_path} only after confirming the original data is safe."
)
return backup_path
def create_or_reuse_backup(data_path: Path, backup_root: Path) -> Path:
current_backup = active_backup_path(backup_root)
if current_backup is not None:
print(f"Reusing active test-cycle backup: {current_backup}")
return current_backup
db_path = data_path / "transcription.db"
if not test_file_unlocked(db_path):
print(f"WARNING: Backup preflight warning: database appears to be in use: {db_path}")
print("WARNING: Proceeding with backup, but hot backups can capture an in-flight state.")
timestamp = datetime.now().astimezone().strftime("%Y%m%d-%H%M%S")
backup_path = backup_root / f"data-backup-{timestamp}"
shutil.copytree(data_path, backup_path)
marker_path = backup_root / ACTIVE_BACKUP_FILENAME
temporary_marker_path = marker_path.with_suffix(".tmp")
temporary_marker_path.write_text(backup_path.name, encoding="utf-8")
temporary_marker_path.replace(marker_path)
print(f"Created test-cycle backup: {backup_path}")
return backup_path
def close_active_backup_cycle(backup_root: Path, backup_path: Path | None = None) -> Path:
current_backup = active_backup_path(backup_root)
if current_backup is None:
raise RuntimeError("No active test backup cycle exists.")
if backup_path is not None and current_backup.resolve() != backup_path.resolve():
return current_backup
(backup_root / ACTIVE_BACKUP_FILENAME).unlink()
print(f"Closed test backup cycle: {current_backup}")
return current_backup
def parse_args(argv: list[str]) -> argparse.Namespace: def parse_args(argv: list[str]) -> argparse.Namespace:
parser = argparse.ArgumentParser( parser = argparse.ArgumentParser(description="Run potentially destructive tests with backup/restore protection.")
description="Run potentially destructive tests with backup/restore protection."
)
parser.add_argument("--auto-restore", action="store_true", help="Restore immediately after successful tests.") parser.add_argument("--auto-restore", action="store_true", help="Restore immediately after successful tests.")
parser.add_argument("--keep-backup", action="store_true", help="Keep the backup even after a successful restore.") parser.add_argument("--keep-backup", action="store_true", help="Keep the backup even after a successful restore.")
parser.add_argument( parser.add_argument(
@@ -130,14 +181,22 @@ def parse_args(argv: list[str]) -> argparse.Namespace:
"--restore-from", "--restore-from",
help="Restore from an existing backup name or absolute backup path instead of running tests.", help="Restore from an existing backup name or absolute backup path instead of running tests.",
) )
parser.add_argument(
"--accept-current-data",
action="store_true",
help="Close the active test cycle without restoring; preserve its backup.",
)
parser.add_argument("command", nargs=argparse.REMAINDER, help="Command to run after '--'.") parser.add_argument("command", nargs=argparse.REMAINDER, help="Command to run after '--'.")
args = parser.parse_args(argv) args = parser.parse_args(argv)
if args.restore_from and args.command: if args.restore_from and args.accept_current_data:
parser.error("--restore-from cannot be combined with a test command.") parser.error("--restore-from cannot be combined with --accept-current-data.")
if not args.restore_from and not args.command: if (args.restore_from or args.accept_current_data) and args.command:
parser.error("A test command is required unless --restore-from is provided.") parser.error("Restore/accept modes cannot be combined with a test command.")
if not args.restore_from and not args.accept_current_data and not args.command:
parser.error("A test command is required unless --restore-from or --accept-current-data is provided.")
return args return args
@@ -159,45 +218,50 @@ def run_command(command: list[str]) -> int:
return completed.returncode return completed.returncode
def main(argv: list[str] | None = None) -> int: def accept_current_data(backup_root: Path) -> int:
args = parse_args(argv or sys.argv[1:]) show_phase("Complete Test Cycle")
repo_root = Path(__file__).resolve().parent.parent backup_path = close_active_backup_cycle(backup_root)
data_dir = repo_root / "data" print(f"Current data accepted. Backup preserved at: {backup_path}")
backup_root = repo_root / ".test-backups" return 0
db_path = data_dir / "transcription.db"
if not data_dir.exists():
raise FileNotFoundError(f"Data directory not found: {data_dir}")
backup_root.mkdir(parents=True, exist_ok=True) def restore_saved_backup(restore_from: str, data_path: Path, backup_root: Path) -> int:
restore_path = Path(restore_from)
if not restore_path.is_absolute():
restore_path = backup_root / restore_path
if args.restore_from: show_phase("Restore Phase")
restore_path = Path(args.restore_from) if not wait_for_restore_preflight(data_path / "transcription.db"):
if not restore_path.is_absolute(): print(f"Restore cancelled. Backup preserved at: {restore_path}")
restore_path = backup_root / restore_path return 1
show_phase("Restore Phase") restore_backup(restore_path, data_path)
if not wait_for_restore_preflight(db_path): current_backup = active_backup_path(backup_root)
print(f"Restore cancelled. Backup preserved at: {restore_path}") if current_backup is not None:
return 1 if current_backup.resolve() == restore_path.resolve():
close_active_backup_cycle(backup_root, restore_path)
else:
print(f"Active test cycle remains unchanged: {current_backup}")
return 0
restore_backup(restore_path, data_dir)
return 0
def restore_requested(args: argparse.Namespace) -> bool:
if args.auto_restore:
return True
if args.skip_restore_prompt:
return False
answer = input("Tests passed. Restore data backup now? [y/N] ").strip().lower()
return answer in {"y", "yes"}
def run_protected_tests(args: argparse.Namespace, data_path: Path, backup_root: Path) -> int:
command = normalize_command(list(args.command)) command = normalize_command(list(args.command))
if not command: if not command:
raise ValueError("No test command provided.") raise ValueError("No test command provided.")
show_phase("Backup Phase") show_phase("Backup Phase")
if not test_file_unlocked(db_path): backup_path = create_or_reuse_backup(data_path, backup_root)
print(f"WARNING: Backup preflight warning: database appears to be in use: {db_path}") backup_name = backup_path.name
print("WARNING: Proceeding with backup, but hot backups can capture an in-flight state.")
timestamp = datetime.now().strftime("%Y%m%d-%H%M%S")
backup_name = f"data-backup-{timestamp}"
backup_path = backup_root / backup_name
shutil.copytree(data_dir, backup_path)
print(f"Created backup: {backup_path}")
test_exit_code = run_command(command) test_exit_code = run_command(command)
if test_exit_code != 0: if test_exit_code != 0:
@@ -208,36 +272,30 @@ def main(argv: list[str] | None = None) -> int:
print(f" uv run python tools/run_destructive_tests.py --restore-from {backup_name}") print(f" uv run python tools/run_destructive_tests.py --restore-from {backup_name}")
return test_exit_code return test_exit_code
should_restore = False if not restore_requested(args):
if args.auto_restore: print(f"Restore skipped by user. Test cycle remains active with backup: {backup_path}")
should_restore = True print("Further test runs will reuse this backup instead of creating another.")
elif args.skip_restore_prompt:
should_restore = False
else:
answer = input("Tests passed. Restore data backup now? [y/N] ").strip().lower()
if answer in {"y", "yes"}:
should_restore = True
if not should_restore:
print(f"Restore skipped by user. Backup kept at: {backup_path}")
print("Restore later with:") print("Restore later with:")
print(f" uv run python tools/run_destructive_tests.py --restore-from {backup_name}") print(f" uv run python tools/run_destructive_tests.py --restore-from {backup_name}")
print("Or accept the current data and close the cycle with:")
print(" uv run python tools/run_destructive_tests.py --accept-current-data")
return 0 return 0
show_phase("Restore Phase") show_phase("Restore Phase")
if not wait_for_restore_preflight(db_path): if not wait_for_restore_preflight(data_path / "transcription.db"):
print(f"Restore cancelled. Backup preserved at: {backup_path}") print(f"Restore cancelled. Backup preserved at: {backup_path}")
return 1 return 1
try: try:
restore_backup(backup_path, data_dir) restore_backup(backup_path, data_path)
close_active_backup_cycle(backup_root, backup_path)
if args.keep_backup: if args.keep_backup:
print(f"Kept backup: {backup_path}") print(f"Kept backup: {backup_path}")
else: else:
shutil.rmtree(backup_path) shutil.rmtree(backup_path)
print(f"Deleted backup: {backup_path}") print(f"Deleted backup: {backup_path}")
except Exception as exc: except (OSError, RuntimeError) as exc:
print("WARNING: Restore failed. Your current data remains unchanged.") print("WARNING: Restore failed. The backup remains available.")
print(f"WARNING: {exc}") print(f"WARNING: {exc}")
print("Likely cause: another process has data/transcription.db open.") print("Likely cause: another process has data/transcription.db open.")
print("Stop the process and retry restore with:") print("Stop the process and retry restore with:")
@@ -247,5 +305,23 @@ def main(argv: list[str] | None = None) -> int:
return 0 return 0
def main(argv: list[str] | None = None) -> int:
args = parse_args(argv or sys.argv[1:])
repo_root = Path(__file__).resolve().parent.parent
data_path = repo_root / "data"
backup_root = repo_root / ".test-backups"
if not data_path.exists():
raise FileNotFoundError(f"Data directory not found: {data_path}")
backup_root.mkdir(parents=True, exist_ok=True)
if args.accept_current_data:
return accept_current_data(backup_root)
if args.restore_from:
return restore_saved_backup(args.restore_from, data_path, backup_root)
return run_protected_tests(args, data_path, backup_root)
if __name__ == "__main__": if __name__ == "__main__":
raise SystemExit(main()) raise SystemExit(main())