[ergofobe]

Two bugs in the same subsystem: what happens to faces and to centroids when a person is deleted or a grouping is corrected.

#87 — an unscoped recount deleted a person and unassigned faces on hidden assets

faces.person_id is on delete set null, so deleting a person does not tidy a row — it detaches every face still pointing at them. refreshCounts deleted whoever had no face on a visible photograph, so a person whose remaining faces all sat in the trash was removed and their faces silently unassigned; restoring the photograph brought the face back belonging to nobody.

Counting and deleting are now different questions. The count still sees only visible photographs; deletion asks whether any face is left at all. A person with none visible is zeroed instead — count and cover both, since a cover left pointing into a trashed photograph would put that crop on the People page.

That defers their end rather than removing it, so two things follow:

• the retention sweep recounts each owner it destroyed a photograph for. Once the photograph itself is gone there is nothing left to restore, and nothing else would notice. Swallowed and logged per owner: every deletion has committed by then, and letting the owner lock time out would strand the owners behind it and report a sweep that worked as a failure.
• listPeople moves its face_count > 0 filter into SQL (the discarded set is no longer transient — one production owner has 5,758 people), and findPeopleByName gains the same predicate, so a kept-but-empty person is not offered by search.

Test: a person whose photographs are all out of sight › survives the recount, keeps its faces, and comes back on restore — recounts, then asserts the person survives at face_count = 0, asserts the face row still names them, then un-trashes and asserts count and cover come back. Failed before the change with Received: undefined (the person had been deleted). is deleted only once it has no faces left anywhere and is not offered by a search for their name cover the other two halves; the existing test that asserted the old behaviour is rewritten.

#88 — reassignFaces recomputed only the destination's centroid

Every source person kept the moved face's embedding in its mean and went on attracting faces it no longer contained. Sources are now read before the move and recomputed alongside the destination.

Test: reassigning faces away from a person › recomputes the source centroid, not only the destination — two people built from orthogonal unit embeddings, so the mean is a value the test can spell out. After moving the face on axis 1 away from Anna, her centroid must read [1, 0, …]. Failed before the change with Expected: 0, Received: 0.70710677 — the axis of the face she no longer had. Two more cover unassigning entirely and sources spread across several people.

Locking

reassignFaces takes the owner's key exclusively (lockedForOwner), where naming a destination previously took that person's key alone.

The people losing a face are not known until the faces are read, and a lock key has to be chosen before the transaction that reads them. That is the same bind recordFace hits when the person it needs does not exist yet, and it is answered the same way: only the owner's key covers a set discovered that late, and taking a person key after the transaction has started would break the fixed order the lattice depends on.

So the lattice from #72 is untouched. lockedForPeople still takes the owner key shared and then person keys exclusive in sorted order; lockedForOwner still takes the owner key exclusive and no person key at all. No path acquires the owner key while holding a person key, so no cycle is constructible — this change adds no new key and no new order. The two forms are mutually exclusive on the owner key, which is the serialisation wanted.

It also costs nothing: reassignFaces ends with an unscoped refreshCounts, which takes that same key exclusively, so a reassignment has always serialised on it.

Because the rebuild now runs under an exclusive owner key, recomputeCentroid became recomputeCentroids — one statement over the whole set using pgvector's avg and l2_normalize, rather than reading 512 floats per face back to average them in JavaScript. The size of that hold is the cost.

Also fixed, because this change made them reachable

• The running mean was sized by the wrong number. face_count is what the library shows and counts only visible photographs; the mean is built from every face. Keeping an out-of-sight person alive made face_count = 0 reachable on a person who still has faces, and updateCentroid reads a count of nothing as a brand new person — so a fresh photograph of someone whose earlier ones were all trashed replaced a fifty-face identity with the single face being filed, permanently. The arithmetic now counts the faces it is actually averaging over. Test: a person out of sight keeps their mean when a new face joins them, which failed at exactly 1.0000000019426158 — the mean was the face just filed.
• recomputeCentroids writes face_count the way refreshCounts does. The recount runs in a second transaction; since the count now gates the People list and search, whatever the first transaction leaves has to stand on its own. Test: leaves a merge survivor visible when the recount afterwards fails.
• It is driven from people by a left join, so a person a reassignment emptied is named too and their mean goes null — which is what stops them attracting a face they no longer contain.

Found, not fixed

• Vaulting a photograph destroys its faces outright. VaultService.moveIn deletes the faces rows in the same transaction that sets vaulted_at, so taking a photograph out of the vault does not restore the grouping — the symmetry An unscoped recount deletes a person and unassigns faces on vaulted or trashed assets #87 establishes for the trash does not hold for the vault. Out of scope here; the comments in this diff were corrected to stop implying otherwise.
• cover_face_id still crosses the two-transaction gap. recomputeCentroids leaves the cover to the follow-up refreshCounts, so between them a reassigned person's tile can show a crop that now belongs to somebody else, and a merge survivor who was previously empty lists with no thumbnail. The face_count half of that window is closed here; the cover half is the same shape and pre-existing.
• refreshFace sizes the mean after clearFaces has run. The stored centroid still embodies the unconfirmed faces that delete just removed, so the divisor is a little small. A numerical wobble in an approximation that is documented as one — the previous divisor was wrong in a different direction — not a new class of error.
• sweepTrash recounts an owner whose account tombstone removeEmptyTombstones is about to remove. Provably wasted work, harmless.

Closes #87
Closes #88

🤖 Generated with Claude Code