Phase 6: enforce the quality gate in CI and export the V4.7 review log

Adds .github/workflows/quality-gate.yml, running the gate on push and pull
request. CI invokes `pre-commit run --all-files` rather than restating the
`ruff check` and `ty check` commands, so the checks keep a single definition
in .pre-commit-config.yaml and local and CI cannot drift (plan task 2).

The workflow writes a .env file rather than exporting an environment
variable. The two are not equivalent here: Settings reads the .env file,
while the external-test skip guard reads os.getenv, so an exported variable
un-skips the external tests and sends them to the network. Measured in CI:
no .env gave 115 failures and 18 errors, an exported dummy key gave 3
failures, and a written .env file reproduced the local baseline exactly.

Negative-tested on a scratch branch: a deliberate lint error failed the run
at `ruff check` with exactly the planted errors, confirming the gate blocks
rather than merely reporting (plan task 4). The subsequent clean run passed
ruff and ty and reported 295 passed, 4 skipped, matching local and
confirming the four credential-gated tests skip cleanly (plan task 3).

That first green run caught a real platform-dependent defect. PromptStore
rejected non-direct-child names via `Path(name).name != name`, which is
platform-dependent: on POSIX a backslash is an ordinary filename character,
so "nested\prompt.md" passed the guard and failed later as NOT_FOUND rather
than VALIDATION. Windows cannot reproduce it. No traversal was possible,
since the path.parent != root check still held, so the impact was a wrong
error category and a red gate. Both separators are now rejected explicitly,
matching the ^[^/\\]+$ pattern config.PromptFilename already used.

Also exports docs/ver4.7/review_log_v4_7.md, the working record kept across
all six phases: 50 entries, 1 still open. The open entry is a pre-existing
/ui redirect defect found during the Phase 3 UI walk and deliberately left
unfixed as outside the V4.7 scope boundary.

Co-authored-by: Copilot App <[email protected]>

.github/workflows/quality-gate.yml

@@ -0,0 +1,45 @@
+name: Quality Gate
+
+# V4.7 Phase 6 / review log [40]. Before this, ruff, ty and pytest were enforced
+# only by .pre-commit-config.yaml, and only for developers who had actually run
+# `pre-commit install`.
+
+on:
+  push:
+  pull_request:
+
+jobs:
+  gate:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out the commit under test
+        uses: actions/checkout@v4
+
+      - name: Install uv
+        run: |
+          curl -LsSf https://astral.sh/uv/install.sh | sh
+          echo "$HOME/.local/bin" >> "$GITHUB_PATH"
+
+      - name: Install dependencies from the lockfile
+        # --locked fails if uv.lock has drifted from pyproject.toml, so a stale
+        # lockfile is caught here rather than producing an untested dependency set.
+        run: uv sync --locked
+
+      - name: Write placeholder configuration
+        # Settings requires openrouter_api_key and 115 tests cannot construct
+        # Settings without it. This is written to a .env file rather than exported
+        # as an environment variable on purpose: the external tests guard on
+        # os.getenv("OPENROUTER_API_KEY"), which reads the process environment and
+        # not the file, so writing the file reproduces the local result exactly -
+        # the 4 external tests skip instead of running against a fake key and
+        # failing. Exporting it instead produces 3 failures.
+        run: echo "OPENROUTER_API_KEY=ci-placeholder-not-a-real-key" > .env
+
+      - name: Lint and type check
+        # Runs the hooks defined in .pre-commit-config.yaml instead of repeating
+        # "ruff check" and "ty check" here. The commands then have one definition,
+        # so the local and CI gates cannot drift apart.
+        run: uv run pre-commit run --all-files --show-diff-on-failure
+
+      - name: Tests
+        run: uv run pytest

docs/ver4.7/review_log_v4_7.md

