transcription/docs/ver4.7/implementation_plan_v4_7.md

# Implementation Plan (Version 4.7)

## Goal

Collapse the duplicated evidence model, remove the `ProcessingArtifact` subsystem and move orientation normalization to ingest, complete the `SourceService` decomposition deferred from V4.6 ([MED-14]), correct the run-time measurement window, and close the remaining correctness and tooling items opened during V4.6. No new user-facing behavior.

## Planning Status

**Frozen.** The boundary is [`scope_boundary_v4_7.md`](scope_boundary_v4_7.md). Feature work is parked in [`../ver4.8/feature_backlog_v4_8.md`](../ver4.8/feature_backlog_v4_8.md).

## Planning Constraints

- Every change traces to a finding ID or a V4.6 review-log entry.
- `ruff check` clean and `ty check` at **0 diagnostics** at the end of every phase, matching the V4.6 exit state.
- The full suite passes at the end of every phase.
- **Test changes are expected in Phases 1 and 2.** This differs from V4.6, where the mechanical moves required no test logic changes. Measured blast radius: 33 references to the removed `job_source` evidence fields across 8 test files, and 10 `ProcessingArtifact` references across 3. Only Phase 4 retains the "no test logic changes" rule.
- Use `.\.venv\Scripts\python.exe -m pytest` (the system interpreter has no packages).
- Back up `data/transcription.db` **and** `data/documents/` before running any migration step. The image backfill rewrites files in place.
- One phase, one commit.

## Expected Project Impact

| Area | Before | After |
| :--- | :--- | :--- |
| `job_source` columns | 9 | **4** - `id`, `job_id`, `source_id`, `status` |
| `JobSourceStatus` on disk | two spellings across two tables | one spelling, plus a new `CANCELLED` member |
| `processing_artifact` | table, model, ~283 lines of service code, 2 rows | removed |
| Orientation normalization | derived per transcription, artifact-backed | applied once at ingest, no derivative |
| Stored image rotation | `quality=95, subsampling=0`, +38% size | qtables reuse, ~6% smaller, higher PSNR |
| `services/sources.py` | 1,389 lines, 4 domain models | ~900 lines, `Source` + `JobSource` |
| `services/evidence.py` | does not exist | ~174 lines, `ExecutionAttempt` reads and export |
| `services/artifacts.py` | planned | **cancelled** - deleted rather than extracted |
| `duration_ms` | provider call + normalization + artifact write + commit | the operation the timeout governs |
| Worker loop errors | every `Exception` logged and suppressed | programming errors distinguishable from provider faults |
| Quality gate | local pre-commit only, inert until installed | enforced in CI |

## Migration Handling

All schema and data changes are delivered by a single idempotent `tools/migrate_v46_to_v47.py`, following the `tools/migrate_v45_to_v46.py` conventions: never invoked at startup, never run by the test suite.

The tool is **built incrementally** - Phase 1 creates it with its own step, Phase 2 appends the next - and is **run at the end of each of those phases** so the live database stays usable at every phase boundary. Idempotency is what makes re-running safe.

Steps, in execution order:

1. Rotate the 58 stored images carrying EXIF orientation 3, in place, using qtables reuse; strip the orientation tag.
2. Drop the `processing_artifact` table and remove its external artifact files.
3. Normalize `execution_attempt.status` to the single declared spelling ([45]).
4. Drop `job_source.raw_transcription`, `ai_metadata`, `raw_api_response`, `executed_at`, `error_detail`.

`tools/migrate_v45_to_v46.py` deliberately accepts both enum spellings because it reads historical backups. Leave that tolerance in place.

## Implementation Phases

### 1. Ingest Normalization and ProcessingArtifact Removal

Deletion comes first, so that Phase 4 never restructures code that is on its way out.

Tasks:

1. Move orientation normalization into the ingest path in `media_storage`, ahead of `write_bytes` (`media_storage.py:57`). Rotate, strip the EXIF orientation tag, then store.
2. Change the encode settings at `normalization.py:84-85` from `quality=95, subsampling=0` to `qtables=im.quantization`, `subsampling=JpegImagePlugin.get_sampling(im)`, `optimize=True`. Import `JpegImagePlugin` explicitly - it is not reachable as an attribute of `PIL.Image`.
3. Delete `resolve_provider_input` and its call at `workflows.py:226`. The stored file is now already upright, so the transcription path reads it directly.
4. Fold the `transcription_quality_warnings` payload (`workflows.py:560-571`) into `execution_attempt.normalized_metadata`.
5. Delete the artifact cluster from `sources.py` (lines 732-1015) and the artifact branch of `build_evidence_export`.
6. Delete the `ProcessingArtifact` model and the `CheckConstraint`.
7. Remove the "Orientation normalized" badge at `sources_page.py:174-178`, the artifact evidence dump at line 417, and the quality-warnings render at line 661.
8. Update `test_normalization.py`, `test_v42_evidence.py`, and `test_db.py`. Normalization tests should now assert on ingest behavior rather than on artifact creation.
9. Create `tools/migrate_v46_to_v47.py` with steps 1 and 2. Back up, run, verify.

