V4.6 Phase 7: drive ty check to zero and add a blocking quality gate [HIGH-06]

Baseline was 207 diagnostics. Two real bugs were hiding in the noise: - tools/run_destructive_tests.py imported ctypes.wintypes at module scope, which raises on non-Windows, and called fcntl unconditionally. The Windows and POSIX implementations now live under a module-level sys.platform split. - tests/ui/test_sources_page.py constructed Source(...) without document_id. Structural fixes, not suppressions: - New src/transcription/db/loading.py owns the SQLModel-field to QueryableAttribute reinterpretation via orm_attribute()/selectinload()/ defer(). This removed 42 "# pyright: ignore[reportArgumentType]" comments across documents/jobs/people/sources. Its docstring records that selectinload(A.b, B.c) is NOT equivalent to the chained form: varargs applies the selectin strategy only to the last path element, which under lazy="raise" raises InvalidRequestError at render time. - db/session.py transaction_scope no longer accepts or yields AsyncSessionTransaction. No caller ever passed one, sessionmaker.begin() yields an AsyncSession, and the dead branch was latently buggy because services call .exec(). Cleared 7 workflows.py diagnostics. - services/registry.py RegistryService is bound by a new RegistryEntry Protocol instead of bare SQLModel, so the shared implementation can read id/label/normalized_label/is_active. Cleared 9 diagnostics. - Column expressions in sources.py/jobs.py/test_store.py wrap in sqlmodel col(), the idiom already used in registry.py. - read_source_navigation wraps its literal tuple bounds in literal(). - normalization.py narrows with isinstance(image, TiffImageFile) rather than comparing image.format, since tag_v2 is TIFF-only. - linked_people.render uses @ui.refreshable_method, the NiceGUI API for bound methods. - The OpenRouter capturing client re-raises ResponseNotRead when the response stream is not async rather than mis-wrapping it. Tooling gate: - New .pre-commit-config.yaml runs ruff check and ty check as blocking hooks. No pre-commit config previously existed. Negative-tested: injecting a type error fails both hooks. - The last two "# pyright: ignore" comments (config.py) are removed; ty does not honor pyright directives. One "# ty: ignore" remains, in tests/test_prompts.py, where the test deliberately assigns to a frozen field to assert ValidationError. - asyncio_default_fixture_loop_scope is pinned to "function" so pytest-asyncio behavior does not shift on upgrade. Verification: ruff check clean, ty check reports 0 diagnostics, 292 passed and 4 skipped, pre-commit passes and demonstrably fails on a regression, and tools/run_destructive_tests.py runs on Windows. Co-authored-by: Copilot App <[email protected]>
2026-08-17 19:57:23 -05:00
parent 597be2691c
commit 66e2dce465
28 changed files with 316 additions and 184 deletions
@@ -2,8 +2,10 @@

 import asyncio
 from types import SimpleNamespace
+from typing import cast

 import pytest
+from openrouter import OpenRouter

 from transcription.config import Settings
 from transcription.providers.base import ProviderError
@@ -13,7 +15,7 @@ from transcription.providers.openrouter import OpenRouterTranscriptionProvider


 class _FakeChat:
-    def __init__(self, response=None, error: Exception | None = None):
+    def __init__(self, response=None, error: BaseException | None = None):
        self._response = response
        self._error = error
        self.calls = []
@@ -26,10 +28,15 @@ class _FakeChat:


 class _FakeClient:
-    def __init__(self, response=None, error: Exception | None = None):
+    def __init__(self, response=None, error: BaseException | None = None):
        self.chat = _FakeChat(response=response, error=error)


+def _fake_client(response=None, error: BaseException | None = None) -> OpenRouter:
+    """Return a stub typed as the SDK client the provider declares."""
+    return cast("OpenRouter", _FakeClient(response=response, error=error))
+
+
@pytest.mark.unit
 class TestOpenRouterProviderInit:
    """Verify OpenRouter provider initialization behavior."""
@@ -37,13 +44,13 @@ class TestOpenRouterProviderInit:
    def test_model_falls_back_to_default_when_unset(self):
        """Provider uses adapter default model when provider_model is None."""
        settings = Settings(openrouter_api_key="test-key", provider_model=None)
-        provider = OpenRouterTranscriptionProvider(settings=settings, client=_FakeClient())
+        provider = OpenRouterTranscriptionProvider(settings=settings, client=_fake_client())
        assert provider.model == DEFAULT_OPENROUTER_MODEL

    def test_model_uses_configured_value(self):
        """Provider uses configured provider_model when present."""
        settings = Settings(openrouter_api_key="test-key", provider_model="vendor/custom-model")
