generated from john/python-template
zoltan57andCopilot App
7dd0d2c9bf
Decompose SourceService along the aggregate boundary and then correct the
instruction file that caused it to grow, in that order. The refactor is the
empirical test of the rule.
services/evidence.py (new)
EvidenceService owns ExecutionAttempt: read_latest_execution_attempt,
list_execution_attempts, promote_machine_attempt, build_evidence_export,
plus the LatestExecutionAttempt projection. Moved verbatim from sources.py.
services/errors.py (new)
The five-class error hierarchy (PromptLoadError, TranscriptionError,
TranscriptionNotFoundError, SourceDeleteBlockedError,
CandidatePromotionError) moved out of sources.py. evidence.py needs
TranscriptionNotFoundError, and test_service_boundaries.py correctly
rejected the sibling import. errors.py defines no *Service class, so it is
a legal shared home. This was the boundary test doing its job, not an
obstacle to route around.
sources.py 1,389 -> 885 lines (1,063 after Phase 2).
services/__init__.py
ServiceBundle and from_session_factory register evidence. Note that
field-by-field ServiceBundle construction silently binds services to the
process-global session factory via default_factory; from_session_factory is
the only safe constructor. Two test bundles were fixed for this.
.github/instructions/services.instructions.md
Rewritten to describe the boundaries the decomposition actually produced,
per plan Phase 3 task 7 and review log [59].
- "1 service class per data model" -> one service class per aggregate.
The table-shaped rule is the measured cause of sources.py reaching
1,389 lines; DocumentType has no lifecycle without Document.
- New Model Ownership section. Junctions are owned by their lifecycle
owner, the service that creates and deletes the rows: document_person
to PeopleService (sole writer, measured), job_source to SourceService.
Two carve-outs are stated rather than left as silent violations:
cascade deletion when a service deletes its own aggregate root, and
status transitions that create and delete nothing (cancel_job,
resubmit_failed_sources), which are Job lifecycle events on the work
queue. EvidenceService.promote_machine_attempt's two-field write to
Source is named and scoped.
- Mandatory CRUD softened to intent. It was already false: five modules
define no service class, EvidenceService has no create/delete because
ExecutionAttempt is append-only, RegistryService uses <op>_entry.
- Separated reading across models via eager loads from the owning root,
which is allowed, from importing another service, which is not. The old
line 13 and lines 75-77 read as contradictory.
- Typo: picutre.
No code was moved to satisfy the rule.
tests/test_service_boundaries.py
Docstring no longer cites the instruction file by line number; that anchor
would desynchronise silently. errors.py added to the neutral-module list.
Verified: 292 passed, 4 skipped, 0 ruff, 0 ty. All 25 /ui/* routes walked
against the live app; 24x 200. /ui/documents/{id}/sources 404s via a 307 that
drops the /ui prefix, confirmed pre-existing (last touched in 6a3ee26) and
left alone as out of scope.
Co-authored-by: Copilot App <[email protected]>
345 lines
14 KiB
Python
345 lines
14 KiB
Python
"""Tests for SourceService revision behavior."""
|
|
|
|
from uuid import uuid4
|
|
|
|
import pytest
|
|
from sqlalchemy import event
|
|
|
|
from transcription.config import Settings
|
|
from transcription.db.models import Document
|
|
from transcription.db.models import Job
|
|
from transcription.db.models import JobSource
|
|
from transcription.db.models import JobSourceStatus
|
|
from transcription.db.models import JobStatus
|
|
from transcription.db.models import Source
|
|
from transcription.services.documents import DocumentService
|
|
from transcription.services.errors import SourceDeleteBlockedError
|
|
from transcription.services.errors import TranscriptionNotFoundError
|
|
from transcription.services.jobs import JobService
|
|
from transcription.services.sources import SourceService
|
|
|
|
|
|
@pytest.mark.integration
|
|
class TestSourceServiceRevisionUpsert:
|
|
"""Verify page-level source revision semantics."""
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_upsert_revision_creates_new_revision(self, default_session_factory):
|
|
documents = DocumentService(session_factory=default_session_factory)
|
|
jobs = JobService(session_factory=default_session_factory)
|
|
transcriptions = SourceService(session_factory=default_session_factory)
|
|
|
|
document = Document(id=uuid4(), name="revision-create")
|
|
await documents.create_document(document=document)
|
|
|
|
job = Job(document_id=document.id, status=JobStatus.TRANSCRIBED)
|
|
await jobs.create_job(job=job)
|
|
|
|
source = Source(
|
|
document_id=document.id,
|
|
page_number=1,
|
|
upload_name="source.jpg",
|
|
filename="source.jpg",
|
|
file_path="uploads/source.jpg",
|
|
file_hash="1" * 64,
|
|
file_size_bytes=1,
|
|
)
|
|
async with transcriptions._session_scope() as session:
|
|
session.add(source)
|
|
await session.flush()
|
|
session.add(JobSource(job_id=job.id, source_id=source.id, status=JobSourceStatus.PENDING))
|
|
await session.commit()
|
|
await session.refresh(source)
|
|
|
|
revision = await transcriptions.upsert_revision_for_source(source_id=source.id, text="User revision")
|
|
fetched = await transcriptions.read_revision_by_source(source.id)
|
|
|
|
assert revision.id == source.id
|
|
assert revision.revised_text == "User revision"
|
|
assert fetched is not None
|
|
assert fetched.id == source.id
|
|
assert fetched.revised_text == "User revision"
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_upsert_revision_updates_existing_single_revision(self, default_session_factory):
|
|
documents = DocumentService(session_factory=default_session_factory)
|
|
jobs = JobService(session_factory=default_session_factory)
|
|
transcriptions = SourceService(session_factory=default_session_factory)
|
|
|
|
document = Document(id=uuid4(), name="revision-update")
|
|
await documents.create_document(document=document)
|
|
|
|
job = Job(document_id=document.id, status=JobStatus.TRANSCRIBED)
|
|
await jobs.create_job(job=job)
|
|
|
|
source = Source(
|
|
document_id=document.id,
|
|
page_number=1,
|
|
upload_name="source.jpg",
|
|
filename="source.jpg",
|
|
file_path="uploads/source.jpg",
|
|
file_hash="2" * 64,
|
|
file_size_bytes=1,
|
|
)
|
|
async with transcriptions._session_scope() as session:
|
|
session.add(source)
|
|
await session.flush()
|
|
session.add(JobSource(job_id=job.id, source_id=source.id, status=JobSourceStatus.PENDING))
|
|
await session.commit()
|
|
await session.refresh(source)
|
|
|
|
first = await transcriptions.upsert_revision_for_source(source_id=source.id, text="Revision v1")
|
|
second = await transcriptions.upsert_revision_for_source(source_id=source.id, text="Revision v2")
|
|
revisions = await transcriptions.list_revisions_by_job(job.id)
|
|
|
|
assert first.id == second.id
|
|
assert second.revised_text == "Revision v2"
|
|
assert len(revisions) == 1
|
|
assert revisions[0].id == first.id
|
|
assert revisions[0].revised_text == "Revision v2"
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_delete_source_from_job_context_removes_source_and_single_link(
|
|
self, default_session_factory, tmp_path
|
|
):
|
|
documents = DocumentService(session_factory=default_session_factory)
|
|
jobs = JobService(session_factory=default_session_factory)
|
|
settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path)
|
|
transcriptions = SourceService(session_factory=default_session_factory, settings=settings)
|
|
|
|
document = Document(id=uuid4(), name="delete-source-success")
|
|
await documents.create_document(document=document)
|
|
|
|
job = Job(document_id=document.id, status=JobStatus.QUEUED)
|
|
await jobs.create_job(job=job)
|
|
|
|
stored_path = tmp_path / "documents" / str(document.id) / "delete.jpg"
|
|
stored_path.parent.mkdir(parents=True, exist_ok=True)
|
|
stored_path.write_bytes(b"data")
|
|
|
|
source = Source(
|
|
document_id=document.id,
|
|
page_number=1,
|
|
upload_name="delete.jpg",
|
|
filename="delete.jpg",
|
|
file_path=str(stored_path),
|
|
file_hash="3" * 64,
|
|
file_size_bytes=4,
|
|
)
|
|
async with transcriptions._session_scope() as session:
|
|
session.add(source)
|
|
await session.flush()
|
|
session.add(JobSource(job_id=job.id, source_id=source.id, status=JobSourceStatus.PENDING))
|
|
await session.commit()
|
|
await session.refresh(source)
|
|
|
|
await transcriptions.delete_source_from_job_context(job_id=job.id, source_id=source.id)
|
|
|
|
with pytest.raises(TranscriptionNotFoundError):
|
|
await transcriptions.read_source(source.id)
|
|
assert not stored_path.exists()
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_delete_source_from_job_context_blocks_when_other_job_links_exist(self, default_session_factory):
|
|
documents = DocumentService(session_factory=default_session_factory)
|
|
jobs = JobService(session_factory=default_session_factory)
|
|
transcriptions = SourceService(session_factory=default_session_factory)
|
|
|
|
document = Document(id=uuid4(), name="delete-source-blocked")
|
|
await documents.create_document(document=document)
|
|
|
|
job_one = Job(document_id=document.id, status=JobStatus.QUEUED)
|
|
job_two = Job(document_id=document.id, status=JobStatus.QUEUED)
|
|
await jobs.create_job(job=job_one)
|
|
await jobs.create_job(job=job_two)
|
|
|
|
source = Source(
|
|
document_id=document.id,
|
|
page_number=1,
|
|
upload_name="shared.jpg",
|
|
filename="shared.jpg",
|
|
file_path="uploads/shared.jpg",
|
|
file_hash="4" * 64,
|
|
file_size_bytes=1,
|
|
)
|
|
async with transcriptions._session_scope() as session:
|
|
session.add(source)
|
|
await session.flush()
|
|
session.add(JobSource(job_id=job_one.id, source_id=source.id, status=JobSourceStatus.PENDING))
|
|
session.add(JobSource(job_id=job_two.id, source_id=source.id, status=JobSourceStatus.PENDING))
|
|
await session.commit()
|
|
await session.refresh(source)
|
|
|
|
with pytest.raises(SourceDeleteBlockedError):
|
|
await transcriptions.delete_source_from_job_context(job_id=job_one.id, source_id=source.id)
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_delete_unlinked_source_succeeds(self, default_session_factory, tmp_path):
|
|
documents = DocumentService(session_factory=default_session_factory)
|
|
settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path)
|
|
transcriptions = SourceService(session_factory=default_session_factory, settings=settings)
|
|
|
|
document = Document(id=uuid4(), name="delete-unlinked-source")
|
|
await documents.create_document(document=document)
|
|
|
|
stored_path = tmp_path / "documents" / str(document.id) / "orphan.jpg"
|
|
stored_path.parent.mkdir(parents=True, exist_ok=True)
|
|
stored_path.write_bytes(b"data")
|
|
|
|
source = Source(
|
|
document_id=document.id,
|
|
page_number=1,
|
|
upload_name="orphan.jpg",
|
|
filename="orphan.jpg",
|
|
file_path=str(stored_path),
|
|
file_hash="5" * 64,
|
|
file_size_bytes=4,
|
|
)
|
|
await transcriptions.create_source(source=source)
|
|
|
|
await transcriptions.delete_unlinked_source(source_id=source.id)
|
|
|
|
with pytest.raises(TranscriptionNotFoundError):
|
|
await transcriptions.read_source(source.id)
|
|
assert not stored_path.exists()
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_delete_unlinked_source_blocks_when_linked(self, default_session_factory):
|
|
documents = DocumentService(session_factory=default_session_factory)
|
|
jobs = JobService(session_factory=default_session_factory)
|
|
transcriptions = SourceService(session_factory=default_session_factory)
|
|
|
|
document = Document(id=uuid4(), name="delete-unlinked-blocked")
|
|
await documents.create_document(document=document)
|
|
|
|
job = Job(document_id=document.id, status=JobStatus.QUEUED)
|
|
await jobs.create_job(job=job)
|
|
|
|
source = Source(
|
|
document_id=document.id,
|
|
page_number=1,
|
|
upload_name="linked.jpg",
|
|
filename="linked.jpg",
|
|
file_path="uploads/linked.jpg",
|
|
file_hash="6" * 64,
|
|
file_size_bytes=1,
|
|
)
|
|
async with transcriptions._session_scope() as session:
|
|
session.add(source)
|
|
await session.flush()
|
|
session.add(JobSource(job_id=job.id, source_id=source.id, status=JobSourceStatus.PENDING))
|
|
await session.commit()
|
|
await session.refresh(source)
|
|
|
|
with pytest.raises(SourceDeleteBlockedError):
|
|
await transcriptions.delete_unlinked_source(source_id=source.id)
|
|
|
|
|
|
@pytest.mark.integration
|
|
class TestSourceServiceQueryShape:
|
|
"""LOW-08: reads must filter and bound in SQL, not in Python."""
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_list_sources_detail_filters_job_id_with_a_join(self, default_session_factory):
|
|
documents = DocumentService(session_factory=default_session_factory)
|
|
jobs = JobService(session_factory=default_session_factory)
|
|
transcriptions = SourceService(session_factory=default_session_factory)
|
|
|
|
document = Document(id=uuid4(), name="join-filter")
|
|
await documents.create_document(document=document)
|
|
job = Job(document_id=document.id, status=JobStatus.QUEUED)
|
|
other_job = Job(document_id=document.id, status=JobStatus.QUEUED)
|
|
await jobs.create_job(job=job)
|
|
await jobs.create_job(job=other_job)
|
|
|
|
linked = Source(
|
|
document_id=document.id,
|
|
page_number=1,
|
|
upload_name="linked.jpg",
|
|
filename="linked.jpg",
|
|
file_path="uploads/linked.jpg",
|
|
file_hash="7" * 64,
|
|
file_size_bytes=1,
|
|
)
|
|
unlinked = Source(
|
|
document_id=document.id,
|
|
page_number=2,
|
|
upload_name="unlinked.jpg",
|
|
filename="unlinked.jpg",
|
|
file_path="uploads/unlinked.jpg",
|
|
file_hash="8" * 64,
|
|
file_size_bytes=1,
|
|
)
|
|
async with transcriptions._session_scope() as session:
|
|
session.add_all((linked, unlinked))
|
|
await session.flush()
|
|
session.add(JobSource(job_id=job.id, source_id=linked.id, status=JobSourceStatus.PENDING))
|
|
session.add(JobSource(job_id=other_job.id, source_id=unlinked.id, status=JobSourceStatus.PENDING))
|
|
await session.commit()
|
|
|
|
statements: list[str] = []
|
|
|
|
async with transcriptions._session_scope() as session:
|
|
bind = session.get_bind()
|
|
|
|
def capture(_conn, _cursor, statement, *_rest):
|
|
statements.append(statement)
|
|
|
|
event.listen(bind, "before_cursor_execute", capture)
|
|
try:
|
|
sources = await transcriptions.list_sources_detail(job_id=job.id, session=session)
|
|
finally:
|
|
event.remove(bind, "before_cursor_execute", capture)
|
|
|
|
assert [source.id for source in sources] == [linked.id]
|
|
|
|
primary = next(item for item in statements if item.lstrip().upper().startswith("SELECT"))
|
|
assert "JOIN" in primary.upper()
|
|
assert "JOBSOURCE" in primary.upper().replace("_", "")
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_read_source_navigation_does_not_scan_every_sibling(self, default_session_factory):
|
|
documents = DocumentService(session_factory=default_session_factory)
|
|
transcriptions = SourceService(session_factory=default_session_factory)
|
|
|
|
document = Document(id=uuid4(), name="navigation-bounds")
|
|
await documents.create_document(document=document)
|
|
|
|
pages = [
|
|
Source(
|
|
document_id=document.id,
|
|
page_number=page_number,
|
|
upload_name=f"page-{page_number}.jpg",
|
|
filename=f"page-{page_number}.jpg",
|
|
file_path=f"uploads/page-{page_number}.jpg",
|
|
file_hash=str(page_number) * 64,
|
|
file_size_bytes=1,
|
|
)
|
|
for page_number in range(1, 5)
|
|
]
|
|
async with transcriptions._session_scope() as session:
|
|
session.add_all(pages)
|
|
await session.commit()
|
|
for page in pages:
|
|
await session.refresh(page)
|
|
|
|
statements: list[str] = []
|
|
|
|
async with transcriptions._session_scope() as session:
|
|
bind = session.get_bind()
|
|
|
|
def capture(_conn, _cursor, statement, *_rest):
|
|
statements.append(statement)
|
|
|
|
event.listen(bind, "before_cursor_execute", capture)
|
|
try:
|
|
navigation = await transcriptions.read_source_navigation(pages[1].id, session=session)
|
|
finally:
|
|
event.remove(bind, "before_cursor_execute", capture)
|
|
|
|
assert navigation.previous_id == pages[0].id
|
|
assert navigation.next_id == pages[2].id
|
|
|
|
adjacency = [item for item in statements if "LIMIT" in item.upper()]
|
|
assert len(adjacency) == 2, statements
|