Consolidate the scattered run instructions and pass/fail criteria into one place:
- design spec gains 'Running the spike' (venv bootstrap + flags + DB prereqs) and
'Definition of done' (Req 0-4 are the bar; coverage does not gate done-ness;
the only follow-up at ~100% CLIP coverage is one no-flag re-run to snapshot the
final coverage into the contract doc) sections, and the status line now reads
'passed 2026-06-27'.
- script docstring Usage now includes the missing 'python3 -m venv .venv' bootstrap
a fresh checkout needs, and points to the spec's definition of done.
Code-review follow-ups on the pgvector spike:
- self-similarity sanity check now compares to the seed's own self-distance
(0 for cosine/L2, ~-1 for inner product <#>) instead of a hardcoded ~0, so it
no longer misfires if Immich ever uses a vector_ip_ops index (correctness P3).
- mark the deliberate raw-SQLite read in the probe as a spike-only exception and
add an M1.5 TODO that the real pgvector reader belongs in shared/photoflow/immich
and SQLite access in shared/photoflow/core (project-standards P2 x2).
- document join_and_coverage's return shape; extract a _pct() helper to dedupe the
coverage-percentage formatting (maintainability P3 x2).
Findings doc refreshed from the latest live run (coverage now ~46%, re-run ongoing).
Fixes both false claims the spec flagged: M1.5 was never 'verified in M1' and M1
did not 'run a read-only feasibility spike' (it deferred it). The spike has now
run separately and passed, so both are updated to point at the findings spec
(smart_search.embedding, 1152-dim, cosine, clean assetId->asset.id join) and the
spike is listed under Related specs.
scripts/pgvector_spike.py probes Immich's Postgres read-only (session-level
read-only guard) and answers the M1.5 prerequisites against the live DB:
catalog-discovered embedding table/column, vector dimension, distance operator
(from the index opclass), the embedding->asset FK join, and coverage over the
IMAGE population. Discovers the asset table name from the FK (asset, not the
legacy assets) rather than hardcoding it, so it survives Immich version drift.
Findings (this Immich version): smart_search.embedding, 1152-dim, cosine <=>
(vector_cosine_ops), assetId->asset.id join clean (0 orphans), ~44% image
coverage (CLIP re-run in progress). Doc is M1.5's version-pinned contract.
Optional Postgres DSN field on Config (REST creds stay required), env.example
entry, and a throwaway scripts/requirements-spike.txt (psycopg3 + pgvector)
kept out of the app's runtime deps. Unblocks the M1.5 pgvector feasibility spike.
Multi-persona review (coherence, feasibility, product-lens, security-lens,
adversarial) surfaced that the spike proved DB access but not that DB access
was the right path or that the signal was useful, and pinned a contract
against a private, in-flight-changing schema. Applied 11 fixes:
- Requirement 0: test the "REST can't expose embeddings" premise instead of
asserting it; record which endpoints were checked and why insufficient.
- Reframe Req 1 "real go/no-go" to access-only; signal-usefulness is M1.5's
first task, not this spike's.
- Mark the schema unsupported/internal, version-pinned; require an M1.5
re-probe/version-guard per Immich upgrade; tie shape to recorded model+ver.
- Fix probe correctness: pgvector adapter / server-side vector_dims (psycopg3
returns vector as string); Postgres-internal join with optional SQLite
cross-check; coverage over the image/embeddable population (both ratios).
- DB-enforced read-only session; standalone .env loading; psycopg+pgvector
added to Deliverables; roadmap correction now fixes both false claims.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Defines the read-only Postgres/pgvector feasibility spike that the
roadmap claims ran in M1 but the M1 plan deferred. Pins the four
pass/fail requirements, the disposable-probe approach, and the
findings-doc contract that unblocks M1.5.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A later successful asset could push last_ingest_at past an earlier asset whose
thumbnail download failed, permanently excluding it from later incremental runs
(only --full recovered it). Now the cursor never advances to/past the earliest
failed asset, never below the prior cursor, and not at all on a --subset run.
Regression tests added. (review finding #9)
- write-back: call upsert_tag inside _apply_tag's try so a tag-create failure
is caught per-asset and apply_all no longer aborts mid-batch (was P1)
- write-back: surface _pipeline/processed write failures in the result instead
of discarding them (was reported as success)
- ui: add title to the high confidence badge so approve-high-confidence's
pre-action count is non-zero; confirm() before single-cluster apply
- core: Store context manager; close DB connection even if a route raises;
guard split_cluster against a first-member boundary (empty cluster); add
writeback_log lookup index
- ingest: split thumbnail download/write error handling and clean up the
.tmp file on a write failure
- tests: upsert/processed write-failure regression tests + split-guard test
m038
