generated from john/python-template
V4.6 Phase 6: async I/O and configuration hygiene
MED-01 - move remaining blocking work off the event loop: - normalization.py gains normalize_orientation_async; the Pillow decode, transpose, and re-encode now run via asyncio.to_thread. The sync entry point stays for tests and documents that it blocks. - OrientationNormalization.digest_sha256 becomes a stored field computed inside normalize_orientation, which already runs off-loop, instead of a property that hashed page-sized derivative bytes on the caller's thread. - SourceService._write_and_digest_artifact performs the artifact write and its sha256 in a single worker-thread hop; both external-artifact write sites are now dispatched through to_thread. - transcribe_image dispatches load_source_payload and build_prompt_execution through to_thread. MED-04 - replace functools.cache on the engine and session factories with explicit URL-keyed registries. dispose_engine and dispose_session_factory now evict only the requested URL; previously cache_clear() tore down every other database in the process, and dispose_engine would construct an engine for an unknown URL purely to throw it away. New tests/test_engine_registry.py covers distinct engines per URL, targeted eviction, and the unknown-URL no-op. config.py - replace object.__setattr__ in normalize_provider_models with a model_validator(mode="before") over the raw input, so the derived selector is produced by normal construction rather than by mutating a frozen instance. model_copy(update=...) was tried first and rejected: pydantic-settings does not support a top-level validator returning anything other than self when validating via __init__. provider_model is now stripped as well as the tuple entries. models.py - add onupdate to the five updated_at columns and to Job.date_updated, and drop the 10 manual "updated_at = datetime.now(UTC)" assignments across the document, job, people, registry, and source services. Verified DDL-neutral by hashing CreateTable output for every table on both the sqlite and postgresql dialects before and after: identical, so this stays in Phase 6 and Phase 2 does not need re-verification. New tests/services/test_timestamps.py asserts an update through each service advances the timestamp. MED-08 - Job.filename no longer swallows every exception to None. Relationships declare lazy="raise", so the new _loaded_attribute helper inspects load state explicitly and returns None only for genuinely unloaded attributes; real errors now surface. Job.error_detail uses the same helper, which also removes its unguarded read of the lazy="raise" job_sources relationship. Verification: ruff check src tests clean; 288 passed, 4 skipped. Co-authored-by: Copilot App <[email protected]>
This commit is contained in:
+32
-10
@@ -122,21 +122,43 @@ class Settings(BaseSettings):
|
||||
raise ValueError("PROVIDER_MODELS must contain at least one model")
|
||||
return value
|
||||
|
||||
@model_validator(mode="after")
|
||||
def normalize_provider_models(self) -> "Settings":
|
||||
"""Build the immutable model selector with the configured default first."""
|
||||
configured = self.provider_models
|
||||
@model_validator(mode="before")
|
||||
@classmethod
|
||||
def normalize_provider_models(cls, data: object) -> object:
|
||||
"""Build the immutable model selector with the configured default first.
|
||||
|
||||
This runs before field validation so the derived value is produced by
|
||||
normal construction rather than by mutating a frozen instance.
|
||||
"""
|
||||
if not isinstance(data, dict):
|
||||
return data
|
||||
|
||||
default_model = data.get("provider_model") or DEFAULT_PROVIDER_MODEL
|
||||
if not isinstance(default_model, str):
|
||||
return data
|
||||
default_model = default_model.strip()
|
||||
|
||||
configured = data.get("provider_models")
|
||||
if configured is None:
|
||||
configured = ()
|
||||
elif isinstance(configured, str):
|
||||
# Left as-is so the field validator can report the malformed value.
|
||||
return {**data, "provider_model": default_model}
|
||||
elif not isinstance(configured, (list, tuple)):
|
||||
return {**data, "provider_model": default_model}
|
||||
elif not configured:
|
||||
# Preserved so validate_provider_models_input can reject it.
|
||||
return {**data, "provider_model": default_model}
|
||||
|
||||
default_model = self.provider_model or DEFAULT_PROVIDER_MODEL
|
||||
object.__setattr__(self, "provider_model", default_model)
|
||||
ordered = (default_model, *configured)
|
||||
deduplicated: list[str] = []
|
||||
for model in ordered:
|
||||
for model in (default_model, *configured):
|
||||
if not isinstance(model, str):
|
||||
return {**data, "provider_model": default_model}
|
||||
normalized = model.strip()
|
||||
if normalized not in deduplicated:
|
||||
deduplicated.append(normalized)
|
||||
object.__setattr__(self, "provider_models", tuple(deduplicated))
|
||||
return self
|
||||
|
||||
return {**data, "provider_model": default_model, "provider_models": tuple(deduplicated)}
|
||||
|
||||
@property
|
||||
def should_bootstrap_schema(self) -> bool:
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
from functools import cache
|
||||
from typing import Any
|
||||
|
||||
from sqlalchemy import URL
|
||||
@@ -39,8 +38,10 @@ def resolve_engine(settings: Settings | None = None) -> AsyncEngine:
|
||||
)
|
||||
|
||||
|
||||
@cache
|
||||
def get_engine(database_url: str, *, sqlite_check_same_thread: bool = False) -> AsyncEngine:
|
||||
_ENGINES: dict[str, AsyncEngine] = {}
|
||||
|
||||
|
||||
def _create_engine(database_url: str, *, sqlite_check_same_thread: bool) -> AsyncEngine:
|
||||
kwargs: dict[str, Any] = {"echo": False, "pool_pre_ping": True}
|
||||
if database_url.startswith("sqlite"):
|
||||
kwargs["connect_args"] = {"check_same_thread": sqlite_check_same_thread}
|
||||
@@ -50,12 +51,34 @@ def get_engine(database_url: str, *, sqlite_check_same_thread: bool = False) ->
|
||||
return create_async_engine(database_url, **kwargs)
|
||||
|
||||
|
||||
def get_engine(database_url: str, *, sqlite_check_same_thread: bool = False) -> AsyncEngine:
|
||||
"""Return the process-wide engine for ``database_url``, creating it on first use.
|
||||
|
||||
Engines are registered per URL so that disposing one leaves every other
|
||||
database untouched.
|
||||
"""
|
||||
engine = _ENGINES.get(database_url)
|
||||
if engine is None:
|
||||
engine = _create_engine(database_url, sqlite_check_same_thread=sqlite_check_same_thread)
|
||||
_ENGINES[database_url] = engine
|
||||
return engine
|
||||
|
||||
|
||||
async def dispose_engine(database_url: str) -> None:
|
||||
engine = get_engine(database_url)
|
||||
try:
|
||||
"""Dispose and unregister the engine for ``database_url`` only.
|
||||
|
||||
Unknown URLs are a no-op rather than provoking the creation of an engine
|
||||
purely so that it can be thrown away.
|
||||
"""
|
||||
engine = _ENGINES.pop(database_url, None)
|
||||
if engine is not None:
|
||||
await engine.dispose()
|
||||
|
||||
|
||||
async def dispose_all_engines() -> None:
|
||||
while _ENGINES:
|
||||
_, engine = _ENGINES.popitem()
|
||||
await engine.dispose()
|
||||
finally:
|
||||
get_engine.cache_clear()
|
||||
|
||||
|
||||
async def refresh_engine(database_url: str) -> AsyncEngine:
|
||||
|
||||
@@ -4,6 +4,7 @@ from datetime import UTC
|
||||
from datetime import date
|
||||
from datetime import datetime
|
||||
from enum import StrEnum
|
||||
from typing import Any
|
||||
from typing import Optional
|
||||
from uuid import UUID
|
||||
from uuid import uuid4
|
||||
@@ -19,14 +20,32 @@ from sqlalchemy import Index
|
||||
from sqlalchemy import LargeBinary
|
||||
from sqlalchemy import UniqueConstraint
|
||||
from sqlalchemy import Uuid
|
||||
from sqlalchemy import inspect as sqlalchemy_inspect
|
||||
from sqlalchemy.dialects.postgresql import JSONB
|
||||
from sqlalchemy.orm.exc import DetachedInstanceError
|
||||
from sqlalchemy.exc import NoInspectionAvailable
|
||||
from sqlalchemy.types import TypeDecorator
|
||||
from sqlmodel import Field
|
||||
from sqlmodel import Relationship
|
||||
from sqlmodel import SQLModel
|
||||
|
||||
|
||||
def _loaded_attribute(instance: object, attribute: str) -> Any | None:
|
||||
"""Return ``attribute`` only when it is already loaded on ``instance``.
|
||||
|
||||
Relationships in this module declare ``lazy="raise"``, so reading an
|
||||
unloaded attribute is an error rather than a silent query. Callers that
|
||||
render optional detail use this to distinguish "not loaded" from "absent"
|
||||
without catching exceptions indiscriminately.
|
||||
"""
|
||||
try:
|
||||
state = sqlalchemy_inspect(instance)
|
||||
except NoInspectionAvailable:
|
||||
return None
|
||||
if attribute in state.unloaded:
|
||||
return None
|
||||
return state.dict.get(attribute)
|
||||
|
||||
|
||||
class JSONBCompat(TypeDecorator):
|
||||
"""JSONB for PostgreSQL and JSON for SQLite/testing backends."""
|
||||
|
||||
@@ -69,7 +88,10 @@ class DocumentType(SQLModel, table=True):
|
||||
normalized_label: str = Field(index=True, unique=True)
|
||||
is_active: bool = True
|
||||
created_at: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
updated_at: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
updated_at: datetime = Field(
|
||||
default_factory=lambda: datetime.now(UTC),
|
||||
sa_column_kwargs={"onupdate": lambda: datetime.now(UTC)},
|
||||
)
|
||||
|
||||
documents: list["Document"] = Relationship(
|
||||
back_populates="document_type_ref", sa_relationship_kwargs={"lazy": "raise"}
|
||||
@@ -87,7 +109,10 @@ class PersonRole(SQLModel, table=True):
|
||||
normalized_label: str = Field(index=True, unique=True)
|
||||
is_active: bool = True
|
||||
created_at: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
updated_at: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
updated_at: datetime = Field(
|
||||
default_factory=lambda: datetime.now(UTC),
|
||||
sa_column_kwargs={"onupdate": lambda: datetime.now(UTC)},
|
||||
)
|
||||
|
||||
document_people: list["DocumentPerson"] = Relationship(
|
||||
back_populates="role_ref", sa_relationship_kwargs={"lazy": "raise"}
|
||||
@@ -106,7 +131,10 @@ class Document(SQLModel, table=True):
|
||||
notes: str | None = None
|
||||
archive_identifier: str | None = None
|
||||
created_at: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
updated_at: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
updated_at: datetime = Field(
|
||||
default_factory=lambda: datetime.now(UTC),
|
||||
sa_column_kwargs={"onupdate": lambda: datetime.now(UTC)},
|
||||
)
|
||||
|
||||
jobs: list["Job"] = Relationship(back_populates="document", sa_relationship_kwargs={"lazy": "raise"})
|
||||
sources: list["Source"] = Relationship(back_populates="document", sa_relationship_kwargs={"lazy": "raise"})
|
||||
@@ -139,7 +167,10 @@ class Person(SQLModel, table=True):
|
||||
sa_column=Column("metadata", JSONBCompat(), nullable=True),
|
||||
)
|
||||
created_at: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
updated_at: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
updated_at: datetime = Field(
|
||||
default_factory=lambda: datetime.now(UTC),
|
||||
sa_column_kwargs={"onupdate": lambda: datetime.now(UTC)},
|
||||
)
|
||||
|
||||
document_people: list["DocumentPerson"] = Relationship(
|
||||
back_populates="person", sa_relationship_kwargs={"lazy": "raise"}
|
||||
@@ -156,7 +187,10 @@ class DocumentPerson(SQLModel, table=True):
|
||||
person_id: UUID = Field(foreign_key="person.id", index=True)
|
||||
role_id: UUID = Field(foreign_key="person_role.id", index=True)
|
||||
created_at: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
updated_at: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
updated_at: datetime = Field(
|
||||
default_factory=lambda: datetime.now(UTC),
|
||||
sa_column_kwargs={"onupdate": lambda: datetime.now(UTC)},
|
||||
)
|
||||
|
||||
__table_args__ = (UniqueConstraint("document_id", "person_id", name="uq_document_person"),)
|
||||
|
||||
@@ -203,7 +237,10 @@ class Job(SQLModel, table=True):
|
||||
),
|
||||
)
|
||||
date_created: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
date_updated: datetime = Field(default_factory=lambda: datetime.now(UTC))
|
||||
date_updated: datetime = Field(
|
||||
default_factory=lambda: datetime.now(UTC),
|
||||
sa_column_kwargs={"onupdate": lambda: datetime.now(UTC)},
|
||||
)
|
||||
provider: str | None = None
|
||||
model: str | None = None
|
||||
prompt_name: str | None = None
|
||||
@@ -218,20 +255,15 @@ class Job(SQLModel, table=True):
|
||||
|
||||
@property
|
||||
def filename(self) -> str:
|
||||
"""Return the filename of the associated source, when available."""
|
||||
if not self.job_sources:
|
||||
return "unknown"
|
||||
|
||||
for job_source in self.job_sources:
|
||||
source = job_source.__dict__.get("source")
|
||||
if source is None:
|
||||
try:
|
||||
source = job_source.source
|
||||
except DetachedInstanceError:
|
||||
source = None
|
||||
except Exception: # noqa: BLE001
|
||||
source = None
|
||||
"""Return the filename of the first loaded source, when available.
|
||||
|
||||
Relationships on this model use ``lazy="raise"``, so this deliberately
|
||||
inspects load state rather than triggering (or swallowing) a lazy load:
|
||||
a read model that did not eager-load its sources gets "unknown" instead
|
||||
of an unhandled error, and genuine errors are no longer hidden.
|
||||
"""
|
||||
for job_source in _loaded_attribute(self, "job_sources") or ():
|
||||
source = _loaded_attribute(job_source, "source")
|
||||
if source is not None:
|
||||
return source.filename
|
||||
|
||||
@@ -240,10 +272,7 @@ class Job(SQLModel, table=True):
|
||||
@property
|
||||
def error_detail(self) -> str | None:
|
||||
"""Return the first available source-level error detail for the job."""
|
||||
if not self.job_sources:
|
||||
return None
|
||||
|
||||
for job_source in self.job_sources:
|
||||
for job_source in _loaded_attribute(self, "job_sources") or ():
|
||||
if job_source.error_detail:
|
||||
return job_source.error_detail
|
||||
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
from collections.abc import AsyncGenerator
|
||||
from contextlib import asynccontextmanager
|
||||
from functools import cache
|
||||
from typing import Annotated
|
||||
|
||||
from fastapi import Depends
|
||||
@@ -17,13 +16,20 @@ from .engine import get_engine
|
||||
type SessionFactory = async_sessionmaker[AsyncSession]
|
||||
|
||||
|
||||
@cache
|
||||
_SESSION_FACTORIES: dict[str, SessionFactory] = {}
|
||||
|
||||
|
||||
def get_session_factory(database_url: str) -> SessionFactory:
|
||||
return async_sessionmaker(
|
||||
bind=get_engine(database_url),
|
||||
class_=AsyncSession,
|
||||
expire_on_commit=False,
|
||||
)
|
||||
"""Return the process-wide session factory for ``database_url``."""
|
||||
factory = _SESSION_FACTORIES.get(database_url)
|
||||
if factory is None:
|
||||
factory = async_sessionmaker(
|
||||
bind=get_engine(database_url),
|
||||
class_=AsyncSession,
|
||||
expire_on_commit=False,
|
||||
)
|
||||
_SESSION_FACTORIES[database_url] = factory
|
||||
return factory
|
||||
|
||||
|
||||
def resolve_session_factory(
|
||||
@@ -46,7 +52,8 @@ type SessionFactoryDep = Annotated[SessionFactory, Depends(resolve_session_facto
|
||||
|
||||
|
||||
async def dispose_session_factory(database_url: str) -> None:
|
||||
get_session_factory.cache_clear()
|
||||
"""Drop the session factory and engine for ``database_url`` only."""
|
||||
_SESSION_FACTORIES.pop(database_url, None)
|
||||
await dispose_engine(database_url)
|
||||
|
||||
|
||||
|
||||
@@ -2,7 +2,6 @@ import logging
|
||||
import shutil
|
||||
from collections.abc import Sequence
|
||||
from dataclasses import dataclass
|
||||
from datetime import UTC
|
||||
from datetime import date
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
@@ -207,7 +206,6 @@ class DocumentService(ServiceBase):
|
||||
"""Update an existing document in the database."""
|
||||
async with self._session_scope(session) as _session:
|
||||
await self._validate_document_type(session=_session, document=document)
|
||||
document.updated_at = datetime.now(UTC)
|
||||
merged = await _session.merge(document)
|
||||
await self._finalize(session=_session, caller_session=session, refresh=(merged,))
|
||||
return merged
|
||||
@@ -453,7 +451,6 @@ class DocumentService(ServiceBase):
|
||||
document = await self._read_document(session=_session, document_id=document_id)
|
||||
document.document_type_id = document_type_id
|
||||
await self._validate_document_type(session=_session, document=document)
|
||||
document.updated_at = datetime.now(UTC)
|
||||
await self._finalize(session=_session, caller_session=session, refresh=(document,))
|
||||
return document
|
||||
|
||||
|
||||
@@ -164,7 +164,6 @@ class JobService(ServiceBase):
|
||||
job.status = status
|
||||
if retry_count_increment:
|
||||
job.retry_count += retry_count_increment
|
||||
job.date_updated = datetime.now(UTC)
|
||||
await self._finalize(session=_session, caller_session=session, refresh=(job,))
|
||||
return job
|
||||
|
||||
@@ -196,7 +195,6 @@ class JobService(ServiceBase):
|
||||
return None
|
||||
|
||||
job.status = JobStatus.PROCESSING
|
||||
job.date_updated = datetime.now(UTC)
|
||||
await self._finalize(session=_session, caller_session=session, refresh=(job,))
|
||||
return job
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import hashlib
|
||||
import io
|
||||
from dataclasses import dataclass
|
||||
@@ -43,14 +44,16 @@ class OrientationNormalization:
|
||||
original_height: int
|
||||
derivative_width: int
|
||||
derivative_height: int
|
||||
|
||||
@property
|
||||
def digest_sha256(self) -> str:
|
||||
return hashlib.sha256(self.content).hexdigest()
|
||||
# Computed eagerly by `normalize_orientation`, which already runs off the
|
||||
# event loop, so callers never hash multi-megabyte derivatives inline.
|
||||
digest_sha256: str
|
||||
|
||||
|
||||
def normalize_orientation(path: str | Path, *, media_type: str) -> OrientationNormalization | None:
|
||||
"""Physically apply supported EXIF rotation, returning None for a safe no-op."""
|
||||
"""Physically apply supported EXIF rotation, returning None for a safe no-op.
|
||||
|
||||
Blocking. Async callers must use :func:`normalize_orientation_async`.
|
||||
"""
|
||||
source_path = Path(path)
|
||||
if media_type not in {"image/jpeg", "image/png", "image/tiff"}:
|
||||
return None
|
||||
@@ -88,8 +91,9 @@ def normalize_orientation(path: str | Path, *, media_type: str) -> OrientationNo
|
||||
) from exc
|
||||
|
||||
suffix = source_path.suffix.lower()
|
||||
content = output.getvalue()
|
||||
return OrientationNormalization(
|
||||
content=output.getvalue(),
|
||||
content=content,
|
||||
media_type=media_type,
|
||||
suffix=suffix,
|
||||
original_orientation=orientation,
|
||||
@@ -98,4 +102,15 @@ def normalize_orientation(path: str | Path, *, media_type: str) -> OrientationNo
|
||||
original_height=original_height,
|
||||
derivative_width=normalized.width,
|
||||
derivative_height=normalized.height,
|
||||
digest_sha256=hashlib.sha256(content).hexdigest(),
|
||||
)
|
||||
|
||||
|
||||
async def normalize_orientation_async(path: str | Path, *, media_type: str) -> OrientationNormalization | None:
|
||||
"""Run :func:`normalize_orientation` off the event loop.
|
||||
|
||||
Pillow decode, transpose, and re-encode are CPU- and disk-bound and scale
|
||||
with page size, so they must not run on the request or worker event loop
|
||||
([MED-01]).
|
||||
"""
|
||||
return await asyncio.to_thread(normalize_orientation, path, media_type=media_type)
|
||||
|
||||
@@ -6,8 +6,6 @@ import logging
|
||||
import re
|
||||
from collections.abc import Sequence
|
||||
from dataclasses import dataclass
|
||||
from datetime import UTC
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from uuid import UUID
|
||||
@@ -134,7 +132,6 @@ class PeopleService(ServiceBase):
|
||||
async def update_person(self, person: Person, *, session: AsyncSession | None = None) -> Person:
|
||||
async with self._session_scope(session) as _session:
|
||||
person.family_search_id = normalize_family_search_id(person.family_search_id)
|
||||
person.updated_at = datetime.now(UTC)
|
||||
merged = await _session.merge(person)
|
||||
try:
|
||||
await self._finalize(session=_session, caller_session=session, refresh=(merged,))
|
||||
@@ -194,7 +191,6 @@ class PeopleService(ServiceBase):
|
||||
role_id=document_person.role_id,
|
||||
require_active=existing.role_id != document_person.role_id,
|
||||
)
|
||||
document_person.updated_at = datetime.now(UTC)
|
||||
merged = await _session.merge(document_person)
|
||||
return await self._finalize_link(session=_session, caller_session=session, link=merged)
|
||||
|
||||
@@ -378,7 +374,6 @@ class PeopleService(ServiceBase):
|
||||
require_active=link.role_id != role_id,
|
||||
)
|
||||
link.role_id = role_id
|
||||
link.updated_at = datetime.now(UTC)
|
||||
return await self._finalize_link(session=_session, caller_session=session, link=link)
|
||||
|
||||
async def remove_document_person_link(
|
||||
@@ -450,7 +445,6 @@ class PeopleService(ServiceBase):
|
||||
_session.add(existing)
|
||||
elif existing.role_id != desired.role_id:
|
||||
existing.role_id = desired.role_id
|
||||
existing.updated_at = datetime.now(UTC)
|
||||
synchronized.append(existing)
|
||||
|
||||
try:
|
||||
|
||||
@@ -11,8 +11,6 @@ from __future__ import annotations
|
||||
|
||||
from abc import abstractmethod
|
||||
from collections.abc import Sequence
|
||||
from datetime import UTC
|
||||
from datetime import datetime
|
||||
from typing import Any
|
||||
from uuid import UUID
|
||||
|
||||
@@ -200,7 +198,6 @@ class RegistryService[ModelT: SQLModel](ServiceBase):
|
||||
entry.label = self.normalize_label(label)
|
||||
entry.normalized_label = self.label_key(label)
|
||||
entry.is_active = is_active
|
||||
entry.updated_at = datetime.now(UTC)
|
||||
try:
|
||||
await self._finalize(session=_session, caller_session=session, refresh=(entry,))
|
||||
except IntegrityError as exc:
|
||||
|
||||
@@ -60,7 +60,7 @@ from .normalization import ORIENTATION_PRODUCER
|
||||
from .normalization import ORIENTATION_PRODUCER_VERSION
|
||||
from .normalization import ORIENTATION_SCHEMA
|
||||
from .normalization import ORIENTATION_SCHEMA_VERSION
|
||||
from .normalization import normalize_orientation
|
||||
from .normalization import normalize_orientation_async
|
||||
from .source_media import lookup_source_mime_type
|
||||
from .source_media import supported_source_formats
|
||||
|
||||
@@ -562,7 +562,6 @@ class SourceService(ServiceBase):
|
||||
|
||||
job.provider = provider or job.provider or self.settings.provider.value
|
||||
job.model = model or job.model or _resolve_transcript_model(provider=self.provider, settings=self.settings)
|
||||
job.date_updated = datetime.now(UTC)
|
||||
metadata_payload = _validate_transcription_metadata(ai_metadata)
|
||||
raw_response_payload = _validate_json_object(raw_api_response, field_name="raw_api_response")
|
||||
|
||||
@@ -779,7 +778,7 @@ class SourceService(ServiceBase):
|
||||
if len(payload_bytes) > self.settings.artifact_inline_threshold_bytes:
|
||||
relative_path = Path(str(source_id)) / f"{artifact_id}.json"
|
||||
external_path = self.settings.artifact_dir / relative_path
|
||||
self._write_external_artifact(path=external_path, content=payload_bytes)
|
||||
await asyncio.to_thread(self._write_external_artifact, path=external_path, content=payload_bytes)
|
||||
inline_payload = None
|
||||
external_reference = relative_path.as_posix()
|
||||
artifact = ProcessingArtifact(
|
||||
@@ -825,7 +824,7 @@ class SourceService(ServiceBase):
|
||||
safe_suffix = suffix if suffix.startswith(".") and suffix[1:].isalnum() else ".bin"
|
||||
relative_path = Path(str(source_id)) / f"{artifact_id}{safe_suffix.lower()}"
|
||||
external_path = self.settings.artifact_dir / relative_path
|
||||
self._write_external_artifact(path=external_path, content=content)
|
||||
payload_sha256 = await asyncio.to_thread(self._write_and_digest_artifact, path=external_path, content=content)
|
||||
artifact = ProcessingArtifact(
|
||||
id=artifact_id,
|
||||
source_id=source_id,
|
||||
@@ -836,7 +835,7 @@ class SourceService(ServiceBase):
|
||||
producer=producer,
|
||||
producer_version=producer_version,
|
||||
external_reference=relative_path.as_posix(),
|
||||
payload_sha256=hashlib.sha256(content).hexdigest(),
|
||||
payload_sha256=payload_sha256,
|
||||
byte_size=len(content),
|
||||
coordinate_metadata=coordinate_metadata,
|
||||
)
|
||||
@@ -854,7 +853,7 @@ class SourceService(ServiceBase):
|
||||
) -> ProviderInput:
|
||||
"""Resolve original or physically orientation-normalized provider input."""
|
||||
media_type = source_mime_type(source.file_path)
|
||||
normalized = normalize_orientation(source.file_path, media_type=media_type)
|
||||
normalized = await normalize_orientation_async(source.file_path, media_type=media_type)
|
||||
if normalized is None:
|
||||
return ProviderInput(
|
||||
path=Path(source.file_path),
|
||||
@@ -909,6 +908,15 @@ class SourceService(ServiceBase):
|
||||
transformation=f"{ORIENTATION_SCHEMA}@{ORIENTATION_SCHEMA_VERSION}",
|
||||
)
|
||||
|
||||
def _write_and_digest_artifact(self, *, path: Path, content: bytes) -> str:
|
||||
"""Persist artifact bytes and return their digest in one off-loop hop.
|
||||
|
||||
Binary derivatives are page-sized, so hashing them belongs in the same
|
||||
worker thread as the write rather than on the event loop ([MED-01]).
|
||||
"""
|
||||
self._write_external_artifact(path=path, content=content)
|
||||
return hashlib.sha256(content).hexdigest()
|
||||
|
||||
def _write_external_artifact(self, *, path: Path, content: bytes) -> None:
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
temporary_path = path.with_suffix(f"{path.suffix}.tmp")
|
||||
@@ -1205,7 +1213,9 @@ async def transcribe_document_image(
|
||||
"""Transcribe a local image using the configured prompt and provider."""
|
||||
runtime_settings = settings or get_settings()
|
||||
if prompt_text is None:
|
||||
prompt_execution = build_prompt_execution(prompt_name=prompt_name, settings=runtime_settings)
|
||||
prompt_execution = await asyncio.to_thread(
|
||||
build_prompt_execution, prompt_name=prompt_name, settings=runtime_settings
|
||||
)
|
||||
else:
|
||||
effective_prompt_name = (prompt_name or runtime_settings.default_prompt_name or DEFAULT_PROMPT_FILE).strip()
|
||||
prompt_execution = PromptExecution(
|
||||
@@ -1216,7 +1226,7 @@ async def transcribe_document_image(
|
||||
temperature=temperature if temperature is not None else runtime_settings.transcription_temperature,
|
||||
top_p=top_p if top_p is not None else runtime_settings.transcription_top_p,
|
||||
)
|
||||
image_bytes, mime_type = load_source_payload(image_path)
|
||||
image_bytes, mime_type = await asyncio.to_thread(load_source_payload, image_path)
|
||||
|
||||
owns_adapter = provider is None
|
||||
adapter = provider or get_transcription_provider(settings=runtime_settings)
|
||||
@@ -1333,7 +1343,10 @@ def validate_source_content(*, filename: str | Path, content: bytes) -> str:
|
||||
|
||||
|
||||
def load_source_payload(source_path: str | Path) -> tuple[bytes, str]:
|
||||
"""Read Source bytes and resolve MIME type from the canonical format policy."""
|
||||
"""Read Source bytes and resolve MIME type from the canonical format policy.
|
||||
|
||||
Blocking. Async callers must dispatch this through ``asyncio.to_thread``.
|
||||
"""
|
||||
path = Path(source_path)
|
||||
|
||||
if not path.exists() or not path.is_file():
|
||||
|
||||
@@ -19,6 +19,7 @@ from transcription.services import ServiceBundle
|
||||
from transcription.services.documents import DocumentService
|
||||
from transcription.services.jobs import JobService
|
||||
from transcription.services.normalization import normalize_orientation
|
||||
from transcription.services.normalization import normalize_orientation_async
|
||||
from transcription.services.sources import SourceService
|
||||
from transcription.services.workflows import process_queued_job
|
||||
|
||||
@@ -226,3 +227,18 @@ async def test_worker_sends_exact_derivative_and_links_attempt_evidence(
|
||||
"transcription_quality_warnings",
|
||||
}
|
||||
assert {artifact.execution_attempt_id for artifact in artifacts} == {attempts[0].id}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_async_wrapper_matches_sync_result_and_precomputes_digest(tmp_path):
|
||||
"""[MED-01]: Pillow work runs off the event loop and hashes its own output."""
|
||||
path = tmp_path / "async-upside-down.jpg"
|
||||
_write_oriented_jpeg(path, orientation=3)
|
||||
|
||||
result = await normalize_orientation_async(path, media_type="image/jpeg")
|
||||
expected = normalize_orientation(path, media_type="image/jpeg")
|
||||
|
||||
assert result is not None
|
||||
assert expected is not None
|
||||
assert result.content == expected.content
|
||||
assert result.digest_sha256 == hashlib.sha256(result.content).hexdigest()
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
"""Phase 6 verification that modification timestamps advance automatically.
|
||||
|
||||
Covers the `onupdate` change: `updated_at` / `date_updated` are now maintained
|
||||
by the ORM column default rather than by hand at each call site, so update paths
|
||||
that previously forgot to set them no longer report a stale timestamp.
|
||||
"""
|
||||
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
|
||||
from transcription.db.models import Document
|
||||
from transcription.db.models import Job
|
||||
from transcription.db.models import JobStatus
|
||||
from transcription.db.models import Person
|
||||
from transcription.services.documents import DocumentService
|
||||
from transcription.services.jobs import JobService
|
||||
from transcription.services.people import PeopleService
|
||||
from transcription.services.people import PersonRoleRegistry
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_document_update_advances_updated_at(default_session_factory):
|
||||
documents = DocumentService(session_factory=default_session_factory)
|
||||
document = await documents.create_document(Document(id=uuid4(), name="timestamps"))
|
||||
original = document.updated_at
|
||||
|
||||
document.name = "timestamps renamed"
|
||||
updated = await documents.update_document(document)
|
||||
|
||||
assert updated.updated_at > original
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_person_update_advances_updated_at(default_session_factory):
|
||||
people = PeopleService(session_factory=default_session_factory)
|
||||
person = await people.create_person(Person(full_name="Grace Hopper"))
|
||||
original = person.updated_at
|
||||
|
||||
person.full_name = "Rear Adm. Grace Hopper"
|
||||
updated = await people.update_person(person)
|
||||
|
||||
assert updated.updated_at > original
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_registry_update_advances_updated_at(default_session_factory):
|
||||
roles = PersonRoleRegistry(session_factory=default_session_factory)
|
||||
role = await roles.create_entry(label="Witness")
|
||||
original = role.updated_at
|
||||
|
||||
updated = await roles.update_entry(role.id, label="Chief Witness", is_active=True)
|
||||
|
||||
assert updated.updated_at > original
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_job_status_update_advances_date_updated(default_session_factory):
|
||||
documents = DocumentService(session_factory=default_session_factory)
|
||||
jobs = JobService(session_factory=default_session_factory)
|
||||
|
||||
document = await documents.create_document(Document(id=uuid4(), name="job-timestamps"))
|
||||
job = await jobs.create_job(Job(document_id=document.id))
|
||||
original = job.date_updated
|
||||
|
||||
updated = await jobs.update_job_state(job_id=job.id, status=JobStatus.PROCESSING)
|
||||
|
||||
assert updated.date_updated > original
|
||||
@@ -0,0 +1,53 @@
|
||||
"""Phase 6 verification for the URL-keyed engine and session-factory registries.
|
||||
|
||||
Covers [MED-04]: replacing `functools.cache` with an explicit registry so that
|
||||
disposing one database's engine cannot silently tear down every other one.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from transcription.db.engine import dispose_engine
|
||||
from transcription.db.engine import get_engine
|
||||
from transcription.db.session import dispose_session_factory
|
||||
from transcription.db.session import get_session_factory
|
||||
|
||||
URL_A = "sqlite+aiosqlite:///./.registry-test-a.db"
|
||||
URL_B = "sqlite+aiosqlite:///./.registry-test-b.db"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_distinct_urls_produce_distinct_engines_and_eviction_is_targeted():
|
||||
engine_a = get_engine(URL_A)
|
||||
engine_b = get_engine(URL_B)
|
||||
|
||||
assert engine_a is not engine_b
|
||||
assert get_engine(URL_A) is engine_a
|
||||
|
||||
await dispose_engine(URL_A)
|
||||
|
||||
assert get_engine(URL_B) is engine_b, "disposing one URL must not evict the others"
|
||||
assert get_engine(URL_A) is not engine_a, "the disposed URL must be rebuilt on demand"
|
||||
|
||||
await dispose_engine(URL_A)
|
||||
await dispose_engine(URL_B)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disposing_an_unregistered_url_is_a_noop():
|
||||
await dispose_engine("sqlite+aiosqlite:///./.registry-test-never-created.db")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_factory_eviction_is_targeted():
|
||||
factory_a = get_session_factory(URL_A)
|
||||
factory_b = get_session_factory(URL_B)
|
||||
|
||||
assert factory_a is not factory_b
|
||||
|
||||
await dispose_session_factory(URL_A)
|
||||
|
||||
assert get_session_factory(URL_B) is factory_b
|
||||
assert get_session_factory(URL_A) is not factory_a
|
||||
|
||||
await dispose_session_factory(URL_A)
|
||||
await dispose_session_factory(URL_B)
|
||||
Reference in New Issue
Block a user