Files
transcription/tests/test_model_contract_guards.py
Jim LancasterandCopilot App 67feeb28af
Quality Gate / gate (push) Failing after 47s
Repair the pre-commit quality gate and clear the ruff backlog
The pre-commit hooks declared `language: system` with bare `ruff`/`ty`
entries, but both are uv-managed dev dependencies and are not on PATH, so every
commit failed with `Executable 'ruff' not found`. Route both through
`uv run`; keep ruff blocking and make ty advisory (verbose) until its 18
whole-project diagnostics are cleared.

With the gate working, clear `ruff check .` to zero:

- 18 auto-fixes (import sorting, blank lines, `max()` simplification,
  `with` merging, unused imports).
- Real defects: `SourceNavigation` annotated but never imported in
  sources_page; two naive `datetime.now()` calls in migration.py now use
  `datetime.now(UTC)`.
- Dead parameters removed: `source_has_photo_table` (computed, passed, never
  read), `_serialize_value(key=...)`, and unused `request` on two NiceGUI
  page handlers where the framework injects it optionally.
- Mechanical line-length wrapping and one `startswith` tuple collapse.
- `# noqa: PLR0915` / `# noqa: PLR1702` on five long UI/migration
  functions, following the convention already used in jobs_page and
  settings_page, rather than refactoring during stabilization.

Full suite green (377 tests, `-m "not external"`).

Co-authored-by: Copilot App <[email protected]>
2026-08-23 16:47:11 -05:00

279 lines
12 KiB
Python