-        provider = OpenRouterTranscriptionProvider(settings=settings, client=_FakeClient())
+        provider = OpenRouterTranscriptionProvider(settings=settings, client=_fake_client())
        assert provider.model == "vendor/custom-model"


@@ -61,7 +68,7 @@ class TestOpenRouterProviderTranscribe:
            openrouter_http_referer="https://example.test",
            openrouter_app_title="Transcription App",
        )
-        provider = OpenRouterTranscriptionProvider(settings=settings, client=client)
+        provider = OpenRouterTranscriptionProvider(settings=settings, client=cast("OpenRouter", client))

        result = await provider.transcribe(
            prompt_text="Prompt body",
@@ -79,7 +86,10 @@ class TestOpenRouterProviderTranscribe:
        """Transcribe passes configured sampling parameters through to OpenRouter."""
        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)
+        provider = OpenRouterTranscriptionProvider(
+            settings=Settings(openrouter_api_key="test-key"),
+            client=cast("OpenRouter", client),
+        )

        result = await provider.transcribe(
            prompt_text="Prompt body",
@@ -110,7 +120,7 @@ class TestOpenRouterProviderTranscribe:
        }
        provider = OpenRouterTranscriptionProvider(
            settings=Settings(openrouter_api_key="test-key"),
-            client=_FakeClient(response=response),
+            client=_fake_client(response=response),
        )

        result = await provider.transcribe(
@@ -134,7 +144,7 @@ class TestOpenRouterProviderTranscribe:
        """Transcribe converts SDK failures to ProviderError."""
        provider = OpenRouterTranscriptionProvider(
            settings=Settings(openrouter_api_key="test-key"),
-            client=_FakeClient(error=RuntimeError("network down")),
+            client=_fake_client(error=RuntimeError("network down")),
        )

        with pytest.raises(ProviderError):
@@ -149,7 +159,7 @@ class TestOpenRouterProviderTranscribe:
        """Caller and shutdown cancellation must not be relabeled as a timeout."""
        provider = OpenRouterTranscriptionProvider(
            settings=Settings(openrouter_api_key="test-key"),
-            client=_FakeClient(error=asyncio.CancelledError()),
+            client=_fake_client(error=asyncio.CancelledError()),
        )

        with pytest.raises(asyncio.CancelledError):
@@ -166,7 +176,7 @@ class TestOpenRouterProviderTranscribe:
        client = _FakeClient(response=response)
        provider = OpenRouterTranscriptionProvider(
            settings=Settings(openrouter_api_key="test-key"),
-            client=client,
+            client=cast("OpenRouter", client),
        )

        await provider.transcribe(
@@ -185,7 +195,7 @@ class TestOpenRouterProviderTranscribe:
        """Transcribe raises ProviderResponseError for missing completion text."""
        provider = OpenRouterTranscriptionProvider(
            settings=Settings(openrouter_api_key="test-key"),
-            client=_FakeClient(response=SimpleNamespace(choices=[])),
+            client=_fake_client(response=SimpleNamespace(choices=[])),
        )

        with pytest.raises(ProviderResponseError):
@@ -204,7 +214,7 @@ class TestOpenRouterProviderTranscribe:
        }
        provider = OpenRouterTranscriptionProvider(
            settings=Settings(openrouter_api_key="test-key"),
-            client=_FakeClient(response=response),
+            client=_fake_client(response=response),
        )

        result = await provider.transcribe(
@@ -14,6 +14,7 @@ from transcription.db.models import JobSource
 from transcription.db.models import Source
 from transcription.providers import RequestManifest
 from transcription.providers import TranscriptionResult
+from transcription.providers.evidence import SourceEvidenceReference
 from transcription.providers.evidence import build_software_context
 from transcription.services import ServiceBundle
 from transcription.services.documents import DocumentService
@@ -62,7 +63,9 @@ def test_orientation_three_is_physically_rotated_and_metadata_removed(tmp_path):
    with Image.open(path) as source_image, Image.open(io.BytesIO(result.content)) as derivative:
        assert source_image.getexif()[274] == 3
        assert derivative.getexif().get(274, 1) == 1
-        assert derivative.getpixel((0, 0))[2] > derivative.getpixel((0, 0))[0]
+        pixel = derivative.getpixel((0, 0))
+        assert isinstance(pixel, tuple)
+        assert pixel[2] > pixel[0]


@pytest.mark.unit
@@ -130,7 +133,9 @@ async def test_resolve_provider_input_persists_exact_derivative(default_session_
    assert provider_input.derivative_id == artifacts[0].id
    assert provider_input.path.read_bytes() != original
    assert hashlib.sha256(provider_input.path.read_bytes()).hexdigest() == provider_input.digest_sha256
-    assert artifacts[0].coordinate_metadata["original_orientation"] == 3
+    coordinate_metadata = artifacts[0].coordinate_metadata
+    assert coordinate_metadata is not None
+    assert coordinate_metadata["original_orientation"] == 3


@pytest.mark.integration
@@ -219,8 +224,11 @@ async def test_worker_sends_exact_derivative_and_links_attempt_evidence(
    attempts = await services.sources.list_execution_attempts(source_id=source.id)
    artifacts = await services.sources.list_processing_artifacts(source_id=source.id)
    source_reference = captured["source_reference"]
+    assert isinstance(source_reference, SourceEvidenceReference)
+    captured_bytes = captured["bytes"]
+    assert isinstance(captured_bytes, bytes)
    assert source_path.read_bytes() == original
-    assert hashlib.sha256(captured["bytes"]).hexdigest() == source_reference.digest_sha256
+    assert hashlib.sha256(captured_bytes).hexdigest() == source_reference.digest_sha256
    assert source_reference.derivative_id is not None
    assert {artifact.artifact_type for artifact in artifacts} == {
        "orientation_normalized_model_input",
@@ -2,6 +2,7 @@ from pathlib import Path
 from uuid import uuid4

 import pytest
+from sqlmodel import col
 from sqlmodel import select

 from transcription.config import Settings
@@ -63,7 +64,7 @@ async def test_create_job_for_document_sorts_sources_and_creates_links(async_ses

    sources = (
        await async_session.exec(
-            select(Source).where(Source.document_id == document.id).order_by(Source.page_number)  # pyright: ignore[reportArgumentType]
+            select(Source).where(Source.document_id == document.id).order_by(col(Source.page_number))
        )
    ).all()
    assert [source.upload_name for source in sources] == ["A_page.pdf", "b_page.pdf"]
@@ -99,7 +100,7 @@ async def test_create_document_job_stores_source_under_document_id_directory(asy

    source = (
        await async_session.exec(
-            select(Source).where(Source.document_id == result.document_id).order_by(Source.page_number)  # pyright: ignore[reportArgumentType]
+            select(Source).where(Source.document_id == result.document_id).order_by(col(Source.page_number))
        )
    ).first()
    assert source is not None
@@ -1,6 +1,7 @@
 from uuid import uuid4

 import pytest
+from pydantic import JsonValue

 from transcription.db.models import Document
 from transcription.db.models import DocumentPerson
@@ -303,8 +304,11 @@ async def test_update_job_source_transcription_persists_provider_json_payloads(d
        JobSource(job_id=job.id, source_id=source.id, status=JobSourceStatus.PENDING)
    )

-    metadata = {"finish_reason": "stop", "usage": {"input_tokens": 11, "output_tokens": 22, "total_tokens": 33}}
-    raw_payload = {"id": "resp_xyz", "choices": [{"message": {"content": "provider transcript"}}]}
+    metadata: dict[str, JsonValue] = {
+        "finish_reason": "stop",
+        "usage": {"input_tokens": 11, "output_tokens": 22, "total_tokens": 33},
+    }
+    raw_payload: dict[str, JsonValue] = {"id": "resp_xyz", "choices": [{"message": {"content": "provider transcript"}}]}

    await transcriptions.update_job_source_transcription(
        job_id=job.id,
@@ -1,6 +1,7 @@
 """Tests for transcription.config — settings loading, provider defaults, and paths."""

 from pathlib import Path
+from typing import Any

 import pytest
 from pydantic import ValidationError
@@ -10,9 +11,9 @@ from transcription.config import Settings
 from transcription.config import parse_cli_settings


-def _make_settings(**overrides) -> Settings:
+def _make_settings(**overrides: Any) -> Settings:
    """Build a Settings instance with a dummy API key unless overridden."""
-    defaults = {"openrouter_api_key": "test-key-abc123", "provider_models": None}
+    defaults: dict[str, Any] = {"openrouter_api_key": "test-key-abc123", "provider_models": None}
    defaults.update(overrides)
    return Settings(**defaults)

@@ -168,6 +169,7 @@ def test_openrouter_client_timeout_tracks_the_configured_budget():

    settings = Settings(openrouter_api_key="test-key", worker_provider_timeout_seconds=123.0)
    provider = OpenRouterTranscriptionProvider(settings=settings)
+    assert provider._capturing_client is not None
    timeout = provider._capturing_client._client.timeout

    assert timeout.read == 123.0
@@ -6,6 +6,7 @@ import pytest
 import sqlalchemy as sa
 from sqlalchemy import inspect
 from sqlalchemy.dialects import postgresql
+from sqlalchemy.exc import SAWarning
 from sqlmodel import SQLModel
 from sqlmodel import select
 from sqlmodel.ext.asyncio.session import AsyncSession
@@ -148,7 +149,7 @@ async def test_create_all_declares_hot_path_indexes(tmp_path):
 def test_metadata_has_no_unresolvable_table_cycle():
    """create_all must be able to order every table, including on PostgreSQL."""
    with warnings.catch_warnings():
-        warnings.simplefilter("error", sa.exc.SAWarning)
+        warnings.simplefilter("error", SAWarning)
        ordered = [table.name for table in SQLModel.metadata.sorted_tables]

    assert ordered.index("source") < ordered.index("execution_attempt")
@@ -1,5 +1,6 @@
 """Tests for the V2 SQLModel persistence layer and relationships."""

+from typing import Any
 from uuid import UUID

 import pytest
@@ -17,8 +18,8 @@ from transcription.db.models import PersonRole
 from transcription.db.models import Source


-def _make_document(**overrides) -> Document:
-    defaults = {
+def _make_document(**overrides: Any) -> Document:
+    defaults: dict[str, Any] = {
        "name": "letter bundle",
        "notes": "Family correspondence",
    }
@@ -51,8 +52,8 @@ def _persist_document(session) -> Document:
    return document


-def _persist_person(session, **overrides) -> Person:
-    defaults = {"full_name": "Ada Lovelace"}
+def _persist_person(session, **overrides: Any) -> Person:
+    defaults: dict[str, Any] = {"full_name": "Ada Lovelace"}
    defaults.update(overrides)
    person = Person(**defaults)
    session.add(person)
@@ -69,8 +70,8 @@ def _persist_job(session, document: Document) -> Job:
    return job


-def _persist_source(session, document: Document, *, page_number: int = 1, **overrides) -> Source:
-    defaults = {
+def _persist_source(session, document: Document, *, page_number: int = 1, **overrides: Any) -> Source:
+    defaults: dict[str, Any] = {
        "document_id": document.id,
        "page_number": page_number,
        "upload_name": "letter.jpg",
@@ -88,8 +89,8 @@ def _persist_source(session, document: Document, *, page_number: int = 1, **over
    return source


-def _persist_job_source(session, job: Job, source: Source, **overrides) -> JobSource:
-    defaults = {
+def _persist_job_source(session, job: Job, source: Source, **overrides: Any) -> JobSource:
+    defaults: dict[str, Any] = {
        "job_id": job.id,
        "source_id": source.id,
        "status": JobSourceStatus.PENDING,
@@ -86,7 +86,7 @@ class TestPromptConfiguration:
        assert execution.temperature == 0.2
        assert execution.top_p == 0.9
        with pytest.raises(ValidationError):
-            execution.prompt_name = "changed.md"
+            execution.prompt_name = "changed.md"  # ty: ignore[invalid-assignment]

    def test_rejects_prompt_path_traversal_even_with_direct_loader_call(self, tmp_path):
        outside_prompt = tmp_path / "outside.md"
@@ -10,6 +10,7 @@ from uuid import uuid4

 import httpx
 import pytest
+from pydantic import JsonValue

 from transcription.benchmarking import EditorialAssessment
 from transcription.benchmarking import score_transcription
@@ -33,6 +34,18 @@ from transcription.services.sources import TranscriptionError
 from transcription.services.sources import transcribe_document_image


+def _json_object(value: JsonValue) -> dict[str, JsonValue]:
+    """Narrow a JSON export member to an object, asserting the export shape."""
+    assert isinstance(value, dict)
+    return value
+
+
+def _json_array(value: JsonValue) -> list[JsonValue]:
+    """Narrow a JSON export member to an array, asserting the export shape."""
+    assert isinstance(value, list)
+    return value
+
+
 class _ChunkedAsyncStream(httpx.AsyncByteStream):
    def __init__(self, chunks: list[bytes]):
        self._chunks = chunks
@@ -261,9 +274,9 @@ async def test_attempts_are_append_only_and_exported_with_integrity(default_sess
        )
    )
    export = await sources.build_evidence_export(source_id=source.id)
-    assert export["source"]["digest_sha256"] == "a" * 64
-    assert [item["attempt_number"] for item in export["attempts"]] == [1, 2]
-    assert export["artifacts"][0]["id"] == str(artifact.id)
+    assert _json_object(export["source"])["digest_sha256"] == "a" * 64
+    assert [_json_object(item)["attempt_number"] for item in _json_array(export["attempts"])] == [1, 2]
+    assert _json_object(_json_array(export["artifacts"])[0])["id"] == str(artifact.id)
    assert "file_path" not in json.dumps(export)
    with pytest.raises(JobDeleteBlockedError):
        await jobs.delete_job_with_guardrails(job_id=job.id)
@@ -1,9 +1,11 @@
 import asyncio
 import logging
+from typing import cast

 import pytest

 from transcription.services import ServiceBundle
+from transcription.services.sources import SourceService
 from transcription.worker import process_next_queued_job
 from transcription.worker import run_worker_loop

@@ -43,7 +45,7 @@ async def test_run_worker_loop_reuses_one_bundle_across_jobs(monkeypatch):
            nonlocal closed
            closed = True

-    bundle = ServiceBundle(sources=_Sources())  # type: ignore[arg-type]
+    bundle = ServiceBundle(sources=cast("SourceService", _Sources()))
    monkeypatch.setattr(
        "transcription.worker.ServiceBundle.from_session_factory",
        classmethod(lambda _cls, _factory=None, **_kwargs: bundle),
@@ -75,7 +77,7 @@ async def test_process_next_closes_provider_for_the_bundle_it_owns(monkeypatch):
            nonlocal closed
            closed = True

-    bundle = ServiceBundle(sources=_Sources())  # type: ignore[arg-type]
+    bundle = ServiceBundle(sources=cast("SourceService", _Sources()))
    monkeypatch.setattr(
        "transcription.worker.ServiceBundle.from_session_factory",
        classmethod(lambda _cls, _factory=None, **_kwargs: bundle),
@@ -101,7 +103,7 @@ async def test_process_next_leaves_a_caller_owned_bundle_open(monkeypatch):
            nonlocal closed
            closed = True

-    bundle = ServiceBundle(sources=_Sources())  # type: ignore[arg-type]
+    bundle = ServiceBundle(sources=cast("SourceService", _Sources()))

    async def _no_job(*, services, session):
        _ = (services, session)
@@ -2,9 +2,9 @@

 from __future__ import annotations

-from collections.abc import AsyncGenerator
 from collections.abc import Awaitable
 from collections.abc import Callable
+from collections.abc import Generator
 from datetime import UTC
 from datetime import datetime
 from pathlib import Path
@@ -32,12 +32,13 @@ from transcription.db.models import Source


@pytest.fixture(scope="session")
-def app_client(tmp_path_factory: pytest.TempPathFactory) -> AsyncGenerator[tuple[FastAPI, TestClient]]:
+def app_client(tmp_path_factory: pytest.TempPathFactory) -> Generator[tuple[FastAPI, TestClient]]:
    """Provide a real application and test client backed by in-memory SQLite."""
    tmp_path = tmp_path_factory.mktemp("ui")
+    database = SqliteSettings(path=str(tmp_path / "ui-tests.db"))
    settings = Settings(
        openrouter_api_key="test-key",
-        database=SqliteSettings(path=str(tmp_path / "ui-tests.db")),
+        database=database,
        environment="test",
        bootstrap_schema_on_startup=True,
        upload_dir=tmp_path / "uploads",
@@ -48,7 +49,7 @@ def app_client(tmp_path_factory: pytest.TempPathFactory) -> AsyncGenerator[tuple

    with TestClient(app) as client:
        runtime_path = Path(str(app.state.runtime.engine.url.database)).resolve()
-        expected_path = Path(settings.database.path).resolve()
+        expected_path = Path(database.path).resolve()
        if runtime_path != expected_path:
            raise RuntimeError(
                "Refusing to initialize destructive UI fixtures against "
@@ -1,12 +1,13 @@
 """Tests for the sources page routes and Source model properties."""

 from pathlib import Path
+from uuid import uuid4

 import pytest
-from sqlalchemy.orm import selectinload
 from sqlmodel import select

 from transcription.db import session_scope
+from transcription.db.loading import selectinload
 from transcription.db.models import Document
 from transcription.db.models import Job
 from transcription.db.models import JobSourceStatus
@@ -24,6 +25,7 @@ class TestSourceModelProperties:
    @pytest.mark.asyncio
    async def test_source_properties_with_no_job_sources(self):
        source = Source(
+            document_id=uuid4(),
            page_number=1,
            upload_name="page_one.png",
            filename="stored_page_one.png",