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 checkclean andty checkat 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_sourceevidence fields across 8 test files, and 10ProcessingArtifactreferences 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.dbanddata/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:
- Rotate the 58 stored images carrying EXIF orientation 3, in place, using qtables reuse; strip the orientation tag.
- Drop the
processing_artifacttable and remove its external artifact files. - Normalize
execution_attempt.statusto the single declared spelling ([45]). - 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:
- Move orientation normalization into the ingest path in
media_storage, ahead ofwrite_bytes(media_storage.py:57). Rotate, strip the EXIF orientation tag, then store. - Change the encode settings at
normalization.py:84-85fromquality=95, subsampling=0toqtables=im.quantization,subsampling=JpegImagePlugin.get_sampling(im),optimize=True. ImportJpegImagePluginexplicitly - it is not reachable as an attribute ofPIL.Image. - Delete
resolve_provider_inputand its call atworkflows.py:226. The stored file is now already upright, so the transcription path reads it directly. - Fold the
transcription_quality_warningspayload (workflows.py:560-571) intoexecution_attempt.normalized_metadata. - Delete the artifact cluster from
sources.py(lines 732-1015) and the artifact branch ofbuild_evidence_export. - Delete the
ProcessingArtifactmodel and theCheckConstraint. - 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. - Update
test_normalization.py,test_v42_evidence.py, andtest_db.py. Normalization tests should now assert on ingest behavior rather than on artifact creation. - Create
tools/migrate_v46_to_v47.pywith 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:
- Add
JobSourceStatus.CANCELLED. Updatejobs.py:378-384to write it instead ofFAILEDplus"Cancelled by user". - Declare one spelling for
JobSourceStatusacross bothjob_source.statusandexecution_attempt.status([45]).job_source.statusalready declaresvalues_callable;execution_attempt.statusdoes not. - Remove
raw_transcription,ai_metadata,raw_api_response,executed_at, anderror_detailfrom theJobSourcemodel. - Redirect every read to
execution_attempt:transcript.py:103-119sorts byexecuted_at- sort byExecutionAttempt.finished_at.sources_page.py:390-393readsai_metadataandraw_api_response.sources_page.py:337-375renders status, executed time, and error detail.models.py:266-278andmodels.py:328-343derive transcript and error fromjob_sources.
- Shrink
update_job_source_transcription(sources.py:524-679) to write only the survivingJobSourcecolumns. Keep the method insources.pyand keep both writes in one session scope. - Confirm
jobs.py:411-424retry still works unchanged. It resetsFAILEDtoPENDING; the attempt history it appears to discard is preserved byExecutionAttempt's unique constraint. - Confirm
workflows.py:432-442work selection is unaffected. It filtersstatus != TRANSCRIBEDwithinjob.job_sources, soCANCELLEDpages 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. - Update the 33 affected test references across the 8 files identified.
- 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 theLatestExecutionAttemptread modelpromote_machine_attempt(679-710)list_execution_attempts(710-732)build_evidence_export(1015-1107)
Tasks:
- Create
services/evidence.pywith anEvidenceService(ServiceBase)following theDocumentServiceconventions. - Move the methods and the
LatestExecutionAttemptdataclass verbatim. Preserve signatures, keyword-only arguments, error types, and_session_scopeusage exactly. - Preserve every explicit
selectinload()chain. Chain, never varargs -selectinload(A.b).selectinload(B.c)andselectinload(A.b, B.c)produce an identical.pathbut are not equivalent, and under thelazy="raise"default set in V4.6 the varargs form raises at render time. Seedb/loading.py. - Update
ServiceBundleto construct and expose the new service via thefrom_session_factoryconstructor added in V4.6. - Update call sites in
sources_page.pyandworkflows.py. - Run
ruff check --fixin the same pass as the import edits - autofix removes imports that are unused at that moment. - Revise
.github/instructions/services.instructions.mdto 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 ofsources.pyreaching 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_sourceanddocument_person, where they intersect. Add an explicit model-ownership table naming the owning service for every model, including junctions andExecutionAttempt. - Lines 30-32, mandatory CRUD for every model, is already false:
prompts.pydoes 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.
- Line 11,
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:
- In
workflows.py, make the recorded duration cover only the operation thewait_forat lines 240-249 governs. Either movemonotonic_started_at(line 221) to immediately before thewait_for, or capture provider latency separately and record that. - 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. - If preprocessing time is still worth keeping, record it as its own value rather than folding it into
duration_ms. - 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:
- Separate genuinely retriable faults from programming errors.
classify_unexpected_erroris already called at line 101 and its result is currently only logged. - Ensure a non-retriable error reaches a terminal state instead of being retried.
- Keep terminal-state and retry persistence atomic per
services.instructions.md:63-65. - 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:
- Add a workflow under
.github/workflows/runningruff check,ty check, andpyteston push and pull request. - Use the same commands as
.pre-commit-config.yamlso local and CI gates cannot drift. - Confirm the 4 tests that skip without
OPENROUTER_API_KEYskip cleanly in CI rather than failing. - 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.pyis cancelled. - Phase 1 before Phase 2. Both touch
workflows.pywrite paths; separating them keeps any regression attributable. - Phase 4 before any V4.8 telemetry work. A model-performance rollup built on the current
duration_mswould 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; assertCANCELLEDis distinguishable fromFAILED; 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
- Ingest normalization and
ProcessingArtifactremoval - Evidence model simplification
- Stage B -
evidence.py, then reviseservices.instructions.mdto match - Measurement window
- Worker exception handling
- 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
- Architecture & Code Review Report
- V4.6 Review Log - resolves the
review log [N]citations used throughout this document - V4.6 Implementation Plan
- V4.8 Feature Backlog
.github/instructions/services.instructions.mdsrc/transcription/db/loading.py- theselectinloadvarargs trap