"""Deterministic guards for the model and persistence contracts.
These cover three checks that `.github/skills/python-code-reviewer/skill.md` requires
on every review but that previously had no automated enforcement:
* **Status vocabulary conformance** - status comparisons and assignments must use the
`JobStatus` / `JobSourceStatus` / `JobPurpose` enums rather than string literals.
* **Relationship loading contract** - relationships declare ``lazy="raise"`` so read
paths must eager-load explicitly, and the documented exceptions in `docs/schema.md`
must match the code exactly.
* **Schema contract fidelity** - the "Field-Accurate Table Contracts" tables in
`docs/schema.md` must list exactly the fields each SQLModel table declares.
See `docs/requirements.md` (REQ-4-102), `docs/schema.md` ("Relationship Loading
Contract"), and `.github/instructions/services.instructions.md`.
"""
from __future__ import annotations
import ast
import re
from pathlib import Path
from sqlmodel import SQLModel
from transcription.db import models as models_module
from transcription.db.models import JobPurpose
from transcription.db.models import JobSourceStatus
from transcription.db.models import JobStatus
PROJECT_ROOT = Path(__file__).resolve().parents[1]
SOURCE_DIR = PROJECT_ROOT / "src" / "transcription"
MODELS_PATH = SOURCE_DIR / "db" / "models.py"
SCHEMA_DOC = PROJECT_ROOT / "docs" / "schema.md"
STATUS_ENUMS = (JobStatus, JobSourceStatus, JobPurpose)
STATUS_VALUES = frozenset(member.value for enum in STATUS_ENUMS for member in enum)
# Attribute names that carry a status enum. A string literal compared against or
# assigned to one of these is a stringly-typed status, even if it happens to match.
STATUS_ATTRIBUTES = frozenset({"status", "purpose"})
# `JobSource.execution_attempts` loads attempt evidence on demand rather than raising,
# because evidence is fetched deliberately by the services that own it. Documented in
# `docs/schema.md` under "Relationship Loading Contract".
DOCUMENTED_LOADING_EXCEPTIONS = {"execution_attempts": "noload"}
def _python_files() -> list[Path]:
return sorted(SOURCE_DIR.rglob("*.py"))
def _relative(path: Path) -> str:
return path.relative_to(PROJECT_ROOT).as_posix()
def _is_status_target(node: ast.expr) -> bool:
"""True for `x.status`, `x.purpose`, and their `.value` unwrappings."""
if isinstance(node, ast.Attribute):
if node.attr in STATUS_ATTRIBUTES:
return True
if node.attr == "value":
return _is_status_target(node.value)
return False
def _string_constant(node: ast.expr) -> str | None:
return node.value if isinstance(node, ast.Constant) and isinstance(node.value, str) else None
def _status_literal_violations(tree: ast.Module) -> list[tuple[int, str]]:
found: list[tuple[int, str]] = []
for node in ast.walk(tree):
if isinstance(node, ast.Compare):
operands = [node.left, *node.comparators]
if not any(_is_status_target(operand) for operand in operands):
continue
for operand in operands:
literal = _string_constant(operand)
if literal is not None:
found.append((node.lineno, literal))
elif isinstance(node, ast.Call):
for keyword in node.keywords:
if keyword.arg not in STATUS_ATTRIBUTES:
continue
literal = _string_constant(keyword.value)
if literal is not None:
found.append((node.lineno, literal))
return found
def test_status_enums_expose_expected_vocabulary():
"""Guard the guard: the scan below is meaningless if the enums are empty."""
assert {member.value for member in JobStatus} == {
"queued",
"processing",
"transcribed",
"partial_success",
"failed",
}
assert {member.value for member in JobSourceStatus} == {
"pending",
"transcribed",
"failed",
"cancelled",
}
assert {member.value for member in JobPurpose} == {"transcription", "retranscription"}
def test_no_stringly_typed_status_comparisons_or_assignments():
"""Status handling must go through the enums, never raw strings.
`attempt.status.value == "transcribed"` silently survives an enum rename and
compares a projection of the value rather than the value itself.
"""
violations: dict[str, list[tuple[int, str]]] = {}
for path in _python_files():
tree = ast.parse(path.read_text(encoding="utf-8"))
found = _status_literal_violations(tree)
if found:
violations[_relative(path)] = found
assert violations == {}
def test_status_string_literals_outside_models_are_accounted_for():
"""Any bare status-valued literal in the package must be a known non-status use.
This is deliberately narrower than the comparison scan: it catches literals that
merely *look* like statuses, so a genuine new one cannot slip in unnoticed.
"""
allowed = {
# `JobStatus` / `JobSourceStatus` / `JobPurpose` member definitions.
"src/transcription/db/models.py",
# `WorkerHealthState` is a separate Literal vocabulary that reuses "failed".
"src/transcription/worker.py",
# Distribution name lookups for `transcription`, not the JobPurpose member.
"src/transcription/config.py",
"src/transcription/providers/evidence.py",
# UI placeholder copy where a value is absent, not a status render.
"src/transcription/ui/pages/jobs_page.py",
}
unexpected: dict[str, list[tuple[int, str]]] = {}
for path in _python_files():
relative = _relative(path)
if relative in allowed:
continue
tree = ast.parse(path.read_text(encoding="utf-8"))
found = [
(node.lineno, node.value)
for node in ast.walk(tree)
if isinstance(node, ast.Constant) and isinstance(node.value, str) and node.value in STATUS_VALUES
]
if found:
unexpected[relative] = found
assert unexpected == {}
def _relationship_loading_strategies() -> dict[str, dict[str, str | None]]:
"""Map each model attribute defined via `Relationship(...)` to its lazy strategy."""
tree = ast.parse(MODELS_PATH.read_text(encoding="utf-8"))
strategies: dict[str, dict[str, str | None]] = {}
for class_node in tree.body:
if not isinstance(class_node, ast.ClassDef):
continue
for statement in class_node.body:
if not isinstance(statement, ast.AnnAssign) or statement.value is None:
continue
call = statement.value
if not isinstance(call, ast.Call) or getattr(call.func, "id", None) != "Relationship":
continue
attribute = statement.target.id if isinstance(statement.target, ast.Name) else "<unknown>"
lazy: str | None = None
for keyword in call.keywords:
if keyword.arg != "sa_relationship_kwargs" or not isinstance(keyword.value, ast.Dict):
continue
for key, value in zip(keyword.value.keys, keyword.value.values, strict=True):
if isinstance(key, ast.Constant) and key.value == "lazy":
lazy = _string_constant(value)
strategies.setdefault(class_node.name, {})[attribute] = lazy
return strategies
def test_relationships_are_discovered():
"""Guard the guard: the loading rules below are meaningless if nothing is scanned."""
strategies = _relationship_loading_strategies()
assert {"Document", "Job", "JobSource", "Source"} <= set(strategies)
assert sum(len(attributes) for attributes in strategies.values()) >= 25
def test_relationships_declare_lazy_raise_except_documented_cases():
"""REQ-4-102: relationships raise on implicit load so read shape stays explicit."""
violations: dict[str, str | None] = {}
for model_name, attributes in _relationship_loading_strategies().items():
for attribute, lazy in attributes.items():
expected = DOCUMENTED_LOADING_EXCEPTIONS.get(attribute, "raise")
if lazy != expected:
violations[f"{model_name}.{attribute}"] = lazy
assert violations == {}
def test_documented_loading_exceptions_match_schema_doc():
"""The exception list is only trustworthy while `docs/schema.md` agrees with it."""
schema_text = SCHEMA_DOC.read_text(encoding="utf-8")
contract = schema_text.split("## Relationship Loading Contract", 1)[1]
for attribute, strategy in DOCUMENTED_LOADING_EXCEPTIONS.items():
assert attribute in contract, f"{attribute} is exempted in code but not documented"
assert f'`lazy="{strategy}"`' in contract
def _documented_table_fields() -> dict[str, list[str]]:
schema_text = SCHEMA_DOC.read_text(encoding="utf-8")
section = schema_text.split("## Field-Accurate Table Contracts", 1)[1]
documented: dict[str, list[str]] = {}
for block in re.split(r"\n### ", section)[1:]:
heading = block.splitlines()[0].strip().strip("`")
documented[heading] = re.findall(r"^\| `([^`]+)` \|", block, re.MULTILINE)
return documented
def _table_models() -> dict[str, type[SQLModel]]:
return {
name: attribute
for name, attribute in vars(models_module).items()
if isinstance(attribute, type)
and issubclass(attribute, SQLModel)
and attribute is not SQLModel
and getattr(attribute, "__table__", None) is not None
}
def test_schema_doc_documents_every_table_model():
"""Every persisted table needs a field contract, and vice versa."""
documented = set(_documented_table_fields())
actual = set(_table_models())
assert actual, "no table models discovered"
assert documented - actual == set(), "schema.md documents tables that no longer exist"
assert actual - documented == set(), "schema.md is missing tables that exist in models.py"
def test_schema_doc_field_contracts_match_models():
"""Check 6: `docs/schema.md` stays field-accurate with `db/models.py`."""
documented = _documented_table_fields()
drift: dict[str, dict[str, list[str]]] = {}
for name, model in _table_models().items():
expected = set(model.model_fields)
listed = set(documented.get(name, []))
if expected != listed:
drift[name] = {
"undocumented_fields": sorted(expected - listed),
"stale_doc_entries": sorted(listed - expected),
}
assert drift == {}
def test_schema_doc_lists_fields_in_declaration_order():
"""Ordering drift is how a doc silently stops being reviewable against the model."""
documented = _documented_table_fields()
misordered = {
name: {"documented": documented[name], "declared": list(model.model_fields)}
for name, model in _table_models().items()
if documented.get(name, []) != list(model.model_fields)
}
assert misordered == {}
def test_schema_doc_enumerations_match_status_enums():
"""The "Authoritative Enumerations" section must list the real members."""
schema_text = SCHEMA_DOC.read_text(encoding="utf-8")
section = schema_text.split("## Authoritative Enumerations", 1)[1].split("\n## ", 1)[0]
drift: dict[str, dict[str, list[str]]] = {}
for enum in STATUS_ENUMS:
block = re.split(rf"\n### {enum.__name__}\n", section)
assert len(block) == 2, f"{enum.__name__} has no section in docs/schema.md"
listed = re.findall(r"^- `([^`]+)`", block[1].split("\n### ", 1)[0], re.MULTILINE)
expected = [member.value for member in enum]
if listed != expected:
drift[enum.__name__] = {"documented": listed, "declared": expected}
assert drift == {}