Verification: re-check EXIF orientation across `data/documents/` - no stored image should report orientation 3, 6, or 8. Spot-check one backfilled page visually.

Exit: suite green, `ty check` at 0, `processing_artifact` gone from schema and code.

### 2. Evidence Model Simplification

Tasks:

1. Add `JobSourceStatus.CANCELLED`. Update `jobs.py:378-384` to write it instead of `FAILED` plus `"Cancelled by user"`.
2. Declare one spelling for `JobSourceStatus` across both `job_source.status` and `execution_attempt.status` ([45]). `job_source.status` already declares `values_callable`; `execution_attempt.status` does not.
3. Remove `raw_transcription`, `ai_metadata`, `raw_api_response`, `executed_at`, and `error_detail` from the `JobSource` model.
4. Redirect every read to `execution_attempt`:
   - `transcript.py:103-119` sorts by `executed_at` - sort by `ExecutionAttempt.finished_at`.
   - `sources_page.py:390-393` reads `ai_metadata` and `raw_api_response`.
   - `sources_page.py:337-375` renders status, executed time, and error detail.
   - `models.py:266-278` and `models.py:328-343` derive transcript and error from `job_sources`.
5. Shrink `update_job_source_transcription` (`sources.py:524-679`) to write only the surviving `JobSource` columns. Keep the method in `sources.py` and keep both writes in one session scope.
6. Confirm `jobs.py:411-424` retry still works unchanged. It resets `FAILED` to `PENDING`; the attempt history it appears to discard is preserved by `ExecutionAttempt`'s unique constraint.
7. Confirm `workflows.py:432-442` work selection is unaffected. It filters `status != TRANSCRIBED` within `job.job_sources`, so `CANCELLED` pages are excluded from a re-run only if that is the intent - **decide explicitly** whether cancelled pages should be re-attempted, and encode the answer in the filter rather than leaving it implicit.
8. Update the 33 affected test references across the 8 files identified.
9. Append migration steps 3 and 4. Back up, run, verify row counts before and after.

Exit: suite green, `ty check` at 0, `job_source` at 4 columns.

### 3. Stage B - Extract `services/evidence.py` ([MED-14])

Read-side only, now applied to a substantially smaller `sources.py`.

Move:

- `read_latest_execution_attempt` (216-245), including the `LatestExecutionAttempt` read model
- `promote_machine_attempt` (679-710)
- `list_execution_attempts` (710-732)
- `build_evidence_export` (1015-1107)

Tasks:

1. Create `services/evidence.py` with an `EvidenceService(ServiceBase)` following the `DocumentService` conventions.
2. Move the methods and the `LatestExecutionAttempt` dataclass verbatim. Preserve signatures, keyword-only arguments, error types, and `_session_scope` usage exactly.
3. Preserve every explicit `selectinload()` chain. **Chain, never varargs** - `selectinload(A.b).selectinload(B.c)` and `selectinload(A.b, B.c)` produce an identical `.path` but are not equivalent, and under the `lazy="raise"` default set in V4.6 the varargs form raises at render time. See `db/loading.py`.
4. Update `ServiceBundle` to construct and expose the new service via the `from_session_factory` constructor added in V4.6.
5. Update call sites in `sources_page.py` and `workflows.py`.
6. Run `ruff check --fix` **in the same pass** as the import edits - autofix removes imports that are unused at that moment.
7. **Revise `.github/instructions/services.instructions.md` to describe the boundaries this decomposition actually produced** (review log [59]). Do this *after* the move, not before - the refactor is the empirical test of the rule, and a rule written in advance would have to be bent to fit. Known defects to correct:
   - **Line 11, `1 service class per data model`** - the rule is table-shaped rather than aggregate-shaped, and is the measured cause of `sources.py` reaching 1,389 lines. Replace with aggregate ownership.
   - **No home for junction tables.** The rule names the four core components (Document, Source, Job, Person) but is silent on `job_source` and `document_person`, where they intersect. Add an explicit model-ownership table naming the owning service for every model, including junctions and `ExecutionAttempt`.
   - **Lines 30-32**, mandatory CRUD for every model, is already false: `prompts.py` does not comply. Soften to describe intent rather than mandate a method set.
   - **Line 13 vs lines 75-77** read as contradictory on whether a service may touch more than one table. Reword the composition section so the ownership rule and the multi-table-operation guidance agree.
   - **Line 77** typo: `picutre`.

Note that line numbers above are pre-Phase-1 positions and will have shifted. Locate by symbol, not by line.

Exit: suite green **with no test logic changes** beyond import paths, `ty check` at 0, and `services.instructions.md` consistent with the post-refactor module layout. Walk every `/ui/*` page and confirm a 200, since `lazy="raise"` turns a missed eager load into a runtime error rather than a slow query.

### 4. Run-Time Measurement Window (review log [55])

Tasks:

1. In `workflows.py`, make the recorded duration cover only the operation the `wait_for` at lines 240-249 governs. Either move `monotonic_started_at` (line 221) to immediately before the `wait_for`, or capture provider latency separately and record that.
2. Apply the same treatment to all three write sites: success (line 278), `TimeoutError` (line 295), and general failure (line 330). The failure paths must keep using the monotonic clock.
3. If preprocessing time is still worth keeping, record it as its own value rather than folding it into `duration_ms`.
4. Update `sources_page.py:400`, which renders the raw integer as `"27612 ms"`.

