diff --git a/docs/step3.md b/docs/step3.md index 408c23e..7e24274 100644 --- a/docs/step3.md +++ b/docs/step3.md @@ -234,7 +234,3 @@ Step 3 is done when: 4. Unit tests for provider and service pass. 5. Full suite remains green under `uv run pytest -q`. 6. Step 4 can call a single service function to process queued jobs. - ---- - -If you want, I can now convert this into a **PR-ready markdown checklist** (same format as Step 2) and then implement it once you confirm. \ No newline at end of file diff --git a/src/transcription/services/__init__.py b/src/transcription/services/__init__.py index 0ac47be..d1f3d7d 100644 --- a/src/transcription/services/__init__.py +++ b/src/transcription/services/__init__.py @@ -8,6 +8,12 @@ from transcription.services.transcription import ( load_prompt_text, transcribe_document_image, ) +from transcription.services.upload import ( + SUPPORTED_UPLOAD_EXTENSIONS, + UploadError, + UploadJobResult, + create_upload_job, +) __all__ = [ "DEFAULT_PROMPT_FILE", @@ -16,4 +22,9 @@ __all__ = [ "load_image_payload", "load_prompt_text", "transcribe_document_image", + "SUPPORTED_UPLOAD_EXTENSIONS", + "UploadError", + "UploadJobResult", + "create_upload_job", ] + diff --git a/src/transcription/services/upload.py b/src/transcription/services/upload.py new file mode 100644 index 0000000..71df6cf --- /dev/null +++ b/src/transcription/services/upload.py @@ -0,0 +1,122 @@ +"""Upload service for storing files and creating queued transcription jobs.""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from pathlib import Path +from uuid import UUID, uuid4 + +from sqlmodel import Session + +from transcription.config import Settings, get_settings +from transcription.db import get_session +from transcription.models import Document, Job, JobStatus + +logger = logging.getLogger(__name__) + +SUPPORTED_UPLOAD_EXTENSIONS = {".jpg", ".jpeg", ".png", ".tif", ".tiff", ".pdf"} + + +class UploadError(RuntimeError): + """Raised when uploaded content cannot be persisted safely.""" + + +@dataclass(frozen=True) +class UploadJobResult: + """Summary of created upload records.""" + + document_id: UUID + job_id: UUID + stored_path: Path + original_filename: str + + +def create_upload_job( + *, + filename: str, + file_bytes: bytes, + session: Session | None = None, + settings: Settings | None = None, +) -> UploadJobResult: + """Persist an uploaded file and create document/job records.""" + runtime_settings = settings or get_settings() + _validate_upload(filename=filename, file_bytes=file_bytes) + + upload_dir = runtime_settings.upload_dir + upload_dir.mkdir(parents=True, exist_ok=True) + + stored_name = _build_stored_filename(filename) + stored_path = upload_dir / stored_name + + try: + stored_path.write_bytes(file_bytes) + except OSError as exc: + raise UploadError(f"Failed to persist upload file: {stored_path}") from exc + + try: + if session is not None: + document, job = _create_upload_records(session=session, original_filename=filename, stored_path=stored_path) + else: + with get_session() as local_session: + document, job = _create_upload_records( + session=local_session, + original_filename=filename, + stored_path=stored_path, + ) + except Exception as exc: # noqa: BLE001 + _best_effort_delete(stored_path) + raise UploadError("Failed to create upload database records") from exc + + logger.info("Created upload job document_id=%s job_id=%s", document.id, job.id) + return UploadJobResult( + document_id=document.id, + job_id=job.id, + stored_path=stored_path, + original_filename=Path(filename).name, + ) + + +def _validate_upload(*, filename: str, file_bytes: bytes) -> None: + if not file_bytes: + raise UploadError("Upload payload is empty") + + safe_name = Path(filename).name + if not safe_name: + raise UploadError("Upload filename is required") + + suffix = Path(safe_name).suffix.lower() + if suffix not in SUPPORTED_UPLOAD_EXTENSIONS: + raise UploadError(f"Unsupported upload extension: {suffix}") + + +def _build_stored_filename(filename: str) -> str: + safe_name = Path(filename).name + return f"{uuid4()}_{safe_name}" + + +def _create_upload_records(*, session: Session, original_filename: str, stored_path: Path) -> tuple[Document, Job]: + document = Document( + filename=Path(original_filename).name, + file_path=str(stored_path), + ) + session.add(document) + session.flush() + + job = Job( + document_id=document.id, + status=JobStatus.QUEUED, + ) + session.add(job) + session.commit() + session.refresh(document) + session.refresh(job) + return document, job + + +def _best_effort_delete(path: Path) -> None: + try: + if path.exists(): + path.unlink() + except OSError: + logger.warning("Failed to clean up upload file after DB error: %s", path) \ No newline at end of file diff --git a/src/transcription/worker.py b/src/transcription/worker.py new file mode 100644 index 0000000..ab987d5 --- /dev/null +++ b/src/transcription/worker.py @@ -0,0 +1,95 @@ +"""Background worker for queued transcription jobs.""" + +from __future__ import annotations + +import logging +import time +from datetime import datetime, timezone +from threading import Event + +from sqlmodel import Session, select + +from transcription.db import get_session +from transcription.models import Document, Job, JobStatus, Transcript +from transcription.services.transcription import transcribe_document_image + +logger = logging.getLogger(__name__) + + +def process_next_queued_job(*, session: Session | None = None) -> bool: + """Process the next queued job and persist terminal outcome. + + Returns True when a job was processed, False when no queued job exists. + """ + if session is None: + with get_session() as local_session: + return _process_next_queued_job(session=local_session) + return _process_next_queued_job(session=session) + + +def _process_next_queued_job(*, session: Session) -> bool: + job = session.exec( + select(Job) + .where(Job.status == JobStatus.QUEUED) + .order_by(Job.created_at) + ).first() + + if job is None: + return False + + logger.info("Picked queued job id=%s", job.id) + job.status = JobStatus.PROCESSING + job.updated_at = datetime.now(timezone.utc) + session.add(job) + session.commit() + session.refresh(job) + + document = session.get(Document, job.document_id) + if document is None: + _upsert_transcript(session=session, job_id=job.id, text=None, error_detail="Document not found") + job.status = JobStatus.FAILED + job.updated_at = datetime.now(timezone.utc) + session.add(job) + session.commit() + logger.error("Job failed because document was missing job_id=%s", job.id) + return True + + try: + result = transcribe_document_image(document.file_path) + _upsert_transcript(session=session, job_id=job.id, text=result.text, error_detail=None) + job.status = JobStatus.TRANSCRIBED + logger.info("Job transcribed job_id=%s provider=%s", job.id, result.provider) + except Exception as exc: # noqa: BLE001 + _upsert_transcript(session=session, job_id=job.id, text=None, error_detail=str(exc)) + job.status = JobStatus.FAILED + logger.exception("Job failed job_id=%s", job.id) + + job.updated_at = datetime.now(timezone.utc) + session.add(job) + session.commit() + return True + + +def _upsert_transcript(*, session: Session, job_id, text: str | None, error_detail: str | None) -> Transcript: + transcript = session.exec(select(Transcript).where(Transcript.job_id == job_id)).first() + if transcript is None: + transcript = Transcript(job_id=job_id) + + transcript.text = text + transcript.error_detail = error_detail + session.add(transcript) + session.commit() + session.refresh(transcript) + return transcript + + +def run_worker_loop(*, stop_event: Event | None = None, poll_interval_seconds: float = 1.0) -> None: + """Run worker polling loop until stop_event is set.""" + while True: + if stop_event is not None and stop_event.is_set(): + logger.info("Worker stop event received") + return + + processed = process_next_queued_job() + if not processed: + time.sleep(poll_interval_seconds) diff --git a/tests/artifacts/transcriptions/Book_Two_-_page_02.jpg.txt b/tests/artifacts/transcriptions/Book_Two_-_page_02.jpg.txt index 817d991..5dd7243 100644 --- a/tests/artifacts/transcriptions/Book_Two_-_page_02.jpg.txt +++ b/tests/artifacts/transcriptions/Book_Two_-_page_02.jpg.txt @@ -11,11 +11,11 @@ BOOK 1 had 54 pages; 14 chapters. BOOK 2 has 70 pages; 18 chapters. BOOK 1 con- sisted largely of first generation family history. BOOK 2 throws more light on the second generation. Sidney promises a BOOK 3 and that may begin to do justice to the third generation. We suggest that Sidney get the help of Louis Shinn -who has a chapter in this book (Chapter 16 - The Last 25 Years on the Doumeeq +who has a chapter in this book (Chapter 16 - The Last 25 Years on the Doumecq Plains. Louis has the gift of seeing, recalling and telling. One sentence in -his chapter gives a great tribute to the Doumeeqers--so far as he knows no one -on the Doumeeq Plains went on relief during the depression. That in a nutshell -shows the sturdy character of the residents of the Doumeeq Plains. +his chapter gives a great tribute to the Doumecqers--so far as he knows no one +on the Doumecq Plains went on relief during the depression. That in a nutshell +shows the sturdy character of the residents of the Doumecq Plains. We promised in BOOK 1 that in BOOK 2 we would give the story of the trip of John E. Cochran and wife to Tennessee, Cuba and the Panama Canal. You will see @@ -32,11 +32,10 @@ enough pictures but we had to take only part of them. We think there are great possibilities in reproducing old pictures. We wish we had a Pickard group. Some Pickard descendant may wish to make a collection. -We are much impressed with the future possibilities of getting a complete genealog- +We are much impressed with the future possibilities of getting a complete geneol- ogy of the Pickard family. Mr. Cochran has a fine chapter on the Pickards but to date we have not had the pleasure of finding all of the family dates. We had intended to give more family data in this book but it takes time to get the correct dates. Often times it requires trips to cemeteries to get dates on the tombstones. Winter is no time to collect dates on tombstones. - -2- diff --git a/tests/artifacts/transcriptions/Omie_Writes_Home.pdf.txt b/tests/artifacts/transcriptions/Omie_Writes_Home.pdf.txt index f4290d3..8aeb73a 100644 --- a/tests/artifacts/transcriptions/Omie_Writes_Home.pdf.txt +++ b/tests/artifacts/transcriptions/Omie_Writes_Home.pdf.txt @@ -13,8 +13,11 @@ original envelope with its 2 cent stamp. The letter has a number of references t children. Peter was Louis; Polly was Edith; and the 'little black rascal' referred to Maurice. Miss Saville was the nurse at the Nome Hospital that was mentioned in the article by Inez in the family newsletter two years ago. + Nome Alaska August 26, 1923 + My Dear Ethel et al. + I don't know when I did write or when you did but I am going to write now however and never the less. But I wish I could talk (I can yet but I @@ -23,6 +26,7 @@ and Polly sit up and listen and that little black rascal of yours would fairly sparkle with listening. Can't I see him listening now to all the yarns we told last summer? +[photo of people on a frozen body of water with icebergs and a boat] You see, we-Miss Saville and I, took a trip north on the Buford and it was very interesting. We went north thru the Bering Strait into the Arctic and as far as the Ice Pack. There the captain @@ -33,7 +37,6 @@ along the icebergs and the walrus were 'heisted' on board by the use of the cran tons of freight and the beasts were so huge that they made the pulleys just creak. They were over 12 feet long, as big around as three cows, had no feet but toenails on their flippers or flappers, no head but their body just suddenly ended with a hole for a mouth and big bristles - all around it similar to a currycomb in coarseness; no ears but huge tusks of ivory. They are the most repulsive looking animals imaginable and tho I have always read about them I never expect such disagreeable looking creatures. They had a rough brown hairy skin and some of @@ -76,7 +79,6 @@ us talk. Miss Saville talked quite a bit. Any how if you folks don't like this I all I had to write about and I know Buster'ud listen anyway and I'd soak ole Peter's head if he didn't and Polly would in my lap and I don't know much about the youngest one of yours so likely he would be squawling. But we did surely enjoy our trip and were gone just long enuf. - I expect there were 150 passengers on board and almost or more of the crew and helpers. We had a stateroom down next to the kitchen and 'twas pretty fierce for odor at times. diff --git a/tests/artifacts/transcriptions/Rod_Moser_Letter_-_p1.jpg.txt b/tests/artifacts/transcriptions/Rod_Moser_Letter_-_p1.jpg.txt index 102f81a..ce6d39c 100644 --- a/tests/artifacts/transcriptions/Rod_Moser_Letter_-_p1.jpg.txt +++ b/tests/artifacts/transcriptions/Rod_Moser_Letter_-_p1.jpg.txt @@ -8,24 +8,23 @@ ISBILL & MOSER DEALERS IN GENERAL MERCHANDISE -Vonore, Tenn. Jany 27 – 1913 -Dear Much Aunt Louie -How are you— -few nights ago received a -letter from your folks, So -I decided to write you +Vonore, Tenn. [Janu]ary 27 – 1913 +Dear [Much Au][illegible] +Has at hour a +few nigh [ago I] said a +letter f[rom] your folks, so +I [decide]d to [write] you a few lines myself & -I am contemplating a -trip out west next summer -& would like for [illegible] to go +I am continuously a +trip out just next summer + & I [want] [lo]t [of figures?] to go where I am. - -Am getting +We are getting up in years & unmarried -so you see the object of +so you see the object o[f] my trip is to get a wife -& if there is any old maid -or widdow out there, I +If th[ere] is any old maids +or widows out there I want you to kiss them -at my [illegible] for me at They -as soon as I get there +at my [hand] and my at them +as soon as I get them diff --git a/tests/services/test_upload.py b/tests/services/test_upload.py new file mode 100644 index 0000000..8f12298 --- /dev/null +++ b/tests/services/test_upload.py @@ -0,0 +1,98 @@ +"""Tests for transcription.services.upload.""" + +from pathlib import Path + +import pytest + +from transcription.config import Settings +from transcription.models import Document, Job, JobStatus +from transcription.services.upload import UploadError, create_upload_job + + +@pytest.mark.unit +class TestUploadValidation: + """Verify upload validation behavior.""" + + def test_rejects_empty_bytes(self, session, tmp_path: Path): + """create_upload_job rejects an empty upload payload.""" + settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path) + with pytest.raises(UploadError): + create_upload_job( + filename="letter.jpg", + file_bytes=b"", + session=session, + settings=settings, + ) + + def test_rejects_unsupported_extension(self, session, tmp_path: Path): + """create_upload_job rejects unsupported filename extensions.""" + settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path) + with pytest.raises(UploadError): + create_upload_job( + filename="notes.txt", + file_bytes=b"content", + session=session, + settings=settings, + ) + + +@pytest.mark.integration +class TestUploadPersistence: + """Verify upload file and record persistence behavior.""" + + def test_writes_file_and_creates_records(self, session, tmp_path: Path): + """create_upload_job writes file and creates document/job records.""" + settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path) + + result = create_upload_job( + filename="letter.jpg", + file_bytes=b"image-bytes", + session=session, + settings=settings, + ) + + assert result.stored_path.exists() + assert result.stored_path.read_bytes() == b"image-bytes" + + document = session.get(Document, result.document_id) + job = session.get(Job, result.job_id) + assert document is not None + assert job is not None + assert document.filename == "letter.jpg" + assert document.file_path == str(result.stored_path) + + def test_uses_unique_stored_filename(self, session, tmp_path: Path): + """create_upload_job stores uploads with unique filenames.""" + settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path) + + first = create_upload_job( + filename="duplicate.jpg", + file_bytes=b"first", + session=session, + settings=settings, + ) + second = create_upload_job( + filename="duplicate.jpg", + file_bytes=b"second", + session=session, + settings=settings, + ) + + assert first.stored_path != second.stored_path + assert first.stored_path.exists() + assert second.stored_path.exists() + + def test_sets_job_status_queued(self, session, tmp_path: Path): + """create_upload_job persists a job with queued status.""" + settings = Settings(openrouter_api_key="test-key", upload_dir=tmp_path) + + result = create_upload_job( + filename="queued.pdf", + file_bytes=b"%PDF-1.4", + session=session, + settings=settings, + ) + + job = session.get(Job, result.job_id) + assert job is not None + assert job.status == JobStatus.QUEUED diff --git a/tests/services/test_worker.py b/tests/services/test_worker.py new file mode 100644 index 0000000..49c15b4 --- /dev/null +++ b/tests/services/test_worker.py @@ -0,0 +1,141 @@ +"""Tests for transcription.worker.""" + +from threading import Event + +import pytest +from sqlmodel import select + +from transcription.models import Document, Job, JobStatus, Transcript +from transcription.providers.base import TranscriptionResult +from transcription.worker import process_next_queued_job, run_worker_loop + + +def _create_queued_job(session, *, filename: str = "doc.jpg", file_path: str = "uploads/doc.jpg") -> Job: + document = Document(filename=filename, file_path=file_path) + session.add(document) + session.commit() + session.refresh(document) + + job = Job(document_id=document.id, status=JobStatus.QUEUED) + session.add(job) + session.commit() + session.refresh(job) + return job + + +@pytest.mark.integration +class TestWorkerQueueBehavior: + """Verify worker behavior when selecting queued jobs.""" + + def test_returns_false_when_queue_empty(self, session): + """process_next_queued_job returns False when there are no queued jobs.""" + processed = process_next_queued_job(session=session) + assert processed is False + + +@pytest.mark.integration +class TestWorkerSuccessPath: + """Verify worker success-path lifecycle transitions and transcript persistence.""" + + def test_transitions_processing_to_transcribed(self, session, monkeypatch): + """process_next_queued_job transitions queued jobs to transcribed on success.""" + job = _create_queued_job(session) + + def _fake_transcribe(_path): + return TranscriptionResult(text="ok", provider="openrouter", model="test-model") + + monkeypatch.setattr("transcription.worker.transcribe_document_image", _fake_transcribe) + + processed = process_next_queued_job(session=session) + session.refresh(job) + + assert processed is True + assert job.status == JobStatus.TRANSCRIBED + + def test_persists_transcript_text_on_success(self, session, monkeypatch): + """process_next_queued_job stores transcript text for successful jobs.""" + job = _create_queued_job(session) + + def _fake_transcribe(_path): + return TranscriptionResult(text="Transcript body", provider="openrouter", model="test-model") + + monkeypatch.setattr("transcription.worker.transcribe_document_image", _fake_transcribe) + + process_next_queued_job(session=session) + + transcript = session.exec( + select(Transcript).where(Transcript.job_id == job.id) + ).first() + assert transcript is not None + assert transcript.text == "Transcript body" + assert transcript.error_detail is None + + +@pytest.mark.integration +class TestWorkerFailurePath: + """Verify worker failure-path lifecycle transitions and error persistence.""" + + def test_sets_failed_and_error_detail_on_failure(self, session, monkeypatch): + """process_next_queued_job marks failed and stores error detail on exception.""" + job = _create_queued_job(session) + + def _fake_transcribe(_path): + raise RuntimeError("provider failure") + + monkeypatch.setattr("transcription.worker.transcribe_document_image", _fake_transcribe) + + processed = process_next_queued_job(session=session) + session.refresh(job) + transcript = session.exec( + select(Transcript).where(Transcript.job_id == job.id) + ).first() + + assert processed is True + assert job.status == JobStatus.FAILED + assert transcript is not None + assert transcript.text is None + assert "provider failure" in transcript.error_detail + + def test_updates_existing_transcript_if_present(self, session, monkeypatch): + """process_next_queued_job updates existing transcript instead of duplicating.""" + job = _create_queued_job(session) + existing = Transcript(job_id=job.id, text="old", error_detail=None) + session.add(existing) + session.commit() + session.refresh(existing) + + def _fake_transcribe(_path): + raise RuntimeError("provider failure") + + monkeypatch.setattr("transcription.worker.transcribe_document_image", _fake_transcribe) + + process_next_queued_job(session=session) + + transcripts = session.exec( + select(Transcript).where(Transcript.job_id == job.id) + ).all() + assert len(transcripts) == 1 + assert transcripts[0].id == existing.id + assert transcripts[0].text is None + assert "provider failure" in transcripts[0].error_detail + + +@pytest.mark.unit +class TestWorkerLoopControl: + """Verify worker loop start/stop behavior.""" + + def test_stops_when_stop_event_is_set(self, monkeypatch): + """run_worker_loop exits when a stop event is set.""" + stop_event = Event() + stop_event.set() + + called = {"value": False} + + def _fake_process_next_queued_job(): + called["value"] = True + return False + + monkeypatch.setattr("transcription.worker.process_next_queued_job", _fake_process_next_queued_job) + + run_worker_loop(stop_event=stop_event, poll_interval_seconds=0.01) + assert called["value"] is False