Github Copilot service realignment & cleanup

This commit is contained in:
Jim Lancaster
2026-08-11 16:02:19 -05:00
parent 0ace10269f
commit 8d5aec4301
22 changed files with 1641 additions and 1344 deletions
+24 -17
View File
@@ -2,23 +2,24 @@ from __future__ import annotations
from datetime import UTC
from datetime import datetime
from pathlib import Path
from uuid import uuid4
import pytest
from sqlmodel import select
from transcription.db.models import Document
from transcription.db.models import Job
from transcription.db.models import DocumentPerson
from transcription.db.models import DocumentPersonRole
from transcription.db.models import DocumentType
from transcription.db.models import Job
from transcription.db.models import Person
from transcription.db.models import PersonRole
from transcription.db.models import Source
from transcription.services.documents import DocumentDeleteBlockedError
from transcription.services.documents import DocumentError
from transcription.services.documents import DocumentService
from transcription.services.people import PeopleError
from transcription.services.people import PeopleService
@pytest.mark.asyncio
@@ -123,6 +124,7 @@ async def test_delete_document_succeeds_when_unlinked(default_session_factory, t
@pytest.mark.asyncio
async def test_delete_document_removes_person_links(default_session_factory, tmp_path):
service = DocumentService(session_factory=default_session_factory)
people_service = PeopleService(session_factory=default_session_factory)
service.settings.upload_dir = tmp_path
document = await service.create_document(
@@ -132,8 +134,8 @@ async def test_delete_document_removes_person_links(default_session_factory, tmp
document_type="memo",
)
)
person = await service.create_person(Person(full_name="Linked Person"))
await service.create_document_person(
person = await people_service.create_person(Person(full_name="Linked Person"))
await people_service.create_document_person(
DocumentPerson(
document_id=document.id,
person_id=person.id,
@@ -141,7 +143,7 @@ async def test_delete_document_removes_person_links(default_session_factory, tmp
)
)
links_before_delete = await service.list_document_people(document_id=document.id)
links_before_delete = await people_service.list_document_people(document_id=document.id)
assert len(links_before_delete) == 1
assert links_before_delete[0].role_id is not None
assert links_before_delete[0].role_ref is not None
@@ -153,7 +155,7 @@ async def test_delete_document_removes_person_links(default_session_factory, tmp
await service.delete_document(document)
assert not document_dir.exists()
assert await service.list_document_people(document_id=document.id) == []
assert await people_service.list_document_people(document_id=document.id) == []
with pytest.raises(DocumentError):
await service.read_document_detail(document.id)
@@ -189,6 +191,7 @@ async def test_delete_document_removes_populated_storage_tree(default_session_fa
@pytest.mark.asyncio
async def test_read_person_detail_loads_document_links(default_session_factory):
service = DocumentService(session_factory=default_session_factory)
people_service = PeopleService(session_factory=default_session_factory)
document = await service.create_document(
Document(
@@ -197,8 +200,8 @@ async def test_read_person_detail_loads_document_links(default_session_factory):
document_type="letter",
)
)
person = await service.create_person(Person(full_name="Linked Person"))
await service.create_document_person(
person = await people_service.create_person(Person(full_name="Linked Person"))
await people_service.create_document_person(
DocumentPerson(
document_id=document.id,
person_id=person.id,
@@ -206,7 +209,7 @@ async def test_read_person_detail_loads_document_links(default_session_factory):
)
)
detail = await service.read_person_detail(person.id)
detail = await people_service.read_person_detail(person.id)
assert detail.id == person.id
assert len(detail.document_people) == 1
@@ -216,7 +219,7 @@ async def test_read_person_detail_loads_document_links(default_session_factory):
@pytest.mark.asyncio
async def test_update_person_refreshes_updated_timestamp(default_session_factory):
service = DocumentService(session_factory=default_session_factory)
service = PeopleService(session_factory=default_session_factory)
created = await service.create_person(
Person(
@@ -235,9 +238,10 @@ async def test_update_person_refreshes_updated_timestamp(default_session_factory
@pytest.mark.asyncio
async def test_delete_person_removes_links_when_linked_documents_exist(default_session_factory):
service = DocumentService(session_factory=default_session_factory)
documents_service = DocumentService(session_factory=default_session_factory)
service = PeopleService(session_factory=default_session_factory)
document = await service.create_document(
document = await documents_service.create_document(
Document(
id=uuid4(),
name="block-person-delete-doc",
@@ -258,19 +262,19 @@ async def test_delete_person_removes_links_when_linked_documents_exist(default_s
links = await service.list_document_people(person_id=person.id)
assert links == []
with pytest.raises(DocumentError):
with pytest.raises(PeopleError):
await service.read_person_detail(person.id)
@pytest.mark.asyncio
async def test_delete_person_succeeds_when_unlinked(default_session_factory):
service = DocumentService(session_factory=default_session_factory)
service = PeopleService(session_factory=default_session_factory)
person = await service.create_person(Person(full_name="Free Person"))
await service.delete_person(person)
with pytest.raises(DocumentError):
with pytest.raises(PeopleError):
await service.read_person_detail(person.id)
@@ -292,9 +296,12 @@ async def test_create_document_reuses_existing_document_type_registry(default_se
@pytest.mark.asyncio
async def test_update_document_person_sets_role_id_from_legacy_role(default_session_factory):
service = DocumentService(session_factory=default_session_factory)
documents_service = DocumentService(session_factory=default_session_factory)
service = PeopleService(session_factory=default_session_factory)
document = await service.create_document(Document(id=uuid4(), name="role-sync-doc", document_type="letter"))
document = await documents_service.create_document(
Document(id=uuid4(), name="role-sync-doc", document_type="letter")
)
person = await service.create_person(Person(full_name="Role Sync Person"))
link = await service.create_document_person(
DocumentPerson(
+36 -16
View File
@@ -9,31 +9,33 @@ from transcription.db.models import Document
from transcription.db.models import Job
from transcription.db.models import JobSource
from transcription.db.models import Source
from transcription.services.store import UploadError
from transcription.services.store import create_upload_job
from transcription.services.people import store_person_portrait
from transcription.services.sources import source_mime_type
from transcription.services.store import SourceStorageError
from transcription.services.store import create_document_job
from transcription.services.store import create_job_for_document
from transcription.services.store import store_person_portrait
from transcription.services.store import store_source_file
@pytest.mark.asyncio
async def test_create_job_for_document_requires_at_least_one_upload(async_session, tmp_path):
async def test_create_job_for_document_requires_at_least_one_source(async_session, tmp_path):
document = Document(id=uuid4(), name="needs-upload")
async_session.add(document)
await async_session.commit()
settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path)
with pytest.raises(UploadError):
with pytest.raises(SourceStorageError):
await create_job_for_document(
document_id=document.id,
uploads=[],
source_files=[],
session=async_session,
settings=settings,
)
@pytest.mark.asyncio
async def test_create_job_for_document_sorts_uploads_and_creates_links(async_session, tmp_path):
async def test_create_job_for_document_sorts_sources_and_creates_links(async_session, tmp_path):
document = Document(id=uuid4(), name="ordered-upload-doc")
async_session.add(document)
await async_session.commit()
@@ -42,7 +44,7 @@ async def test_create_job_for_document_sorts_uploads_and_creates_links(async_ses
result = await create_job_for_document(
document_id=document.id,
uploads=[
source_files=[
("folder/b_page.pdf", b"b"),
("folder/A_page.pdf", b"a"),
],
@@ -61,9 +63,7 @@ async def test_create_job_for_document_sorts_uploads_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(Source.page_number) # pyright: ignore[reportArgumentType]
)
).all()
assert [source.upload_name for source in sources] == ["A_page.pdf", "b_page.pdf"]
@@ -83,10 +83,10 @@ async def test_create_job_for_document_sorts_uploads_and_creates_links(async_ses
@pytest.mark.asyncio
async def test_create_upload_job_stores_source_under_document_id_directory(async_session, tmp_path):
async def test_create_document_job_stores_source_under_document_id_directory(async_session, tmp_path):
settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path)
result = await create_upload_job(
result = await create_document_job(
filename="single-page.jpg",
file_bytes=b"image-bytes",
session=async_session,
@@ -99,9 +99,7 @@ async def test_create_upload_job_stores_source_under_document_id_directory(async
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(Source.page_number) # pyright: ignore[reportArgumentType]
)
).first()
assert source is not None
@@ -129,3 +127,25 @@ def test_store_person_portrait_stores_file_under_person_id_directory(tmp_path):
assert stored_path.parent == (tmp_path / "persons" / str(person_id))
assert stored_path.exists()
@pytest.mark.parametrize(
("filename", "expected_mime_type"),
[
("page.jpg", "image/jpeg"),
("page.JPEG", "image/jpeg"),
("page.png", "image/png"),
("page.tif", "image/tiff"),
("page.TIFF", "image/tiff"),
("page.pdf", "application/pdf"),
],
)
def test_source_mime_type_uses_canonical_source_policy(filename, expected_mime_type):
assert source_mime_type(filename) == expected_mime_type
def test_source_storage_rejects_unsupported_format(tmp_path):
settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path)
with pytest.raises(SourceStorageError):
store_source_file(filename="page.txt", file_bytes=b"text", settings=settings)
+14 -12
View File
@@ -1,4 +1,4 @@
"""Tests for source revision behavior in TranscriptionService."""
"""Tests for SourceService revision behavior."""
from uuid import uuid4
@@ -12,20 +12,20 @@ from transcription.db.models import JobStatus
from transcription.db.models import Source
from transcription.services.documents import DocumentService
from transcription.services.jobs import JobService
from transcription.services.transcription import SourceDeleteBlockedError
from transcription.services.transcription import TranscriptionNotFoundError
from transcription.services.transcription import TranscriptionService
from transcription.services.sources import SourceDeleteBlockedError
from transcription.services.sources import SourceService
from transcription.services.sources import TranscriptionNotFoundError
@pytest.mark.integration
class TestTranscriptionServiceRevisionUpsert:
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 = TranscriptionService(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)
@@ -62,7 +62,7 @@ class TestTranscriptionServiceRevisionUpsert:
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 = TranscriptionService(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)
@@ -97,10 +97,12 @@ class TestTranscriptionServiceRevisionUpsert:
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):
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)
transcriptions = TranscriptionService(session_factory=default_session_factory)
transcriptions = SourceService(session_factory=default_session_factory)
transcriptions.settings.upload_dir = tmp_path
document = Document(id=uuid4(), name="delete-source-success")
@@ -139,7 +141,7 @@ class TestTranscriptionServiceRevisionUpsert:
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 = TranscriptionService(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)
@@ -172,7 +174,7 @@ class TestTranscriptionServiceRevisionUpsert:
@pytest.mark.asyncio
async def test_delete_unlinked_source_succeeds(self, default_session_factory, tmp_path):
documents = DocumentService(session_factory=default_session_factory)
transcriptions = TranscriptionService(session_factory=default_session_factory)
transcriptions = SourceService(session_factory=default_session_factory)
transcriptions.settings.upload_dir = tmp_path
document = Document(id=uuid4(), name="delete-unlinked-source")
@@ -203,7 +205,7 @@ class TestTranscriptionServiceRevisionUpsert:
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 = TranscriptionService(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)
+21 -20
View File
@@ -13,48 +13,50 @@ from transcription.db.models import Source
from transcription.services.documents import DocumentDeleteBlockedError
from transcription.services.documents import DocumentService
from transcription.services.jobs import JobService
from transcription.services.transcription import SourceDeleteBlockedError
from transcription.services.transcription import TranscriptionService
from transcription.services.people import PeopleService
from transcription.services.sources import SourceDeleteBlockedError
from transcription.services.sources import SourceService
@pytest.mark.asyncio
async def test_document_service_handles_person_and_document_person_crud(default_session_factory):
async def test_people_service_handles_person_and_document_person_crud(default_session_factory):
documents = DocumentService(session_factory=default_session_factory)
people_service = PeopleService(session_factory=default_session_factory)
document = await documents.create_document(Document(id=uuid4(), name="person-doc"))
person = await documents.create_person(Person(full_name="Ada Lovelace"))
person = await people_service.create_person(Person(full_name="Ada Lovelace"))
assert document.document_type_id is None
link = await documents.create_document_person(
link = await people_service.create_document_person(
DocumentPerson(document_id=document.id, person_id=person.id, role=DocumentPersonRole.AUTHOR)
)
fetched = await documents.read_document_person(link.id)
fetched = await people_service.read_document_person(link.id)
assert fetched.id == link.id
assert fetched.role == DocumentPersonRole.AUTHOR
assert fetched.role_id is not None
updated_link = await documents.update_document_person(
updated_link = await people_service.update_document_person(
DocumentPerson(id=link.id, document_id=document.id, person_id=person.id, role=DocumentPersonRole.RECIPIENT)
)
assert updated_link.role == DocumentPersonRole.RECIPIENT
assert updated_link.role_id is not None
listed = await documents.list_document_people(document_id=document.id)
listed = await people_service.list_document_people(document_id=document.id)
assert len(listed) == 1
people = await documents.list_people()
people = await people_service.list_people()
assert len(people) == 1
await documents.delete_document_person(updated_link)
assert len(await documents.list_document_people(document_id=document.id)) == 0
await people_service.delete_document_person(updated_link)
assert len(await people_service.list_document_people(document_id=document.id)) == 0
@pytest.mark.asyncio
async def test_transcription_service_manages_source_crud(default_session_factory):
documents = DocumentService(session_factory=default_session_factory)
transcriptions = TranscriptionService(session_factory=default_session_factory)
transcriptions = SourceService(session_factory=default_session_factory)
document = await documents.create_document(Document(id=uuid4(), name="source-doc"))
source = await transcriptions.create_source(
@@ -88,9 +90,7 @@ async def test_transcription_service_manages_source_crud(default_session_factory
@pytest.mark.asyncio
async def test_transcription_service_job_source_crud_uses_caller_session(default_session_factory):
documents = DocumentService(session_factory=default_session_factory)
jobs = JobService(session_factory=default_session_factory)
transcriptions = TranscriptionService(session_factory=default_session_factory)
transcriptions = SourceService(session_factory=default_session_factory)
async with transcriptions._session_scope() as session:
document = Document(id=uuid4(), name="job-source-doc")
@@ -138,10 +138,11 @@ async def test_transcription_service_job_source_crud_uses_caller_session(default
@pytest.mark.asyncio
async def test_document_detail_loads_linked_person_relationship(default_session_factory):
documents = DocumentService(session_factory=default_session_factory)
people_service = PeopleService(session_factory=default_session_factory)
document = await documents.create_document(Document(id=uuid4(), name="detail-person-doc"))
person = await documents.create_person(Person(full_name="Grace Hopper"))
await documents.create_document_person(
person = await people_service.create_person(Person(full_name="Grace Hopper"))
await people_service.create_document_person(
DocumentPerson(
document_id=document.id,
person_id=person.id,
@@ -162,7 +163,7 @@ async def test_document_detail_loads_linked_person_relationship(default_session_
async def test_document_delete_is_blocked_with_source_and_job_dependencies(default_session_factory):
documents = DocumentService(session_factory=default_session_factory)
jobs = JobService(session_factory=default_session_factory)
transcriptions = TranscriptionService(session_factory=default_session_factory)
transcriptions = SourceService(session_factory=default_session_factory)
document = await documents.create_document(Document(id=uuid4(), name="blocked-by-deps"))
job = await jobs.create_job(Job(document_id=document.id))
@@ -197,7 +198,7 @@ async def test_document_delete_is_blocked_with_source_and_job_dependencies(defau
async def test_source_delete_blocks_when_linked_to_multiple_jobs(default_session_factory):
documents = DocumentService(session_factory=default_session_factory)
jobs = JobService(session_factory=default_session_factory)
transcriptions = TranscriptionService(session_factory=default_session_factory)
transcriptions = SourceService(session_factory=default_session_factory)
document = await documents.create_document(Document(id=uuid4(), name="multi-job-source-doc"))
job_one = await jobs.create_job(Job(document_id=document.id))
@@ -229,7 +230,7 @@ async def test_source_delete_blocks_when_linked_to_multiple_jobs(default_session
async def test_update_job_source_transcription_persists_provider_json_payloads(default_session_factory):
documents = DocumentService(session_factory=default_session_factory)
jobs = JobService(session_factory=default_session_factory)
transcriptions = TranscriptionService(session_factory=default_session_factory)
transcriptions = SourceService(session_factory=default_session_factory)
document = await documents.create_document(Document(id=uuid4(), name="provider-payloads-doc"))
job = await jobs.create_job(Job(document_id=document.id))
+2 -2
View File
@@ -36,8 +36,8 @@ class TestWorkflowReliability:
)
object.__setattr__(
services,
"transcriptions",
services.transcriptions.__class__(session_factory=default_session_factory),
"sources",
services.sources.__class__(session_factory=default_session_factory),
)
async with services.jobs._session_scope() as session: