The pre-commit hooks declared `language: system` with bare `ruff`/`ty`
entries, but both are uv-managed dev dependencies and are not on PATH, so every
commit failed with `Executable 'ruff' not found`. Route both through
`uv run`; keep ruff blocking and make ty advisory (verbose) until its 18
whole-project diagnostics are cleared.
With the gate working, clear `ruff check .` to zero:
- 18 auto-fixes (import sorting, blank lines, `max()` simplification,
`with` merging, unused imports).
- Real defects: `SourceNavigation` annotated but never imported in
sources_page; two naive `datetime.now()` calls in migration.py now use
`datetime.now(UTC)`.
- Dead parameters removed: `source_has_photo_table` (computed, passed, never
read), `_serialize_value(key=...)`, and unused `request` on two NiceGUI
page handlers where the framework injects it optionally.
- Mechanical line-length wrapping and one `startswith` tuple collapse.
- `# noqa: PLR0915` / `# noqa: PLR1702` on five long UI/migration
functions, following the convention already used in jobs_page and
settings_page, rather than refactoring during stabilization.
Full suite green (377 tests, `-m "not external"`).
Co-authored-by: Copilot App <[email protected]>
Move orientation normalization to the Source-ingest boundary and delete the
ProcessingArtifact subsystem it was built to serve.
Stored pages are now already upright, so nothing downstream derives a rotated
copy: every stored byte is the byte a provider is later sent. Rotation runs in
store_source_file ahead of hashing, so source.file_hash and file_size_bytes
describe exactly what is on disk. normalize_orientation becomes bytes-in /
bytes-out, and JPEG output reuses the source quantization tables and chroma
subsampling instead of re-quantizing at a fixed quality - measured at 50.3-56.1
dB PSNR at -6% size, against 50.0-53.5 dB at +38% for quality=95.
ProcessingArtifact held 2 rows against 77 successful transcriptions; the
subsystem effectively never ran. Deleting it removes the artifact cluster from
sources.py, the derivative resolution in workflows.py, the pre-provider commit
that only existed to make an artifact row durable, and the artifact evidence
dump from the Source detail page. The transcription_quality_warnings payload
folds into execution_attempt.normalized_metadata, so that feature keeps working
without the table.
tools/migrate_v46_to_v47.py carries steps 1 and 2: it rotated the 58 stored
images carrying EXIF orientation 3 in place, updated their recorded hash and
size, dropped processing_artifact and removed its one external file. It is
idempotent, keyed on state rather than a version marker.
tools/migrate_v45_to_v46.py is deleted. That migration is complete, and after
V4.7 it would restore a V4.5 backup into a schema that no longer matches.
Also fixes tests/test_config.py, which read the developer's local .env and
failed whenever WORKER_MAX_RETRIES was set.
Co-authored-by: Copilot App <[email protected]>
Baseline was 207 diagnostics. Two real bugs were hiding in the noise:
- tools/run_destructive_tests.py imported ctypes.wintypes at module scope,
which raises on non-Windows, and called fcntl unconditionally. The Windows
and POSIX implementations now live under a module-level sys.platform split.
- tests/ui/test_sources_page.py constructed Source(...) without document_id.
Structural fixes, not suppressions:
- New src/transcription/db/loading.py owns the SQLModel-field to
QueryableAttribute reinterpretation via orm_attribute()/selectinload()/
defer(). This removed 42 "# pyright: ignore[reportArgumentType]" comments
across documents/jobs/people/sources. Its docstring records that
selectinload(A.b, B.c) is NOT equivalent to the chained form: varargs
applies the selectin strategy only to the last path element, which under
lazy="raise" raises InvalidRequestError at render time.
- db/session.py transaction_scope no longer accepts or yields
AsyncSessionTransaction. No caller ever passed one, sessionmaker.begin()
yields an AsyncSession, and the dead branch was latently buggy because
services call .exec(). Cleared 7 workflows.py diagnostics.
- services/registry.py RegistryService is bound by a new RegistryEntry
Protocol instead of bare SQLModel, so the shared implementation can read
id/label/normalized_label/is_active. Cleared 9 diagnostics.
- Column expressions in sources.py/jobs.py/test_store.py wrap in sqlmodel
col(), the idiom already used in registry.py.
- read_source_navigation wraps its literal tuple bounds in literal().
- normalization.py narrows with isinstance(image, TiffImageFile) rather than
comparing image.format, since tag_v2 is TIFF-only.
- linked_people.render uses @ui.refreshable_method, the NiceGUI API for bound
methods.
- The OpenRouter capturing client re-raises ResponseNotRead when the response
stream is not async rather than mis-wrapping it.
Tooling gate:
- New .pre-commit-config.yaml runs ruff check and ty check as blocking hooks.
No pre-commit config previously existed. Negative-tested: injecting a type
error fails both hooks.
- The last two "# pyright: ignore" comments (config.py) are removed; ty does
not honor pyright directives. One "# ty: ignore" remains, in
tests/test_prompts.py, where the test deliberately assigns to a frozen
field to assert ValidationError.
- asyncio_default_fixture_loop_scope is pinned to "function" so
pytest-asyncio behavior does not shift on upgrade.
Verification: ruff check clean, ty check reports 0 diagnostics, 292 passed
and 4 skipped, pre-commit passes and demonstrably fails on a regression, and
tools/run_destructive_tests.py runs on Windows.
Co-authored-by: Copilot App <[email protected]>
ui/homepage_store.py was the only storage path in the codebase derived from
Path(__file__).parents[3] rather than from Settings. That made it the one
storage root the operator could not relocate, and it resolved incorrectly
outside a source checkout - an installed distribution would have written
homepage content into the package directory in site-packages.
- config.py: add homepage_dir, defaulting to ./data/homepage so the location
is unchanged for anyone launching from the repository root.
- homepage_store.py: resolve the directory and markdown path from Settings,
with an optional settings parameter on every function so callers and tests
can override without patching module constants. HOME_PAGE_DIR and
HOME_PAGE_MARKDOWN_PATH constants are replaced by homepage_dir() and
homepage_markdown_path().
- tests/ui/test_homepage_store.py: covers the setting being honored, markdown
round-tripping, image storage and listing, and two configurations not
sharing storage.
Note: the default is now CWD-relative, matching artifact_dir and upload_dir,
rather than anchored to the repository root.
Verification: ruff check src tests clean; 292 passed, 4 skipped.
Co-authored-by: Copilot App <[email protected]>
These changes all regenerate the same schema, so they land together and revert
together. A partially applied schema pass is not a valid state.
Remove hand-rolled migrations [HIGH-05]
- Delete upgrade_schema and the _upgrade_person_family_search_id /
_upgrade_v42_evidence_tables / _upgrade_v45_selection_columns chain, plus the
two tests that exercised them. The DDL was SQLite-shaped raw SQL that would
not have run on PostgreSQL. create_all now derives everything from metadata
and remains gated by Settings.should_bootstrap_schema. No raw ALTER TABLE or
CREATE INDEX string remains in src.
Break the foreign key cycle [HIGH-08]
- Declare Source.preferred_execution_attempt_id with use_alter=True and an
explicit constraint name. source / job_source / execution_attempt formed an
unresolvable cycle that made metadata.sorted_tables emit an SAWarning and
order execution_attempt before source, which would have been a hard
create_all failure on PostgreSQL and was invisible on SQLite.
- As a side effect the column is now a dialect-aware Uuid rather than the
hardcoded CHAR(32) the raw upgrade DDL produced, so it emits native UUID on
PostgreSQL.
Index the hot filters [HIGH-04]
- Add composite Index("ix_job_status_date_created", "status", "date_created")
for the worker poll, and index the foreign keys the worker and detail pages
filter on: job.document_id, source.document_id, job_source.job_id,
job_source.source_id, document.document_type_id, and the three
document_person foreign keys.
Stop preloading by default [CRIT-02]
- Flip 16 relationships from lazy="selectin" to lazy="raise". The bidirectional
selectin defaults meant loading one Job pulled a large connected subgraph.
- Three further relationships (ExecutionAttempt.job_source,
ProcessingArtifact.execution_attempt, ProcessingArtifact.source) declared no
lazy at all and defaulted to "select", which raises MissingGreenlet under
async. These are now "raise" as well.
- Only 5 of 262 tests failed under the flip; the service layer already carried
explicit eager loads. Fixes went into the service queries, never back into
the models:
- PeopleService._finalize_link refreshes document, person, and role_ref so
the DocumentPerson write endpoints can still project them.
- JobService.update_job_state loads job_sources -> source so the Job it
returns still answers .error_detail and .filename.
- Two tests that bypassed the service layer now load explicitly.
- Audited every UI relationship access against its feeding service method; all
resolve to *_detail / list_*_detail variants with complete eager loads.
Tests
- Assert the composite and hot foreign key indexes exist in a fresh schema.
- Assert metadata.sorted_tables raises no SAWarning and orders source before
execution_attempt.
- Assert preferred_execution_attempt_id is a Uuid that compiles to UUID on
PostgreSQL and that its foreign key carries use_alter.
- Guard CRIT-02 from regression: no mapped relationship may declare a lazy
strategy outside {raise, noload}.
The development database was rebuilt from metadata rather than upgraded; the
previous file is retained out of tree as the Phase 8 migration source.
Verified: 266 passed, 4 skipped; ruff check clean.
Co-authored-by: Copilot App <[email protected]>
Pure remediation; no behavior change. Every item traces to a finding in
docs/architecture_code_review_2026-08-17.md.
Deletions
- Delete app_state.py, which had zero importers and whose get_session_factory
raised TypeError at runtime [HIGH-01].
- Delete services/transcription.py and point build_prompt_execution imports at
services/sources.py; drop the store.py compatibility aliases [MED-05].
- Delete ServiceBase.queue and its unparameterized asyncio.Queue [MED-07].
- Delete db/operations.get_next_queued_job, a divergent duplicate [CRIT-01].
- Drop the discarded load_docs parameter from list_jobs [LOW-03].
Config
- Delete worker_retry_backoff_seconds; no backoff behavior existed anywhere, so
wiring it would have been a new feature [MED-02].
- Wire sqlite_check_same_thread through get_engine. The engine hardcoded the
setting's own default, so this preserves behavior exactly [MED-02].
- Replace DATABASE_URL in docker-compose.yml with the nested DATABASE__DRIVER /
DATABASE__PATH names. Settings uses env_nested_delimiter with extra="ignore",
so DATABASE_URL was silently discarded [MED-10].
UI
- Move the 23KB inline VIBESCRIBE_LOGO_SVG to ui/static/vibescribe_logo.svg and
load it through a cached read_svg sibling of read_css [MED-09].
- Route the portrait upload failure through error_presenter.show_error [LOW-07].
- Cancel the job detail auto-refresh timer instead of only deactivating it, and
name its interval constant [LOW-06].
Worker
- Make WorkerNotifier runtime_checkable and validate the resolved object in
resolve_worker_notifier, which previously returned any non-None attribute
unchecked [LOW-04].
Docs and lint
- Fix two stale paths in services.instructions.md, one of which pointed at the
module deleted here [LOW-02].
- ruff check --fix to zero [LOW-01].
Verified: 264 passed, 4 skipped; ruff check clean.
Co-authored-by: Copilot App <[email protected]>
Jim Lancaster
