Files
transcription/docs/ver4.7/implementation_plan_v4_7.md
T
2026-08-18 09:24:41 -05:00

16 KiB

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. Feature work is parked in ../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.