V4.6 Phase 4: service layer consolidation

Removes the duplicated registry CRUD, the hand-written not-found raises, and
the three divergent media writers. Behavior is preserved: every existing
Document Type and Person Role test passes unchanged, which is the primary
proof for MED-11.

[MED-11] Generic registry service
- New services/registry.py owns RegistryService[ModelT]: list, list with
  counts, create with IntegrityError -> conflict mapping, read, update,
  delete with built-in and referenced guards, is_referenced, and label
  normalization/casefold keying.
- DocumentTypeRegistry and PersonRoleRegistry declare only the model, error
  class, noun, short noun, retainer phrase, and reference columns.
- DocumentService and PeopleService keep their public method names and
  delegate. Every user-facing message, error category, and suggestion string
  is reproduced verbatim; only the noun is templated.
- Deleted _normalize_registry_label, _document_type_label_key,
  _normalize_role_label, _person_role_label_key,
  _document_type_is_referenced, and _person_role_is_referenced.

[MED-12] Shared not-found lookup
- ServiceBase._get_or_raise(model, id, *, session, error, noun, suggestion,
  options) loads by primary key or raises the caller's error type.
- documents.py: local _get_document_or_raise deleted; replaced by _read_document
  and adopted at read_document, delete_document, and set_document_type, which
  previously bypassed the helper and hand-wrote the raise.
- sources.py: 8 identical Source raises and 1 Job raise collapsed into
  _read_source / _get_or_raise.
- jobs.py and people.py already funneled through local _not_found builders and
  were left alone.

[MED-13][MED-01] Single media writer
- New services/media_storage.py owns validate -> name -> mkdir -> write ->
  wrap OSError. The write runs in asyncio.to_thread, so uploads no longer block
  the event loop.
- store_source_file, store_person_portrait, and store_homepage_image now share
  it and are async. Callers in store.py, people_page.py, and home_page.py await
  them. mkdir failures are now also translated to a domain error instead of
  escaping as a raw OSError.
- homepage_store gains HomepageStorageError so its write reports like the others.

[MED-14, partial] Service independence
- New services/source_media.py owns SOURCE_MIME_TYPES, SOURCE_EXTENSIONS,
  lookup_source_mime_type, and supported_source_formats.
- documents.py no longer imports services/sources.py. Its print projection uses
  the non-raising lookup and raises DocumentError, so DocumentService no longer
  emits a TranscriptionError.
- api/v4_print.py imports the mapping from the policy module.
- store.py and workflows.py still import sources.py; both are orchestration
  modules, which services.instructions.md:75-77 explicitly permits.
- Splitting SourceService itself remains deferred to V4.7.

[LOW-08] Query shape
- list_sources_detail filters job_id with a JOIN on JobSource instead of
  loading every Source and filtering in Python.
- read_source_navigation replaces the full ordered-id scan and .index() with
  two row-value comparisons bounded by LIMIT 1.
- list_processing_artifacts gains the limit parameter its summary sibling
  already had.
- build_evidence_export runs artifact integrity hashing and file reads through
  asyncio.to_thread.

Tests
- tests/test_service_boundaries.py: AST guard asserting no service module
  imports a sibling service module, plus a guard that the scan is non-empty.
- tests/services/test_transcription_service.py: asserts the job_id filter emits
  a JOIN, and that navigation emits exactly two LIMIT queries.
- tests/services/test_store.py: the two storage tests are now async.

Verified: 276 passed, 4 skipped; ruff check clean.
This commit is contained in:
zoltan57
2026-08-17 16:46:15 -05:00
parent 7b9715b3f1
commit 97b3d0fd62
15 changed files with 814 additions and 462 deletions
+65
View File
@@ -0,0 +1,65 @@
"""Structural rules for the services package.
`.github/instructions/services.instructions.md:13` requires that service classes
stay independent of one another. Shared behavior belongs in a neutral module
(`base.py`, `registry.py`, `source_media.py`, `media_storage.py`), and any
operation spanning two services belongs in an orchestration module.
"""
from __future__ import annotations
import ast
from pathlib import Path
SERVICES_DIR = Path(__file__).resolve().parents[1] / "src" / "transcription" / "services"
# Modules that intentionally compose several services rather than owning one table.
ORCHESTRATION_MODULES = frozenset({"store", "workflows", "__init__"})
def _module_paths() -> list[Path]:
return sorted(SERVICES_DIR.glob("*.py"))
def _defines_service_class(tree: ast.Module) -> bool:
return any(
isinstance(node, ast.ClassDef) and node.name.endswith("Service") and node.name != "RegistryService"
for node in tree.body
)
def _service_modules() -> dict[str, ast.Module]:
modules: dict[str, ast.Module] = {}
for path in _module_paths():
if path.stem in ORCHESTRATION_MODULES:
continue
tree = ast.parse(path.read_text(encoding="utf-8"))
if _defines_service_class(tree):
modules[path.stem] = tree
return modules
def _imported_sibling_modules(tree: ast.Module) -> set[str]:
imported: set[str] = set()
for node in ast.walk(tree):
if isinstance(node, ast.ImportFrom) and node.level == 1 and node.module:
imported.add(node.module.split(".")[0])
elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module:
parts = node.module.split(".")
if parts[:2] == ["transcription", "services"] and len(parts) > 2:
imported.add(parts[2])
return imported
def test_service_modules_are_discovered():
"""Guard the guard: the rule below is meaningless if nothing is scanned."""
assert set(_service_modules()) >= {"documents", "jobs", "people", "sources"}
def test_no_service_module_imports_another_service_module():
"""MED-14: a service module must not depend on a sibling service module."""
modules = _service_modules()
violations = {
name: sorted(_imported_sibling_modules(tree) & set(modules) - {name}) for name, tree in modules.items()
}
assert {name: found for name, found in violations.items() if found} == {}