list_embeddings cloned the full FaceDetectionRow inside the filter_map
just to pair it with the base64-encoded embedding. The 2 KB BLOB was
already on the row — at 20k unassigned faces that's 40 MB of pointless
heap traffic per Apollo cluster-suggest run. Move the bytes out via
Option::take() so the row drops the BLOB instead of duplicating it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PATCH /image/faces/{id} can return 503 from three places (face client
disabled, transient embed error, mid-flight disable) and none of them
were logging — operator sees the status code but nothing in the Rust
log explaining why. Add warn! lines at each so future bbox-edit
failures aren't silent. Response body is unchanged so existing clients
keep working.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
face_detections is keyed on content_hash (one row per unique bytes,
shared across libraries / duplicate paths) but total_photos was
COUNT(*) over image_exif rows. A file present at multiple rel_paths or
across libraries inflated the denominator without inflating the
numerator, leaving a permanent gap (e.g. 1101/1103 with nothing
actually pending detection).
Switch total_photos to COUNT(DISTINCT content_hash) so numerator and
denominator live in the same domain. Exclude rows with NULL
content_hash from the count — they're held in the hash-backfill
backlog, not the detection backlog, and counting them pins the bar
below 100% for the duration of that pass.
CLAUDE.md: document the stats domain rule next to the rest of the
face-detection notes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
list_unscanned_candidates pulled every hashed image_exif row, including
videos. filter_excluded then dropped them client-side without writing a
marker, so the same set re-appeared every watcher tick — emitting the
"backlog drain — running detection on N candidate(s)" log forever and
producing no progress.
face_stats.total_photos counted the same video rows in the denominator,
so the SCANNED percentage was structurally capped below 100%.
Add an image-extension SQL predicate (case-insensitive, sourced from
file_types::IMAGE_EXTENSIONS) and apply it to both queries. Videos
never enter the candidate set, total_photos counts only what can
actually be scanned, and 100% becomes reachable.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Symptom: ImageApi restart, then ~60 minutes of silence — no
face_watch lines at all. Cause: backfill + face-detection candidate
build were both gated inside process_new_files, which during quick
scans (every 60s) only walks files modified in the last interval.
The pre-existing unhashed / unscanned backlog never entered the
candidate set, so it only drained on the full-scan path (default
once per hour). Surfaced as "scan stuck at 1101/13118" — most of
those rows were waiting on the next full scan.
Two new per-tick passes that work directly off the DB:
(1) backfill_unhashed_backlog uses ExifDao::get_rows_missing_hash to
pull unhashed rows in id order, capped (FACE_HASH_BACKFILL_MAX_PER_TICK
default 2000), and writes content_hash for each. No filesystem
walk — the walk was the gating filter that hid the backlog.
(2) process_face_backlog uses a new FaceDao::list_unscanned_candidates
(LEFT-anti-join on content_hash via raw SQL, GROUP BY hash so
duplicates fire one detect call) to pull a capped batch of
hashed-but-unscanned rows (FACE_BACKLOG_MAX_PER_TICK default 64)
and runs the existing face_watch detection pipeline on them.
Both run only when face_client.is_enabled(). The cap on (2) is small
because each candidate is a real Apollo round-trip — 64/tick at 60s
quick interval ≈ 64 detections/min, which paces an 8-core CPU
inference comfortably while keeping a steady flow visible in logs.
process_new_files's own backfill stays in place for the same-tick
flow (a brand-new upload gets hashed AND face-scanned in the tick
where it's discovered) but is now belt-and-suspenders.
Test backstop pinning the new DAO method's filter contract: only
hashed, unscanned, in-library rows are returned; scanned rows,
unhashed rows, and other-library rows are filtered out.
Two reasons manually-drawn bboxes were never resolving a face on
re-detection:
(1) The bbox arrives in display space (browser already applied EXIF
orientation when rendering the carousel), but the `image` crate
in crop_image_to_bbox opens raw pre-rotation pixels. For any
phone photo with Orientation 6/8/etc., applying the bbox without
rotating first crops a completely different region of the image
— landing on background, hair, or empty pixels. Now reads the
EXIF Orientation tag and applies it before indexing into the
canonical-oriented dims.
(2) Padding was 10 % on each side. A typical 200×250 face bbox +
10 % becomes ~240×300; insightface resizes that to det_size=640,
so the face fills ~95 % of the input. RetinaFace's anchors
expect faces at 20–60 % of input dimensions; at 95 % it
routinely returns zero detections. Bumped to 50 % padding so the
crop is 2× the bbox dims and the face occupies ~50 % of the
input — anchor-friendly. Bbox is still clamped to image bounds,
so edge-of-image cases just get less padding on the clipped
side.
Together these explain why bbox-edit re-embed practically always
fell into the "no face detected" branch (and bbox-edit reverts
without the recent soft-fallback commit). Per-photo embedding
quality also improves slightly — same face, more context, better
landmarks for ArcFace.
Moving a tagged bbox off-center (to fine-tune position, or onto a
back-of-head the operator already manually tagged) made
update_face_handler 422 because the re-embed step ran detection on
the new crop and found nothing. Frontend's catch then reverted the
optimistic update — visible as the bbox snapping back the moment the
user released their drag.
The re-embed is a soft contract: a fresh ArcFace vector is preferable,
but the operator's bbox edit is sacred. Now:
- empty faces[] → keep old embedding, apply the bbox, log info
- permanent embed error → keep old embedding, apply the bbox, log info
- bad-bytes embedding → keep old embedding, apply the bbox, log warn
- transient failure (cuda_oom, engine unavailable) still 503s so
the operator can retry — those are recoverable and we don't want
to silently drift cluster math on retries that succeed later
Cost: a slightly stale embedding for the row, which marginally
affects clustering / auto-bind cosine for files re-detected against
this person. Accepted because dropping the user's manual drag every
time the new crop happens to lose detection is a much worse UX —
especially for the force-create rows (back of head, profile) where
re-detection will *always* fail.
Adds an opt-in 'force' flag to POST /image/faces. When set, the handler
skips the Apollo embed call entirely and stores the row with a
2048-byte zero-vector embedding under the sentinel model_version
'manual_no_embed'. The row participates as a browse-by-person tag but
is excluded from clustering and auto-bind:
- face_clustering._decode_b64_embedding filters norm<=0 (already)
- cluster suggester groups by model_version, so the sentinel never
mixes with real buffalo_l rows
- cosine_similarity with a zero vector resolves to 0/NaN, never
crossing the 0.4 auto-bind threshold
Use case: tag someone looking away from the camera, profile shot,
heavily-occluded face — anywhere the detector returns no_face_in_crop
on the user's drawn region. The frontend only sets force=true after a
422 from a strict create plus an explicit operator confirmation, so
the normal "draw a centered face" UX still gets a real ArcFace
embedding.
Two unit tests pinning the response shape that PATCH/POST /image/faces
relies on. They use the existing in-memory SQLite harness and exercise
the helper directly:
- assigned: person_name resolves through the persons join and bbox /
source / person_id round-trip cleanly.
- unassigned: person_name is None (not stale, not omitted), person_id
is None.
These would have caught the prior regression — when the handlers
returned a bare FaceDetectionRow, person_name was structurally absent
from the response shape. A test that asserts person_name is populated
when person_id is set forces the join (or any equivalent) to exist.
A dangling-person_id case isn't covered: the FK on face_detections
makes that state structurally impossible at rest (ON DELETE SET NULL
zeroes the column when a person is removed), so there's nothing to
defend against.
Both create_face_handler and update_face_handler returned the bare
FaceDetectionRow, so PATCH /image/faces/{id} (used by both bbox edits
and person assignment) replied without person_name. The carousel
overlay does an optimistic replace on this row — replacing the joined
FaceWithPerson with a row that has person_name = undefined visibly
dropped the VFD label off the bbox after every save.
Add a small hydrate_face_with_person helper that does the persons
lookup and assembles a FaceWithPerson, used by both handlers. The
list endpoint already does the join, so the PATCH/POST shape now
matches it.
Phase 2 stored the new bbox on PATCH /image/faces/{id} but logged
"embedding now stale (Phase 3 will re-embed)" and moved on. That left
the embedding column pointing at the *old* face area while the bbox
described a new one — auto-bind cosine similarity and the cluster
suggester would silently rank the row as "the same face it was before
the edit" forever after, even though the geometry no longer matched.
Now: when the PATCH includes a bbox, the handler:
1. Looks up the row to find its photo (library_id + rel_path).
2. Crops the new bbox region with the same crop_image_to_bbox helper
manual-create uses (10% pad on each side so the detector has
ear/jaw context).
3. POSTs the crop to face_client.embed for a fresh ArcFace vector.
4. Stores both the new bbox AND the new embedding in one
update_face transaction.
Errors map cleanly:
- face_client disabled → 503 (bbox edit needs Apollo).
- decode failure / no face in crop → 422.
- Apollo CUDA OOM / unavailable → 503 transient.
- Underlying row missing → 404.
About 100-500ms per edit on CPU, dominated by Apollo's inference call.
Acceptable for a manual operator action; the alternative (stale
embedding) silently broke the rest of the face stack.
Prerequisite for the upcoming carousel-side draw/resize bbox UI —
without re-embed, every operator-driven bbox tweak would corrode the
clustering/auto-bind quality. ApiPatchFaceBody on Apollo's side
already passes bbox through verbatim, so no Apollo change needed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A single global "Ignored" person row, marked is_ignored=true, that the
frontend lazily creates on first use to hold strangers, false
detections, and faces the user doesn't want bound to a real person.
Schema (new migration 2026-04-29-000200_add_is_ignored):
- persons.is_ignored BOOLEAN NOT NULL DEFAULT 0
- Partial index on (is_ignored) WHERE is_ignored = 1; small WHERE
set means a tiny index that only ever services the bucket lookup.
Why a real persons row instead of a separate table or status enum:
- face_detections.person_id stays a clean foreign key — no special
code paths for "ignored faces" anywhere else in the schema.
- The cluster-suggester already filters by `person_id IS NULL`, so
bound-to-ignored faces are naturally excluded from re-clustering
without any change.
- merge / rename / delete all work on it with the existing routes
(the management UI just hides it from default views).
DAO additions / changes:
- get_or_create_ignored_person (idempotent; race-safe via the
UNIQUE COLLATE NOCASE on persons.name + retry-on-409 fallback).
- list_persons gains an include_ignored parameter; default false
so the management screen hides the bucket unless asked.
- find_persons_by_names_ci filters is_ignored=0 in SQL so the
auto-bind path can NEVER target the bucket — even if the user
happens to tag photos as "Ignored", the heuristic look-up skips
it. Bucket assignment is always an explicit operator action.
- update_person accepts is_ignored: Option<bool> so a person can
be moved into / out of the bucket without a delete + recreate.
Routes:
- POST /persons/ignore-bucket — returns the bucket, creating it on
first call. Frontend uses this lazily right before binding.
- GET /persons gains ?include_ignored=true; default behavior
unchanged.
- PATCH /persons/{id} now accepts is_ignored.
Tests: ignore_bucket_idempotent_and_filters_auto_bind covers the
contract: bucket is idempotent across calls, find_persons_by_names_ci
skips it (even on exact name match), default list_persons hides it,
include_ignored=true surfaces it. All other tests updated to pass
the new is_ignored: false / Option<bool> fields explicitly.
cargo test --lib: 181/0; fmt + clippy clean for new code.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apollo's cluster suggester wants to render a *face*-cropped thumbnail
for each cluster's representative — a multi-person photo with the
cluster about 'one' of them was unreadable when the thumb showed the
whole image. Plumbing bbox through means the UI can crop to the rep
face without an extra round-trip per cluster.
FaceEmbeddingRow gains bbox_x/y/w/h (Optional<f32>, mirrors the column
nullability — for status='detected' rows the CHECK constraint
guarantees they're populated, but the type stays nullable as
documentation). list_embeddings already loaded these from the
underlying FaceDetectionRow; this commit just stops dropping them
when constructing the response.
No DB changes; no behavior change for existing callers (the new
fields are additive).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Filter <3-char tags and emoji/symbol-bearing tags out of the bootstrap
candidate list before grouping. Manual testing surfaced these as noise
the operator never tickets — they pushed real candidates lower in the
list and made the UI harder to scan. This is a hard filter (drop from
candidates entirely), not a heuristic flag — looks_like_person still
governs the default-checked decision for the rows that *do* survive.
is_plausible_name_token rules:
- >= 3 chars after trimming (rejects "AB", "OK", whitespace-only)
- Each char is alphabetic (any script — covers Renée, José, 田中太郎),
whitespace, name-punctuation (' - . _ U+2019), or ASCII digit
- Anything else (emoji, symbols, math, arrows, control codes) drops
the whole tag
Digits stay allowed at this layer; looks_like_person handles "Trip 2018"
on the heuristic side. Lets a "Sarah2" alias still appear so the
operator can spot and confirm it manually, just unticked by default.
Cargo version bump 1.0.0 → 1.1.0 marks the face-recog feature surface
landing — Phase 2's schema + endpoints, Phase 3's file-watch hook, and
Phase 4's bootstrap + auto-bind are all behind APOLLO_FACE_API_BASE_URL,
so legacy 1.0 deploys without that env see no behavior change.
Tests: 1 new (faces::tests::is_plausible_name_token_filters_short_and_emoji)
covers the accept-list (Latin/accented/Asian scripts, hyphenated and
apostrophe names) and the reject-list (length floor, emoji classes,
symbols, leading/trailing whitespace handling).
cargo test --lib: 180 / 0; fmt + clippy clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the existing string people-tags into the new persons table and
auto-binds new detections to a same-named person when the photo carries
exactly one matching tag. ImageApi has no notion of which tags are
people-tags today (purely a user mental model), so this is operator-
confirmed: the suggester surfaces candidates with a heuristic flag, the
operator confirms, then bootstrap creates persons rows. Auto-bind
follows on every detection thereafter.
New endpoints:
GET /tags/people-bootstrap-candidates
Per case-insensitive name group: display name (most-frequent
capitalization), normalized lowercase, summed usage_count,
looks_like_person heuristic flag, already_exists check against
the persons table. Sorted persons-likely-first then by count.
POST /persons/bootstrap
Body: {names: [string]}. Idempotent — pre-fetches the existing-
name set so a duplicate request reports per-row "already exists"
instead of 409-ing each insert. Created rows get
created_from_tag=true; failed rows surface in `skipped` with a
reason.
looks_like_person heuristic — conservative on purpose because the
operator confirms in the UI:
- 1–2 whitespace-separated words
- Each word starts uppercase, no digits anywhere
- Single-word names not on a small denylist (cat, christmas, beach,
sunset, untagged, ...). Two-word names skip the denylist so
"Sarah Smith" is never false-rejected.
FaceDao additions:
- find_persons_by_names_ci — bulk lowercase-name → person_id lookup
via sql_query (Diesel's BoxedSelectStatement + LOWER() doesn't
play well with the type system).
- person_reference_embedding — L2-normalized mean of a person's
detected embeddings, *filtered by model_version* so a future
buffalo_xl row can never contaminate an in-flight buffalo_l auto-
bind decision. Returns None when the person has no faces yet.
- assign_face_to_person — sets face_detections.person_id and, only
when persons.cover_face_id is NULL, claims this face as cover. The
UI's hand-picked cover survives later auto-binds.
- decode_embedding_bytes / cosine_similarity helpers — pub(crate)
so face_watch can decode the wire bytes once and feed them through
the cosine threshold.
Auto-bind in face_watch::process_one:
After every successful detect, for each newly-stored auto face we
pull the photo's tags, look up which (if any) map to existing
persons, and:
- skip when zero or multiple distinct persons are matched
(multi-match is genuinely ambiguous; cluster suggester handles it)
- on first face for a person: bind unconditionally so bootstrap can
ever produce a usable reference
- thereafter: bind iff cosine(new_emb, person_ref) >=
FACE_AUTOBIND_MIN_COS (default 0.4, env-tunable to 0..=1)
The reference embedding comes from person_reference_embedding under
the same model_version as the candidate, so a model upgrade never
silently re-anchors a person's centroid.
Plumbing: watch_files now constructs its own SqliteTagDao alongside the
other watcher DAOs and threads it through process_new_files →
run_face_detection_pass → process_one. The handler-side TagDao
registration in main.rs already covers bootstrap_candidates_handler;
no extra app_data wiring needed.
Tests: 8 new (faces.rs):
- looks_like_person accepts/rejects/two-word-skips-denylist (3)
- cosine_similarity on identical / orthogonal / opposite / mismatch /
zero / empty inputs
- decode_embedding_bytes round-trip + size validation
- find_persons_by_names_ci groups case + handles empty input
- person_reference_embedding filters by model_version (buffalo_l ref
must not include buffalo_xl rows)
- assign_face_to_person sets cover when unset, doesn't overwrite
cargo test --lib: 179 / 0; fmt + clippy clean for new code.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manual smoke test caught a bug: POST /persons with a duplicate name
returned 500 with the body 'insert person Cameron' instead of the
intended 409 Conflict.
Root cause: the handler keyed on `format!("{}", e).contains("unique")`,
but anyhow's plain Display only renders the *outermost* context
("insert person Cameron") and hides the diesel error nested below
('UNIQUE constraint failed: persons.name'). The string check was a
false negative on every duplicate.
Fix: walk the source chain and downcast for
diesel::result::Error::DatabaseError(UniqueViolation, _) — exposed
via a shared `is_unique_violation` helper used by both
create_person_handler and update_person_handler. Error bodies for
non-unique failures now use `{:#}` so the body actually carries the
underlying cause when the user surfaces it.
merge_persons_handler also moves to `{:#}` for richer error bodies;
the "itself" check was already structural and unaffected.
Regression test (faces::tests::is_unique_violation_walks_chain) pins
both the bug shape ({} doesn't surface UNIQUE) and the fix
(is_unique_violation correctly downcasts the chain), so a future
refactor of error handling can't silently re-bury this.
cargo test --lib: 171 / 0; fmt + clippy clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PathExcluder was iterating every component of the absolute path,
including the system prefix. Two of the existing memories tests had
been failing on master because tempdir() lives under /tmp on Linux
and a pattern like "tmp" then matched the system /tmp component
rather than anything the user actually asked to exclude. Phase 3's
file-watch hook will use the same code to skip @eaDir / .thumbnails
under each library's BASE_PATH, so the bug would hide every photo
on a host whose BASE_PATH passes through a directory named the same
as a user pattern.
Fix: store base in PathExcluder and strip it before scanning
components. A path that lives outside base falls through to the
no-match branch (defensive — nothing legit hits that today).
Also extracted the face_client error classification into a pure
classify_error_response(status, body) so the marker-row contract
with Apollo (422 → Permanent / 'failed', 5xx → Transient / defer)
is unit-testable without spinning up an HTTP server.
New tests:
memories::tests::test_path_excluder_* — 2 previously
failing tests now pass.
ai::face_client::tests::classify_* — 4 cases:
422 decode_failed → Permanent, 503 cuda_oom → Transient
(handles both string and {code:..} detail shapes), 5xx →
Transient + other 4xx → Permanent, unparseable HTML body still
classifies on status.
faces::tests::crop_* — 3 cases:
invalid bbox rejected, valid bbox round-trips through JPEG
decode, corner crop with 10% padding clamps inside source.
cargo test --lib: 165 passed / 0 failed (was 156 / 2 failed).
cargo fmt and clippy on new code clean. The remaining
sort_by clippy warnings in pre-existing files (memories.rs,
files.rs, exif.rs) are unrelated and present on master.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Land the persistence model and HTTP surface for local face recognition.
Inference still lives in Apollo (Phase 1); this side adds the data home
plus every endpoint Apollo's UI and FileViewer-React will consume.
Schema (new migration 2026-04-29-000000_add_faces):
- persons: visual identities. Optional entity_id bridges to the
existing knowledge-graph entities table; auto-bridging is left to
the management UI (we don't muddy LLM provenance from face rows).
UNIQUE(name COLLATE NOCASE) so 'alice' / 'Alice' fold to one row.
- face_detections: keyed on content_hash (cross-library dedup), with
status='detected' carrying bbox + 512-d embedding BLOB, and
'no_faces' / 'failed' marker rows that tell Phase 3's file watcher
not to re-scan. Marker invariant enforced via CHECK; partial UNIQUE
on content_hash WHERE status='no_faces' guards against double-marks.
Schema regenerated with `diesel print-schema` against a clean migration
run; joinables added for face_detections → libraries / persons and
persons → entities.
face_client.rs (sibling of apollo_client.rs):
- reqwest multipart, 60 s timeout (CPU inference on a backlog can be
slow; bounded threadpool on Apollo serializes calls anyway).
- FaceDetectError::{Permanent, Transient, Disabled} — Phase 3 keys
its marker-row decision on this. 422 → mark failed, 5xx → defer.
- APOLLO_FACE_API_BASE_URL falls back to APOLLO_API_BASE_URL when
unset; both unset = is_enabled() false, callers no-op.
faces.rs (DAO + handlers):
- SqliteFaceDao implements the full FaceDao trait; person face counts
go through sql_query because diesel's BoxedSelectStatement +
group_by trips trait-resolver recursion.
- merge_persons re-points face rows in a transaction, copies notes
when target's are empty, deletes src.
- manual POST /image/faces resolves content_hash through image_exif,
crops the user-drawn bbox with 10% padding (detector wants context
around ears/jaw), POSTs the crop to face_client.embed for a real
ArcFace vector, then inserts source='manual'.
- Cluster-suggest (Phase 6) gets its data from
GET /faces/embeddings — base64-encoded paged BLOBs so Apollo's
DBSCAN can stream them without ImageApi pre-aggregating.
Endpoints registered alongside add_*_services in main.rs:
GET /faces/stats?library=
GET /faces/embeddings?library=&unassigned=&limit=&offset=
GET /image/faces?path=&library=
POST /image/faces (manual create via embed)
PATCH /image/faces/{id}
DELETE /image/faces/{id}
GET /persons?library=
POST /persons
GET /persons/{id}
PATCH /persons/{id}
DELETE /persons/{id}?cascade=set_null|delete (set_null default)
POST /persons/{id}/merge
GET /persons/{id}/faces?library=
The file-watch hook (Phase 3) and the rerun-on-one-photo handler
(Phase 6) live behind the FaceDao methods marked dead_code today —
they're called only when those phases land. Same shape for the trait
methods that aren't reached by Phase 2 routes.
Tests: 3 DAO unit tests cover person CRUD + case-insensitive uniqueness,
marker-row idempotency (mark_status is a no-op when any row exists),
and merge re-pointing faces.
Cargo.toml: reqwest gains the `multipart` feature.
cargo build / cargo test --lib / cargo fmt / cargo clippy --all-targets
all clean for the new code; the two pre-existing test_path_excluder
failures and the pre-existing sort_by clippy warnings are unrelated and
present on master.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