@@ -0,0 +1,415 @@
+# V4.7 Implementation Review Log
+
+Working record kept during the V4.7 architectural cleanup release.
+
+This file is the canonical reference for citations of the form **`review log [N]`** in V4.7 and later planning documents. The numbers below are those `N` values. They are independent of the [V4.6 log](../ver4.6/review_log_v4_6.md), which has its own numbering.
+
+The log was maintained live in a session-scoped database and exported here so the citations remain resolvable in later sessions. It is a historical record: entries are not rewritten after the fact, except where a later phase resolved an entry that was open at the time, in which case the resolution is appended to the body and marked `RESOLVED:`. Where an entry conflicts with a committed planning document, **the planning document wins**.
+
+## Legend
+
+| Field | Meaning |
+| :--- | :--- |
+| `kind` | `question` - needed a decision; `comment` - observation; `deviation` - departure from plan; `risk` - identified hazard |
+| `status` | `open` - unresolved; `answered` - resolved by a decision; `noted` - recorded, no action required |
+| `finding` | Finding ID in [architecture_code_review_2026-08-17.md](../architecture_code_review_2026-08-17.md), where one applies. Most V4.7 entries have none, because V4.7 works from the [implementation plan](implementation_plan_v4_7.md) rather than from that pre-V4.6 snapshot. Where an entry carries a one-line summary instead, it appears as a bold lead-in to the body. |
+
+**50 entries** - 1 open, 18 answered, 31 noted.
+
+## Still Open
+
+These carry forward past V4.7.
+
+| ID | Finding | Summary | Disposition |
+| :--- | :--- | :--- | :--- |
+| [32] | - | /ui/documents/{id}/sources redirects to /sources, dropping the /ui prefix | Pre-existing and outside the V4.7 scope boundary - deliberately left unfixed |
+
+## Full Log
+
+### Phase 0 - baseline and backups
+
+#### [1] Backups taken and verified
+
+*comment* - **noted**
+
+data/transcription.db and data/documents/ copied to C:\GitHub\_backups\transcription_v47_20260818-092616. All 76 document files SHA256-identical to source. A consistent SQLite snapshot (transcription.consistent.db) was also produced via the sqlite3 backup API because the live DB file is locked by a running app process, making a plain file copy potentially torn.
+
+#### [2] The application appears to be running and holds data/transcription.db
+
+*risk* - **answered**
+
+Two python processes started 2026-08-18 04:58 (PID 14340 is .venv python). Get-FileHash on data/transcription.db failed with a sharing violation. The Phase 1 migration rewrites data/documents/ JPEGs in place and later phases ALTER the live schema; both are unsafe while the app is running. Need the user to stop the app before any migration run. RESOLVED: the user stopped the app before the migration ran; the DB accepted an immediate write lock beforehand.
+
+#### [3] Working tree is clean; the V4.7 doc edits are already committed
+
+*deviation* - **noted**
+
+The brief said both V4.7 docs have uncommitted edits on disk. git status --short is empty and HEAD is 246d7f9 "V4.7 final scope changes", which contains them. Nothing was reverted or stashed; the working tree content matches what the brief described.
+
+#### [4] Baseline is not fully green: 1 pre-existing test failure caused by .env leakage
+
+*risk* - **answered**
+
+tests/test_config.py::TestWorkerReliabilitySettings::test_worker_retry_defaults asserts worker_max_retries == 0 but the local .env sets WORKER_MAX_RETRIES=1, and _make_settings() reads .env. ruff and ty are both clean. This is a test-isolation defect predating V4.7 (introduced when the 2026-08-18 calibration wrote WORKER_MAX_RETRIES into .env), not a code defect. It would pass in CI, where no .env exists, which makes it a latent local/CI divergence relevant to Phase 6. Awaiting a decision on whether to fix it. RESOLVED: tests/test_config.py::_make_settings now builds Settings with _env_file=None, isolating it from the local .env. The local/CI divergence this entry flagged is gone, and CI run 182 confirms the same 295 passed / 4 skipped result on Linux with no .env of the developer's.
+
+### Phase 1 - artifact removal and ingest rotation
+
+#### [5] Where does ingest-time rotation actually belong: media_storage or store.py?
+
+*question* - **answered**
+
+Plan Phase 1 task 1 says rotate inside media_storage ahead of write_bytes. But store.py computes file_hash and file_size_bytes from the ORIGINAL file_bytes (store.py:93, 168) independently of write_media_bytes, which returns only a Path. Rotating inside write_media_bytes would leave Source.file_hash and file_size_bytes describing bytes that were never stored. write_media_bytes is also shared with person portraits and homepage images. Recommend rotating the bytes once at the Source-ingest boundary (store_source_file or its two callers in store.py) so hash, size and stored file all describe the same upright bytes, and leaving media_storage a generic byte writer. [ANSWERED 2026-08-18 by user] Rotate at the Source-ingest boundary, before file_hash/file_size_bytes are computed, so the hash and size describe the stored upright bytes. media_storage stays a generic byte writer. This is a deliberate deviation from Phase 1 task 1 as written.
+
+#### [6] Fate of artifact_dir / artifact_inline_threshold_bytes settings and data/artifacts/
+
+*question* - **answered**
+
+Removing ProcessingArtifact orphans Settings.artifact_dir and Settings.artifact_inline_threshold_bytes (config.py:104,106), the JobService artifact deletion path (jobs.py:307-350), and the on-disk data/artifacts/ tree. config.py is outside the services instruction file. Proposal: delete both settings and the jobs.py deletion path as part of the same removal, and have the migration delete the external artifact files (migration step 2 already says so). Confirm. [ANSWERED 2026-08-18 by user] Delete both Settings.artifact_dir and Settings.artifact_inline_threshold_bytes.
+
+#### [7] Phase 1 blast radius is wider than the plan task list
+
+*comment* - **noted**
+
+Beyond the listed sites, ProcessingArtifact is also referenced by: sources.py delete guards (lines 296-300 and 450-462, which block Source deletion when artifacts exist), jobs.py job-deletion artifact cleanup (307-350), Source.processing_artifacts and ExecutionAttempt.artifacts relationships (models.py:322,431), the selectinload(ExecutionAttempt.artifacts) in list_execution_attempts (sources.py:719), and the ProcessingArtifact lookup that validates model_input_artifact_id inside update_job_source_transcription (sources.py:648-664). All must go with the model.
+
+#### [10] Quality-warnings render: delete (per plan) or re-point at normalized_metadata?
+
+*question* - **answered**
+
+Plan Phase 1 task 7 says delete the render at sources_page.py:661, but task 4 folds the same payload into execution_attempt.normalized_metadata and decision A forbids user-facing change. The render has never fired in practice because it reads attempt.artifacts and only 2 artifact rows exist. [ANSWERED 2026-08-18 by user] Keep the display and re-point it at normalized_metadata. Deviation from Phase 1 task 7 as written; task 4 now has a consumer.
+
+#### [11] SourceEvidenceReference.derivative_id / transformation kept but no longer populated
+
+*deviation* - **noted**
+
+With ProcessingArtifact gone there is no derivative to reference, so both fields are always None. They were left in place rather than removed: RequestManifest is a frozen, versioned evidence contract whose canonical bytes feed request_manifest_sha256, so removing fields would change the digest of every future manifest and arguably require a schema_version bump - cost out of proportion to deleting two optional fields. Raise if you would rather see the contract cleaned up.
+
+#### [12] The session.commit() at workflows.py:228 is removed with resolve_provider_input
+
+*deviation* - **noted**
+
+That commit existed to make the artifact row written during provider-input resolution durable before the provider call. With no artifact write there is nothing pending to commit - the PROCESSING claim was already committed at line 198 / 426 - so the call is removed rather than left as a no-op. This also removes one of the two things Phase 4 has to get out of the duration measurement window.
+
+#### [13] The image backfill must also update source.file_hash and file_size_bytes
+
+*risk* - **answered**
+
+Migration step 1 as written only rotates the stored JPEGs and strips the EXIF tag. But source.file_hash and source.file_size_bytes were computed from the pre-rotation bytes, and after Phase 1 the transcription path derives the evidence digest (SourceEvidenceReference.digest_sha256) straight from source.file_hash. Rotating the file without updating the row would make every backfilled Source advertise a digest that does not match the bytes actually sent to the provider - the exact class of defect the evidence model exists to prevent. The migration therefore rewrites both columns for each rotated image in the same transaction. Not a change of intent, an omission in the step description.
+
+#### [14] Orientation normalization must not change what ingest accepts
+
+*deviation* - **noted**
+
+**Undecodable upload bytes are a normalization no-op, not a rejection**
+
+validate_source_content only checks emptiness and filename; it never decoded the image, so bytes that Pillow cannot open (e.g. the b"image-bytes" fixture in tests/services/test_store.py) were accepted and stored. Moving rotation into store_source_file initially turned that into an OrientationNormalizationError, i.e. a user-facing rejection of previously accepted uploads. Decision A forbids user-facing change, so Image.open failure now logs and returns None; the error is retained only for a decode that succeeded and a rewrite that then failed.
+
+#### [15] Artifact-only tests removed with the subsystem
+
+*deviation* - **noted**
+
+**Two tests deleted rather than rewritten**
+
+tests/test_v42_evidence.py::test_large_json_artifact_uses_constrained_atomic_storage and ::test_rejects_inline_artifact_with_incorrect_integrity exercised only external artifact storage and inline artifact integrity. Both behaviours are deleted by Phase 1, so the tests have no surviving subject. tests/ui/test_sources_page.py lost one assertion ("Derived Artifacts"), and tests/test_db.py lost the processing_artifact table assertion.
+
+#### [16] Fate of the superseded v4.5 to v4.6 migration tool
+
+*question* - **answered**
+
+**tools/migrate_v45_to_v46.py no longer type-checks**
+
+The old migration references ProcessingArtifact (line 157), SourceService._verify_artifacts_integrity (line 169), and carries processing_artifact: 2 in EXPECTED_SOURCE_COUNTS (line 83). All three are gone. It is currently the only remaining ty failure. The plan says to keep its enum-spelling tolerance but does not address this. Options: delete the completed one-time tool; or strip the artifact code path from it. RESOLVED: the completed one-time tool was deleted (Phase 1). tools/ now contains only migrate_v46_to_v47.py, and ty is clean.
+
+#### [17] tools/migrate_v45_to_v46.py removed rather than repaired
+
+*question* - **answered**
+
+**Superseded v4.5 to v4.6 migration deleted**
+
+User decision. The migration is complete, the live database is already V4.6, and after V4.7 it would restore a V4.5 backup into a schema that no longer matches (job_source is stripped in Phase 2). Two doc references remain, both citing it only as a conventions template: implementation_plan_v4_7.md lines 39 and 50, scope_boundary_v4_7.md line 160. Line 50 (enum-spelling tolerance) is now moot. Recoverable from git history if ever needed.
+
+#### [18] DEFAULT_ARTIFACT_DIR constant in migrate_v46_to_v47.py
+
+*deviation* - **noted**
+
+**Migration records the deleted artifact_dir default itself**
+
+Step 2 must delete external artifact files, but Settings.artifact_dir was deleted in the same phase. The migration therefore carries the historical V4.6 default (data/artifacts) as its own constant with a --artifact-dir override, rather than depending on a setting that no longer exists. One external file was present and removed; the directory is now empty.
+
+#### [19] Phase 1 migration outcome
+
+*risk* - **answered**
+
+**Migration executed and verified against the live corpus**
+
+Ran after the user stopped the app and after a fresh pre-migration backup to C:\GitHub\_backups\transcription_v47_premigration_20260818-101232. Result: 58 images rotated, 18 already upright, 0 missing; processing_artifact dropped (2 rows) and its 1 external file removed. Verification: no stored image reports orientation 3/6/8; source.file_hash and file_size_bytes match every file on disk (0 mismatches over 76); 58 of 76 files differ from the backup; PSNR against the un-rotated backup is 50.3 / 51.1 / 56.1 dB (min/median/max) across the 57 JPEGs, allowing for the -6 percent size reduction; a re-run reports rotated=0 and table already absent, confirming idempotency. Visual spot-check of 1547e555 confirmed the page was genuinely stored upside down and is now upright.
+
+### Phase 2 - evidence model simplification
+
+#### [8] Should CANCELLED pages be re-attempted when a job is re-run?
+
+*question* - **answered**
+
+Plan Phase 2 task 7. _resolve_job_sources (workflows.py:432-442) selects work by status != TRANSCRIBED, so once CANCELLED exists as a distinct status a re-run would silently pick cancelled pages back up. Options: (a) exclude CANCELLED from work selection, so cancelling is sticky and a page must be explicitly re-queued; (b) include it, so re-running a job means "do everything not yet transcribed"; (c) clear CANCELLED back to PENDING in the existing retry path (jobs.py:411-424) and exclude it from work selection, which makes re-attempt an explicit user action through the retry button. Decision required before Phase 2 task 1. RESOLVED: re-attempt them. Resubmit accepts FAILED and CANCELLED. Rationale: today cancel writes FAILED, so resubmit already resets cancelled pages to PENDING; introducing a distinct CANCELLED status without widening the resubmit filter would silently make cancelled work unrecoverable, a user-facing regression that decision A forbids. The decision is encoded in the resubmit candidate filter, which is the real decision point - _resolve_job_sources only ever sees these rows after resubmit has already set PENDING. UI copy on both the cancel and resubmit pages is updated to match.
+
+#### [20] Plan task 4 targets a dead module
+
+*question* - **answered**
+
+**ui/components/transcript.py deleted instead of redirected**
+
+Phase 2 task 4 directs transcript.py:103-119 to sort by ExecutionAttempt.finished_at instead of job_source.executed_at. Investigation showed the module is entirely unreferenced: no import of transcription.ui.components.transcript exists in src, tests, or docs, and both public functions (render_original_transcription_card, render_revision_row) have zero callers. Rewriting it would mean maintaining unreachable code against the new evidence model. User decision: delete the module. Recoverable from git history.
+
+#### [21] Defect [45] fixed by declaring one enum spelling
+
+*deviation* - **noted**
+
+**execution_attempt.status gains values_callable**
+
+ExecutionAttempt.status was a bare JobSourceStatus annotation, so SQLAlchemy persisted enum names (TRANSCRIBED) while job_source.status persisted values (transcribed) via values_callable. That is why the two columns matched on 0 of 79 rows. execution_attempt.status now declares the identical SAEnum with values_callable and native_enum=False. Existing rows carry the old spelling and are rewritten by migration step 3.
+
+#### [22] Dead property made more expensive by the evidence move
+
+*question* - **answered**
+
+**Job.error_detail deleted rather than re-derived**
+
+Plan task 4 lists models.py:266-278 (Job.error_detail) for redirection. A full-repo search found zero readers: JobTableRow has no such field and the job detail page never calls it. Re-deriving it from ExecutionAttempt would require a two-level eager load (job_sources -> execution_attempts) on every Job, across a lazy=raise then lazy=noload chain, where a missing load returns an empty list and the property would silently answer None instead of raising. No information is lost: error_detail survives on ExecutionAttempt and is reachable via list_execution_attempts and read_latest_execution_attempt. A future job-level failure view should query attempts directly anyway, since first-error-across-pages is the wrong shape for a partial-success job. User decision: delete.
+
+#### [23] Replacement ordering key after executed_at is dropped
+
+*question* - **answered**
+
+**Source.latest_job_source orders by Job.date_created**
+
+JobSource retains only id, job_id, source_id and status, so max(job_sources, key=executed_at) needs a key from a neighbour. Job.date_created is chosen over the latest ExecutionAttempt.finished_at: it is always present (a PENDING page has no attempt at all), it is already eager-loaded by read_source_detail, and since (job_id, source_id) is unique per source the ordering is exactly most recent job. The two differ only when a job created earlier finishes later, which the single-worker queue does not produce. User decision.
+
+#### [24] The "Cancelled by user" string has no home after job_source is stripped
+
+*deviation* - **noted**
+
+**Cancel no longer records a reason string**
+
+cancel_job previously wrote error_detail="Cancelled by user" onto job_source. That column is gone, and cancel deliberately makes no provider call so it writes no ExecutionAttempt. The reason is now carried by JobSourceStatus.CANCELLED itself, which is strictly more precise than a free-text string. UI copy on the cancel page was updated to say "cancelled" and to state that cancelled sources can be resubmitted.
+
+#### [25] jobs_page "Failed Sources" became "Resubmittable Sources"
+
+*deviation* - **noted**
+
+**Resubmit UI counter renamed**
+
+The resubmit candidate filter now accepts FAILED and CANCELLED per the user decision in entry 8, so the page counter had to count both. Renamed the metadata row and the blocked-error message accordingly.
+
+#### [26] sources_page no longer renders ai_metadata/raw_api_response when no attempt exists
+
+*comment* - **noted**
+
+**Legacy job_source evidence fallback deleted from the detail page**
+
+The "no ExecutionAttempt" branch of _render_provider_evidence used to fall back to the job_source JSON columns for historical rows. Those columns are gone, so the branch now renders only the empty state. Verified against the evidence baseline: all 77 successful transcriptions have a matching execution_attempt row, so no live row loses its evidence display.
+
+#### [27] latest_error_detail reads through job_sources -> execution_attempts
+
+*risk* - **noted**
+
+**Model properties now require a two-level eager load**
+
+Source.latest_error_detail feeds a visible "Error Detail" column on the sources table. Because JobSource.execution_attempts is lazy="noload" it returns empty rather than raising when not loaded, so a caller that forgets the chained selectinload gets a silent blank instead of an error. list_sources_detail and the model-property test were both updated to chain selectinload(...).selectinload(orm_attribute(...)). Any new caller must do the same.
+
+#### [28] job_source.status and execution_attempt.status now agree on every row
+
+*comment* - **noted**
+
+**Defect [45] verified fixed against the live database**
+
+Before: 0/79 rows matched, because execution_attempt persisted enum names and job_source persisted values. After migration step 3: 79/80 join rows agree. The single disagreement is job_source 09cd5f77 which has two attempts - attempt 1 failed, attempt 2 transcribed - so the queue row correctly reflects the final outcome while the history preserves the failure. Comparing job_source against its LATEST attempt gives 79/79.
+
+#### [29] list_sources_detail resolves latest_status and latest_error_detail for all 76 rows
+
+*comment* - **noted**
+
+**Two-level eager load verified against live data, not just tests**
+
+Ran SourceService.list_sources_detail against the migrated production database: 76 sources, 75 transcribed / 1 failed, and the one failed row still exposes latest_error_detail - now read from execution_attempt rather than the dropped job_source column. This closes the silent-blank risk recorded in entry 27 for the shipped call path.
+
+### Phase 3 - evidence service extraction and the ownership rule
+
+#### [9] Junction ownership: which service owns job_source and document_person?
+
+*question* - `MED-14` - **answered**
+
+services.instructions.md names four core components (Document, Source, Job, Person) and is silent on the two junctions, which is exactly where two owners intersect. Candidate tie-break rules: (a) the junction belongs to the service that creates its rows; (b) it belongs to the aggregate whose lifecycle it shares (job_source dies with the Job, document_person dies with the Document); (c) it belongs to the side that reads it most. These do not agree for job_source: it is created by store.py orchestration, its lifecycle is the Job, and it is read predominantly through Source pages, which is how it ended up in sources.py. Decision required at Phase 3 task 7. RESOLVED: measurement showed document_person has a single writer (people.py, every create/delete/sync) and needs no tie-break; documents.py only eager-loads through it. job_source is genuinely contested between sources.py (row existence + per-page outcome) and jobs.py (job-lifecycle status transitions). User selected the LIFECYCLE rule: the service that creates and deletes rows owns the junction, so job_source -> SourceService. Two scoped carve-outs written into the rule: (1) cascade deletion of junction rows when a service deletes its own aggregate root (JobService.delete_job_with_guardrails); (2) status transitions that create and delete nothing (cancel_job, resubmit_failed_sources), because those are Job lifecycle events. No code was moved.
+
+#### [30] Where should the shared transcription error hierarchy live?
+
+*question* - **answered**
+
+**Extraction immediately violated the existing no-sibling-import rule**
+
+tests/test_service_boundaries.py enforces services.instructions.md:13 - a service module must not import a sibling. evidence.py needed TranscriptionNotFoundError, which sources.py also raises, so the extraction failed the rule on the first run. Measured ownership: CandidatePromotionError is now raised only in evidence.py; PromptLoadError and SourceDeleteBlockedError only in sources.py; TranscriptionNotFoundError in both; TranscriptionError is the shared base, caught by store.py. User chose to move the whole five-class hierarchy to a neutral services/errors.py: one obvious home, one import path, and the exception a caller catches no longer changes when an operation moves between services.
+
+#### [31] Two test bundles broke on adding a fifth service, not on the refactor itself
+
+*comment* - **noted**
+
+**ServiceBundle default factories silently bind to the real database**
+
+test_v45_candidates and test_workflows_reliability constructed ServiceBundle(...) field by field. Adding the evidence field meant it fell back to field(default_factory=EvidenceService), which resolves the process-global session factory rather than the test one - so the tests silently queried the wrong database instead of failing loudly. Both were changed to ServiceBundle.from_session_factory(...), which is immune to future additions. This is the same global-singleton hazard recorded in the 2026-08-17 review at line 272.
+
+#### [32] /ui/documents/{id}/sources redirects to /sources, dropping the /ui prefix
+
+*risk* - **open**
+
+**Pre-existing broken redirect found during the UI walk**
+
+The Phase 3 exit criterion requires walking every /ui/* page. 24 of 25 routes return 200. documents_page.py returns RedirectResponse(url=f"/sources?document_id=...") without the /ui mount prefix, so following the 307 lands on a 404. Confirmed pre-existing: documents_page.py has no uncommitted diff and was last touched in 6a3ee26, well before V4.7. Out of the V4.7 scope boundary, so NOT fixed - raised for the user to decide.
+
+#### [33] Instruction-file defects corrected
+
+*comment* - **noted**
+
+**services.instructions.md rewritten after the decomposition, per the mandated order**
+
+All five defects from plan Phase 3 task 7 fixed. (a) Line 11 "1 service class per data model" replaced with one service class per AGGREGATE, with DocumentType-under-DocumentService as the worked example; this is the measured cause of sources.py reaching 1,389 lines. (b) Added a Model Ownership section with a table covering every model plus an explicit junction-table rule, which the file previously had no home for. (c) The mandatory-CRUD rule (old lines 30-32) was already false: prompts.py, quality.py, normalization.py, media_storage.py and source_media.py define no service class at all, EvidenceService deliberately exposes no create/delete because ExecutionAttempt is append-only, and RegistryService uses generic <op>_entry naming. Softened to intent plus an explicit "do not add unused CRUD to satisfy symmetry". (d) Old line 13 (services fully independent) read as contradicting old lines 75-77 (compose across tables); reworded to separate READING across models via eager loads from the owning root, which is allowed, from IMPORTING another service, which is not. (e) Typo "picutre" removed. Also recorded the real enforcement mechanism: tests/test_service_boundaries.py, and errors.py as the neutral shared-type home.
+
+#### [34] Line-number citation removed from the boundary test
+
+*risk* - **noted**
+
+**test_service_boundaries.py cited the rule by line number**
+
+The test docstring pinned .github/instructions/services.instructions.md:13. Rewriting the file invalidated that anchor. Replaced with a section-name citation ("Structure") so future edits to the instruction file cannot silently desynchronise the test docstring. errors.py was also added to the docstring list of neutral modules.
+
+### Phase 4 - measurement window
+
+#### [35] Both cited offenders were already deleted
+
+*deviation* - **noted**
+
+**Phase 4 premise partly overtaken by Phase 1**
+
+The plan states the session.commit() at line 228 "remains inside" the measurement window. Diffed against f86c0ff~1: at V4.6 the window held resolve_provider_input (async; normalization + artifact write + DB work) and that commit. Phase 1 deleted both. What remains between the clock and the wait_for is build_provider_input, now pure field copying because normalization moved to ingest and file_hash is already stored. Measured at 6.2 us per call with zero awaits, so it cannot yield to the event loop. Plan tasks 1-2 were therefore already satisfied in substance; the clock was still moved to make the property structural rather than incidental.
+
+#### [36] No preprocessing left to record separately
+
+*deviation* - **noted**
+
+**Plan task 3 declined**
+
+Task 3 offered recording preprocessing time as its own value. After Phase 1 there is no preprocessing in the window: 6.2 us of attribute copying. Adding a preprocessing_ms column to measure that is unnecessary complexity and was declined under the guiding principle. Raised rather than decided silently.
+
+#### [37] Undocumented 475ms contributor the plan did not identify
+
+*risk* - **answered**
+
+**Lazy provider construction was inside the timed region**
+
+The regression test measured 890ms where ~200ms was expected. Cause: services.sources.provider is a lazy property, and it appears as an argument expression to _call_transcriber, so it is evaluated after the clock starts but before wait_for begins timing. Measured 475ms to construct OpenRouterTranscriptionProvider on first access and 0.001ms after. The first attempt of every worker process therefore booked ~0.5s of HTTP client construction as provider latency. This plausibly accounts for the low end of the historical 0.4-2.0s local_timeout overshoot, and Phase 1 did not touch it. The property is loop-invariant, so it was hoisted above the per-source loop, which also removes the repeated attribute lookup from the two evidence-capture sites.
+
+#### [38] test_timeout_duration_excludes_pre_call_setup
+
+*comment* - **noted**
+
+**Regression guard added**
+
+New test in tests/services/test_workflows_reliability.py simulates 400ms of blocking setup against a 200ms provider budget and asserts the recorded duration_ms sits near the budget and well clear of budget+setup. Verified to fail on the pre-fix code (625 < 540 assertion error) and pass after, so it is a real guard rather than a tautology. This is the plan Phase 4 verification criterion expressed as a test.
+
+#### [39] sources_page.py no longer prints raw milliseconds
+
+*comment* - **noted**
+
+**Duration render scaled**
+
+Plan task 4. _format_duration renders >=1s as "27.6 s" and below that as "612 ms", per user selection. No test asserted the old format.
+
+### Phase 5 - worker fault containment
+
+#### [40] Probed behaviour: the defect is a stranded job, not a silent retry
+
+*deviation* - **noted**
+
+**Plan task 4 describes a failure mode that does not occur**
+
+The plan asks for a test that a deliberate programming error "does not silently retry". Probed empirically with an injected AttributeError. Mode A, error raised after the claim commits (inside advance_job): raised exactly ONCE, job left at PROCESSING, retry_count 0, and never re-claimed because claim_next_queued_job filters status == QUEUED. That is a permanently stranded job with one swallowed log line, not a retry. advance_job PROCESSING branch, commented "Recover mid-flight jobs", is unreachable from the worker for the same reason. Mode B, error raised before or during the claim: 20 raises in 1.2s, an unbounded hot spin at the poll interval. The plan context says worst-case silent burn is 60s under WORKER_MAX_RETRIES=1, but Mode B never reaches the per-job retry machinery so nothing caps it. Both modes share the root cause the plan correctly identifies.
+
+#### [41] Flag set in 9 places, read in none
+
+*comment* - **noted**
+
+**retriable was decorative**
+
+Measured across src/: retriable is assigned at errors.py:40/47/79, sources.py:877/884, store.py:127/205/366, workflows.py:284/580/593/606 and read nowhere. classify_unexpected_error already returns retriable=False, so the classification existed and was discarded. Phase 5 makes it load-bearing in two places.
+
+#### [42] User chose: stop the worker loop
+
+*question* - **answered**
+
+**Loop policy for a non-retriable error with no job to mark**
+
+Mode B has no claimed job, so there is no row to mark FAILED and no reason to expect the next poll to differ. Options offered were stop the loop, circuit-breaker after N consecutive failures, or exponential backoff. User selected stopping the loop, logged at CRITICAL, returning cleanly so the exception does not surface only at app shutdown via worker_consumer_lifespan wait_for.
+
+#### [43] User chose: mark FAILED and keep going
+
+*question* - **answered**
+
+**Loop policy for a non-retriable error where the job CAN be marked failed**
+
+Distinct from entry 42 and not covered by it. Mode A can contain the failure on the job row, so stopping the loop would let one poison job halt transcription for every other job. User selected containment: mark the job FAILED, which is visible in the UI and resubmittable, and continue polling.
+
+#### [44] Containment write uses its own transaction
+
+*risk* - **noted**
+
+**Terminal write runs on a possibly dirty session**
+
+_advance_job_with_containment rolls back the caller session before marking the job FAILED, and calls update_job_state with no session so the service owns and commits its own transaction. This satisfies plan task 3 atomicity: the terminal write cannot be left half-applied by whatever failure poisoned the caller session.
+
+#### [45] test_run_worker_loop_survives_process_next_exception replaced
+
+*deviation* - **noted**
+
+**An existing test encoded the defective behaviour**
+
+That test asserted the loop SURVIVES a RuntimeError and continues, which is exactly the Mode B defect. It was replaced by test_run_worker_loop_stops_on_non_retriable_exception, plus a new test_run_worker_loop_survives_retriable_exception so suppression of genuinely transient faults stays covered. Unlike Phase 3, changing test logic here is the point of the phase. Both new guards plus the Mode A guard were verified to FAIL on pre-fix code: the Mode B test times out, which is the infinite spin made visible.
+
+### Phase 6 - CI enforcement
+
+#### [46] Remote is Gitea 1.27.2, not GitHub
+
+*comment* - **noted**
+
+**The plan assumes GitHub Actions**
+
+Remote is bbchops/transcription on Gitea 1.27.2, which reads .github/workflows/ and proxies actions/checkout@v4 to GitHub. Workflow syntax needed no change. Note the remote default branch is traumatized, not main.
+
+#### [47] Runner availability cannot be confirmed via the API
+
+*risk* - **noted**
+
+**Repo-scoped runner list returns 0; admin endpoint returns 403**
+
+Existence of CI could not be asserted by query on this host. Proven instead by observation: an instance-level runner named docker-runner executed the jobs. Anyone re-verifying this must trigger a run rather than trust the runner API.
+
+#### [48] CI writes a .env file instead of exporting an env var
+
+*deviation* - **noted**
+
+**Settings reads the .env file; the external-test skip guard reads os.getenv**
+
+The two read different sources, and locally both conditions hold at once, which is why 4 tests skip. Measured in CI: no .env = 115 failed / 18 errors; exported dummy var = 3 failed (externals un-skip and hit the network); written .env file = the exact local baseline. Only openrouter_api_key is required.
+
+#### [49] CI invokes pre-commit rather than repeating ruff/ty commands
+
+*deviation* - **noted**
+
+**Plan task 2 asks CI to run the same checks as local**
+
+Satisfied structurally rather than by copying command strings: CI runs uv run pre-commit run --all-files, so the checks have a single definition in .pre-commit-config.yaml and CI cannot drift from local. Hooks are language: system and uv run puts .venv on PATH.
+
+#### [50] Platform-dependent prompt name guard, caught by CI on its first green run
+
+*deviation* - **noted**
+
+**The direct-child name guard relied on Path(name).name != name**
+
+On POSIX, backslash is an ordinary filename character, so nested\prompt.md passed the direct-child guard and failed later as NOT_FOUND instead of VALIDATION. Windows can never reproduce it. No traversal was possible because the path.parent != root check still held, so severity is a wrong error category plus a red gate. Fixed by rejecting / and \ explicitly, matching the ^[^/\\]+$ pattern config.PromptFilename already used. User approved the code fix over weakening the test.

src/transcription/services/prompts.py

@@ -104,8 +104,11 @@ class PromptStore:
 
     def _resolve_existing_prompt(self, name: str) -> Path:
         normalized_name = name.strip()
+        # Path().name is platform-dependent: POSIX treats "\" as an ordinary filename
+        # character, so reject both separators explicitly to match config.PromptFilename.
         if (
             not normalized_name
+            or any(separator in normalized_name for separator in ("/", "\\"))
             or Path(normalized_name).name != normalized_name
             or Path(normalized_name).suffix.lower() != PROMPT_EXTENSION
         ):