Phase 1 already removes normalization and the artifact write from this window, which narrows the gap but does not close it - the `session.commit()` at line 228 remains inside it.

Verification: a recorded timeout duration should sit at or just under the configured budget, not 0.4-2.0 s above it as in the three historical `local_timeout` rows.

### 5. Worker Exception Handling (review log [8])

`worker.py:96-106`.

Tasks:

1. Separate genuinely retriable faults from programming errors. `classify_unexpected_error` is already called at line 101 and its result is currently only logged.
2. Ensure a non-retriable error reaches a terminal state instead of being retried.
3. Keep terminal-state and retry persistence atomic per `services.instructions.md:63-65`.
4. Add a test that a deliberate programming error in the loop does not silently retry.

Context: with `WORKER_MAX_RETRIES=1` and a 30 s timeout the worst-case silent burn is 60 s, down from 360 s, so this is no longer urgent - but it remains the real fix behind that risk.

### 6. CI Enforcement ([HIGH-06], review log [40])

Tasks:

1. Add a workflow under `.github/workflows/` running `ruff check`, `ty check`, and `pytest` on push and pull request.
2. Use the same commands as `.pre-commit-config.yaml` so local and CI gates cannot drift.
3. Confirm the 4 tests that skip without `OPENROUTER_API_KEY` skip cleanly in CI rather than failing.
4. Negative-test the workflow by pushing a deliberate lint error on a scratch branch.

## Sequencing Constraints

- **Phase 1 before Phase 3.** Code scheduled for deletion is never extracted first. This is why the previously planned `services/artifacts.py` is cancelled.
- **Phase 1 before Phase 2.** Both touch `workflows.py` write paths; separating them keeps any regression attributable.
- **Phase 4 before any V4.8 telemetry work.** A model-performance rollup built on the current `duration_ms` would chart preprocessing mixed with provider latency.

## Test Strategy

- **Phase 1:** normalization tests move from asserting artifact creation to asserting ingest behavior. Add a test that a stored image never retains EXIF orientation 3, 6, or 8.
- **Phase 2:** assert that evidence reads resolve through `execution_attempt`; assert `CANCELLED` is distinguishable from `FAILED`; verify both status columns round-trip identically and existing rows read back correctly after the fix-up.
- **Phase 3:** the suite passes with **no test logic changes**. Only import paths update. A required behavioral change signals the move was not mechanical - stop and re-examine.
- **Phase 4:** assert the recorded duration is bounded by the configured timeout.
- **Phase 5:** new test that a programming error does not silently retry.
- **Phase 6:** CI must fail on an injected lint error.

## Risks

| Risk | Mitigation |
| :--- | :--- |
| The image backfill corrupts originals - it rewrites files in place with no archival master | Back up `data/documents/` before running; idempotent step keyed on the EXIF tag so a second run is a no-op; visually spot-check a backfilled page |
| Dropping `job_source` columns loses data that turns out not to be duplicated | Verified 77/77 identical on all three evidence columns before dropping; re-run that comparison inside the migration and abort on any mismatch |
| A read still expects a removed `job_source` column and fails only at render time | Grep-driven checklist in Phase 2 task 4; walk every `/ui/*` page after the phase |
| Cancelled pages are silently re-attempted, or silently never re-attempted | Phase 2 task 7 forces an explicit decision in the work-selection filter |
| A moved query loses an eager load and trips `lazy="raise"` at render time | Preserve `selectinload` chains verbatim; walk every `/ui/*` page after Phase 3 |
| `ruff check --fix` deletes an import mid-move | Edit imports and usages in the same pass, as in V4.6 |
| Circular imports between `sources.py` and `evidence.py` | Composition is sanctioned by `services.instructions.md:75-77`; keep the dependency one-directional |
| Scope creep into refactoring `update_job_source_transcription` | Out of scope; the boundary records why |

## Delivery Order

1. Ingest normalization and `ProcessingArtifact` removal
2. Evidence model simplification
3. Stage B - `evidence.py`, then revise `services.instructions.md` to match
4. Measurement window
5. Worker exception handling
6. CI enforcement

## Done Criteria

Every item in the scope boundary's Acceptance Criteria is satisfied, the suite is green, `ty check` reports 0 diagnostics, and no user-facing behavior has changed.

## Related Local References

- [V4.7 Scope Boundary](scope_boundary_v4_7.md)
- [Architecture & Code Review Report](../architecture_code_review_2026-08-17.md)
- [V4.6 Review Log](../ver4.6/review_log_v4_6.md) - resolves the `review log [N]` citations used throughout this document
- [V4.6 Implementation Plan](../ver4.6/implementation_plan_v4_6.md)
- [V4.8 Feature Backlog](../ver4.8/feature_backlog_v4_8.md)
- `.github/instructions/services.instructions.md`
- `src/transcription/db/loading.py` - the `selectinload` varargs trap