diff --git a/backend/__tests__/integration/faceSeparationsSurviveRescan.test.js b/backend/__tests__/integration/faceSeparationsSurviveRescan.test.js new file mode 100644 index 00000000..b7b12ae3 --- /dev/null +++ b/backend/__tests__/integration/faceSeparationsSurviveRescan.test.js @@ -0,0 +1,567 @@ +/** + * "Not the same person" has to outlive re-derivation (#1132). + * + * The decision used to be stored as a pair of event_people.id, and neither + * person ids nor face ids survive: + * + * - recluster() deletes every person and re-assigns, so person ids die but + * photo_faces.id survives + * - a full re-scan replaces a photo's faces outright, so FACE ids die too + * + * The embedding is the only stable handle, so that is what the separation is + * keyed on. These tests simulate both kinds of re-derivation by destroying the + * ids and rebuilding from the same vectors — which is exactly what the real + * paths do — and assert the constraint still binds. + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +process.env.NODE_ENV = 'test'; +process.env.TEST_DATABASE_PATH = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-sep-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'sep-test-secret'; + +const { bootCrmDb } = require('./helpers/crmDb'); + +let db; let cleanup; let clustering; + +const THRESHOLDS = { face_match_threshold: 0.6, face_quality_min_score: 0.7, face_quality_min_px: 40 }; +const DIM = 64; + +/** Two unit vectors whose dot product is exactly `target`, on basis (i, i+1). */ +function pairAtSimilarity(target, basis) { + const a = new Float32Array(DIM); + const b = new Float32Array(DIM); + a[basis] = 1; + b[basis] = target; + b[basis + 1] = Math.sqrt(1 - target * target); + return [a, b]; +} + +async function seedEvent(slug) { + const [row] = await db('events').insert({ + slug, event_type: 'wedding', event_name: slug, event_date: '2026-01-01', + host_email: 'h@example.com', admin_email: 'a@example.com', password_hash: 'x', + share_link: `${slug}-share`, expires_at: new Date().toISOString(), + }).returning('id'); + return typeof row === 'object' ? row.id : row; +} + +async function insertPerson(eventId, centroid, overrides = {}) { + const [row] = await db('event_people').insert({ + event_id: eventId, + centroid: clustering.packEmbedding(centroid), + face_count_total: 1, + model_version: 'test-v1', + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + ...overrides, + }).returning('id'); + return typeof row === 'object' ? row.id : row; +} + +/** The mirror of pairAtSimilarity's second vector: same similarity, other side. */ +function mirrorAtSimilarity(target, basis) { + const b = new Float32Array(DIM); + b[basis] = target; + b[basis + 1] = -Math.sqrt(1 - target * target); + return b; +} + +async function insertFaceWithPhoto(eventId, personId, centroid) { + const [p] = await db('photos').insert({ + event_id: eventId, filename: `${Math.random()}.jpg`, path: '/tmp/x.jpg', type: 'individual', + }).returning('id'); + const photoId = typeof p === 'object' ? p.id : p; + const [f] = await db('photo_faces').insert({ + photo_id: photoId, event_id: eventId, person_id: personId, + bbox_x: 0, bbox_y: 0, bbox_w: 200, bbox_h: 200, det_score: 0.99, + embedding: clustering.packEmbedding(centroid), + model_version: 'test-v1', created_at: new Date().toISOString(), + }).returning('id'); + return { faceId: typeof f === 'object' ? f.id : f, photoId }; +} + +async function insertFace(eventId, personId, centroid) { + const { faceId } = await insertFaceWithPhoto(eventId, personId, centroid); + return faceId; +} + +/** + * What a re-scan does to identity: the people are gone and the faces come back + * with brand-new ids. Same vectors, nothing else preserved. + */ +async function simulateRescan(eventId, vectors) { + await db('photo_faces').where({ event_id: eventId }).del(); + await db('event_people').where({ event_id: eventId }).del(); + const ids = []; + for (const vec of vectors) { + const personId = await insertPerson(eventId, vec); + await insertFace(eventId, personId, vec); + ids.push(personId); + } + return ids; +} + +describe('separations survive re-derivation (#1132)', () => { + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + clustering = require('../../src/services/faceClustering'); + }, 120000); + + afterAll(async () => { if (cleanup) await cleanup(); }); + + describe('the matcher', () => { + it('binds a pair that still looks like the one that was separated', () => { + const [a, b] = pairAtSimilarity(0.64, 0); + expect(clustering.separationForbids(a, b, [{ a, b }])).toBe(true); + }); + + it('binds regardless of which way round the candidates arrive', () => { + const [a, b] = pairAtSimilarity(0.64, 0); + // Neither the stored pair nor the candidate pair has a meaningful order. + expect(clustering.separationForbids(b, a, [{ a, b }])).toBe(true); + }); + + it('lapses once a side has drifted past recognition', () => { + const [a, b] = pairAtSimilarity(0.64, 0); + // A cluster reshaped far enough is no longer the cluster the + // photographer pointed at, so the constraint should stop applying rather + // than bind something they never saw. + const drifted = new Float32Array(DIM); + drifted[10] = 1; + expect(clustering.separationForbids(drifted, b, [{ a, b }])).toBe(false); + }); + + it('does not bind two clusters that are both the SAME side', () => { + // A split leaves two halves of one cluster, so the pair it records is + // often similar to itself — here 0.95. Two candidates that are plainly + // both side A (0.97 to each other) each clear the bar against BOTH + // stored sides, so a test that only asks "does each side match + // something" says yes and refuses to let that person cluster with + // itself. It fragments into singletons — the person the split was not + // even about. + const [a, b] = pairAtSimilarity(0.95, 0); + const x = new Float32Array(DIM); x[0] = 1; + const y = mirrorAtSimilarity(0.97, 0); + expect(clustering.separationForbids(x, y, [{ a, b }])).toBe(false); + // The pair it was actually about still binds. + expect(clustering.separationForbids(a, b, [{ a, b }])).toBe(true); + }); + + it('ignores a separation recorded under a different embedding model', () => { + const [a, b] = pairAtSimilarity(0.64, 0); + // Vectors from another model are meaningless here, not merely stale — + // the same rule assignment and consolidation apply to person centroids. + expect(clustering.separationForbids(a, b, [{ a, b, modelVersion: 'test-v2' }], + { modelVersion: 'test-v1' })).toBe(false); + expect(clustering.separationForbids(a, b, [{ a, b, modelVersion: 'test-v1' }], + { modelVersion: 'test-v1' })).toBe(true); + }); + + it('ignores an unrelated pair entirely', () => { + const [a, b] = pairAtSimilarity(0.64, 0); + const [x, y] = pairAtSimilarity(0.64, 20); + expect(clustering.separationForbids(x, y, [{ a, b }])).toBe(false); + }); + }); + + describe('across a re-scan', () => { + it('still refuses to merge the pair after every id has changed', async () => { + const eventId = await seedEvent('sep-rescan'); + // Well above the auto-merge threshold: only the separation keeps them apart. + const [a, b] = pairAtSimilarity(0.97, 0); + const idA = await insertPerson(eventId, a); + const idB = await insertPerson(eventId, b); + await insertFace(eventId, idA, a); + await insertFace(eventId, idB, b); + + await clustering.dismissMergeSuggestion(eventId, idA, idB); + + const newIds = await simulateRescan(eventId, [a, b]); + // The premise: nothing the old row named still exists. + expect(newIds).not.toContain(idA); + expect(newIds).not.toContain(idB); + + const merged = await clustering.consolidate(eventId, { thresholds: THRESHOLDS }); + + expect(merged).toEqual([]); + expect(await db('event_people').where({ event_id: eventId })).toHaveLength(2); + }); + + it('keeps the pair out of the suggestion list too', async () => { + const eventId = await seedEvent('sep-rescan-suggest'); + const [a, b] = pairAtSimilarity(0.64, 0); // inside the suggestion band + const idA = await insertPerson(eventId, a); + const idB = await insertPerson(eventId, b); + + await clustering.dismissMergeSuggestion(eventId, idA, idB); + await simulateRescan(eventId, [a, b]); + + expect(await clustering.suggestMerges(eventId, { thresholds: THRESHOLDS })).toEqual([]); + }); + + it('a split still binds after the ids it recorded are gone', async () => { + const eventId = await seedEvent('sep-split-rescan'); + // Two faces that look alike enough to have been clustered together, but + // are not the same vector — which is what a split is FOR, and the only + // case it can survive re-derivation in. Two byte-identical embeddings + // carry no information about which side is which, so a separation + // between them has nothing to key on once the ids are gone. + const [base, other] = pairAtSimilarity(0.96, 0); + const personId = await insertPerson(eventId, base); + await insertFace(eventId, personId, base); + const extra = await insertFace(eventId, personId, other); + + const newPersonId = await clustering.splitPerson(eventId, personId, [extra]); + expect(newPersonId).toBeTruthy(); + + // The snapshot must have been taken AFTER recomputeCentroid — before it, + // the new person has no centroid at all. + const row = await db('event_people_merge_dismissals').where({ event_id: eventId }).first(); + expect(row.centroid_a).toBeTruthy(); + expect(row.centroid_b).toBeTruthy(); + + await simulateRescan(eventId, [base, other]); + expect(await clustering.consolidate(eventId, { thresholds: THRESHOLDS })).toEqual([]); + }); + }); + + describe('when a photo is hard-deleted', () => { + const { purgePhotoFaces } = require('../../src/services/faceProcessor'); + + it('drops the separation when one side has no photos left', async () => { + const eventId = await seedEvent('sep-purge-gone'); + const [a, b] = pairAtSimilarity(0.64, 0); + const idA = await insertPerson(eventId, a); + const idB = await insertPerson(eventId, b); + await insertFace(eventId, idA, a); + const { photoId } = await insertFaceWithPhoto(eventId, idB, b); + + await clustering.dismissMergeSuggestion(eventId, idA, idB); + await purgePhotoFaces(photoId); + + // Person B is gone with its only photo. The row held a COPY of its + // centroid, so leaving it standing would keep a vector derived from a + // deleted photo alive in a table nothing else touches. + expect(await db('event_people').where({ id: idB }).first()).toBeUndefined(); + expect(await db('event_people_merge_dismissals').where({ event_id: eventId })).toHaveLength(0); + }); + + it('keeps the constraint when a side still has another cluster on it', async () => { + const eventId = await seedEvent('sep-purge-descendant'); + const [a, b] = pairAtSimilarity(0.64, 0); + const idA = await insertPerson(eventId, a); + const idB = await insertPerson(eventId, b); + await insertFace(eventId, idA, a); + await clustering.dismissMergeSuggestion(eventId, idA, idB); + + // Re-derivation can leave one stored side represented by more than one + // current person. Deleting the photo behind ONE of them must not throw + // the whole decision away — the other still stands for that side, and the + // pair would be free to merge again. + const twin = new Float32Array(DIM); + for (let i = 0; i < DIM; i++) twin[i] = 0.98 * b[i]; + twin[6] = Math.sqrt(1 - 0.98 ** 2); + const survivor = await insertPerson(eventId, twin); + await insertFace(eventId, survivor, twin); + const { photoId } = await insertFaceWithPhoto(eventId, idB, b); + + const { purgePhotoFaces } = require('../../src/services/faceProcessor'); + await purgePhotoFaces(photoId); + + expect(await db('event_people').where({ id: idB }).first()).toBeUndefined(); + const rows = await db('event_people_merge_dismissals').where({ event_id: eventId }); + expect(rows).toHaveLength(1); + // Re-anchored onto the survivor, so it still binds. + expect(clustering.separationForbids(a, twin, [{ + a: clustering.unpackEmbedding(rows[0].centroid_a), + b: clustering.unpackEmbedding(rows[0].centroid_b), + }])).toBe(true); + }); + + it('re-takes the snapshot from what is left when the person survives', async () => { + const eventId = await seedEvent('sep-purge-survives'); + const [a, b] = pairAtSimilarity(0.64, 0); + const idA = await insertPerson(eventId, a); + const idB = await insertPerson(eventId, b); + await insertFace(eventId, idA, a); + await insertFace(eventId, idB, b); + // A second face on B, close enough that B stays recognisably B — so + // purging it moves B's centroid rather than deleting the person, and the + // side still resolves to B afterwards. + const other = new Float32Array(DIM); + for (let i = 0; i < DIM; i++) other[i] = 0.95 * b[i]; + other[5] = Math.sqrt(1 - 0.95 ** 2); + const { photoId } = await insertFaceWithPhoto(eventId, idB, other); + await clustering.recomputeCentroid(idB); + + await clustering.dismissMergeSuggestion(eventId, idA, idB); + const before = await db('event_people_merge_dismissals').where({ event_id: eventId }).first(); + + await purgePhotoFaces(photoId); + + const after = await db('event_people_merge_dismissals').where({ event_id: eventId }).first(); + expect(after).toBeTruthy(); + expect(Buffer.from(after.centroid_b).equals(Buffer.from(before.centroid_b))).toBe(false); + // It now equals the recomputed centroid — nothing of the deleted face left. + const person = await db('event_people').where({ id: idB }).first(); + expect(Buffer.from(after.centroid_b).equals(Buffer.from(person.centroid))).toBe(true); + }); + }); + + describe('when the photographer changes their mind', () => { + it('a manual merge clears the separation between the merged people', async () => { + const eventId = await seedEvent('sep-merge-overrules'); + const [a, b] = pairAtSimilarity(0.97, 0); + const idA = await insertPerson(eventId, a); + const idB = await insertPerson(eventId, b); + await insertFace(eventId, idA, a); + await insertFace(eventId, idB, b); + + await clustering.dismissMergeSuggestion(eventId, idA, idB); + // ...and then decides they ARE the same person after all. + await clustering.mergePeople(eventId, [idB], idA); + + // The row is keyed on the centroids as well as the ids, so leaving it + // would survive the ids it names: the next recluster would recognise + // those two sides and pull the merge apart again. + expect(await db('event_people_merge_dismissals').where({ event_id: eventId })).toHaveLength(0); + + await simulateRescan(eventId, [a, b]); + expect(await clustering.consolidate(eventId, { thresholds: THRESHOLDS })).toHaveLength(1); + }); + }); + + describe('cleanup after the ids have already died', () => { + // The rows these paths must find are exactly the ones whose person ids no + // longer resolve — that is the state this whole feature creates. Matching + // on ids alone walks past them, which is worse than not cleaning up at + // all: the surviving row still enforces its vectors. + + it('a merge clears a separation that had already outlived its ids', async () => { + const eventId = await seedEvent('sep-merge-stale'); + const [a, b] = pairAtSimilarity(0.97, 0); + const idA = await insertPerson(eventId, a); + const idB = await insertPerson(eventId, b); + await clustering.dismissMergeSuggestion(eventId, idA, idB); + + // A recluster: same vectors, brand-new people. The row now names nobody. + const [newA, newB] = await simulateRescan(eventId, [a, b]); + expect([newA, newB]).not.toContain(idA); + + await clustering.mergePeople(eventId, [newB], newA); + + expect(await db('event_people_merge_dismissals').where({ event_id: eventId })).toHaveLength(0); + // And it stays merged through the next re-derivation. + await simulateRescan(eventId, [a, b]); + expect(await clustering.consolidate(eventId, { thresholds: THRESHOLDS })).toHaveLength(1); + }); + + it('a purge clears a separation that had already outlived its ids', async () => { + const eventId = await seedEvent('sep-purge-stale'); + const [a, b] = pairAtSimilarity(0.64, 0); + const idA = await insertPerson(eventId, a); + const idB = await insertPerson(eventId, b); + await clustering.dismissMergeSuggestion(eventId, idA, idB); + + // Same recluster, then hard-delete the photo behind the B side. + await db('photo_faces').where({ event_id: eventId }).del(); + await db('event_people').where({ event_id: eventId }).del(); + const newA = await insertPerson(eventId, a); + await insertFace(eventId, newA, a); + const newB = await insertPerson(eventId, b); + const { photoId } = await insertFaceWithPhoto(eventId, newB, b); + + const { purgePhotoFaces } = require('../../src/services/faceProcessor'); + await purgePhotoFaces(photoId); + + expect(await db('event_people').where({ id: newB }).first()).toBeUndefined(); + // The row named idA/idB, neither of which exists — but its centroid_b is + // a copy of a vector derived from the photo that was just destroyed. + expect(await db('event_people_merge_dismissals').where({ event_id: eventId })).toHaveLength(0); + }); + }); + + describe('when the whole gallery is deleted', () => { + it('deleteEventCascade clears the separations too', () => { + // Source inspection, deliberately. deleteEventCascade takes an admin + // context and does filesystem cleanup, so driving it here would test the + // scaffolding rather than the contract. The contract is narrow and + // absolute: this table now holds centroid BLOBs, it has no event FK by + // design, and nothing else in the codebase would ever reach it — so the + // one delete has to be in the cascade or the embeddings outlive the + // gallery. Same approach as the contract tests added for #596. + const src = fs.readFileSync( + path.join(__dirname, '..', '..', 'src', 'routes', 'adminEvents', 'helpers.js'), 'utf8' + ); + const body = src.slice(src.indexOf('async function deleteEventCascade')); + expect(body).toContain('event_people_merge_dismissals\').where(\'event_id\', eventId).del()'); + // Guarded, not caught: a failed statement aborts the transaction on PG. + expect(body).toContain('hasTable(\'event_people_merge_dismissals\')'); + }); + + it('permanent archive deletion clears the face data too', () => { + // Same contract, second door. This route deletes the event row directly + // and leans on the FK cascade, which is inert on SQLite — and no FK + // reaches the dismissals table on either engine. archiveEvent's purge is + // nonfatal, so an event really can arrive here still holding embeddings. + const src = fs.readFileSync( + path.join(__dirname, '..', '..', 'src', 'routes', 'adminArchives.js'), 'utf8' + ); + expect(src).toContain('event_people_merge_dismissals'); + expect(src).toContain('db(\'photo_faces\').where(\'event_id\', req.params.id).del()'); + expect(src).toContain('db(\'event_people\').where(\'event_id\', req.params.id).del()'); + }); + }); + + describe('during assignment', () => { + it('will not put a new face into a cluster it was separated from', async () => { + const eventId = await seedEvent('sep-assign'); + const [a, b] = pairAtSimilarity(0.97, 0); + const idA = await insertPerson(eventId, a); + const idB = await insertPerson(eventId, b); + await clustering.dismissMergeSuggestion(eventId, idA, idB); + + // A face that looks like side B arrives. Its nearest centroid is A (0.97, + // far above the 0.6 match threshold), and before #1132 it would simply + // have joined — reforming the pair the photographer pulled apart, because + // assignment consulted no separations at all. + await db('event_people').where({ id: idB }).del(); + const [p] = await db('photos').insert({ + event_id: eventId, filename: 'new.jpg', path: '/tmp/n.jpg', type: 'individual', + }).returning('id'); + const photoId = typeof p === 'object' ? p.id : p; + const [f] = await db('photo_faces').insert({ + photo_id: photoId, event_id: eventId, + bbox_x: 0, bbox_y: 0, bbox_w: 200, bbox_h: 200, det_score: 0.99, + embedding: clustering.packEmbedding(b), model_version: 'test-v1', + created_at: new Date().toISOString(), + }).returning('id'); + const faceId = typeof f === 'object' ? f.id : f; + + const assignments = await clustering.assignFaces( + eventId, [{ id: faceId, embedding: clustering.packEmbedding(b), model_version: 'test-v1', + det_score: 0.99, bbox_w: 200, bbox_h: 200 }], + { thresholds: THRESHOLDS }, + ); + + expect(assignments).toHaveLength(1); + expect(assignments[0].personId).not.toBe(idA); + // It opened its own person rather than being forced into the wrong one. + expect(assignments[0].personId).toBeTruthy(); + }); + + it('holds back a face that is only loosely like the side it belongs to', async () => { + const eventId = await seedEvent('sep-assign-loose'); + // The separated sides are CENTROIDS; an individual face sits well below + // its own centroid — that is why faces join at 0.6 and not at 0.92. A + // face 0.85-like its own side would clear no strict bar against it, and + // before this it walked straight into the other person during a + // recluster, which is the exact merge the photographer undid. + const [sideA, sideB] = pairAtSimilarity(0.7, 0); + const idA = await insertPerson(eventId, sideA); + const idB = await insertPerson(eventId, sideB); + await clustering.dismissMergeSuggestion(eventId, idA, idB); + await db('event_people').where({ id: idB }).del(); + + // 0.65 to side A — above the 0.6 match threshold, so it would join A — + // and 0.85 to side B, which is where it actually belongs. + const face = new Float32Array(DIM); + face[0] = 0.65; face[1] = 0.553; face[2] = Math.sqrt(1 - 0.65 ** 2 - 0.553 ** 2); + + const [p] = await db('photos').insert({ + event_id: eventId, filename: 'loose.jpg', path: '/tmp/l.jpg', type: 'individual', + }).returning('id'); + const [f] = await db('photo_faces').insert({ + photo_id: typeof p === 'object' ? p.id : p, event_id: eventId, + bbox_x: 0, bbox_y: 0, bbox_w: 200, bbox_h: 200, det_score: 0.99, + embedding: clustering.packEmbedding(face), model_version: 'test-v1', + created_at: new Date().toISOString(), + }).returning('id'); + + const assignments = await clustering.assignFaces( + eventId, [{ id: typeof f === 'object' ? f.id : f, embedding: clustering.packEmbedding(face), + model_version: 'test-v1', det_score: 0.99, bbox_w: 200, bbox_h: 200 }], + { thresholds: THRESHOLDS }, + ); + + expect(assignments[0].personId).not.toBe(idA); + expect(assignments[0].personId).toBeTruthy(); + }); + + it('binds while the clusters are still being rebuilt one face at a time', async () => { + const eventId = await seedEvent('sep-assign-rebuild'); + // recluster() empties event_people and re-assigns from scratch, so for + // the first faces of a batch the "person" on the other side of the + // comparison is a cluster of ONE. A settled centroid it is not, and + // holding it to the strict threshold meant the pair was already merged + // by the time the constraint could bind — with nothing left to split it. + const [sideA, sideB] = pairAtSimilarity(0.7, 0); + const idA = await insertPerson(eventId, sideA); + const idB = await insertPerson(eventId, sideB); + await clustering.dismissMergeSuggestion(eventId, idA, idB); + await db('event_people').where({ event_id: eventId }).del(); + + // Two faces, one per side, each a little off its own side's centroid — + // 0.91, just under the strict bar — and 0.66 to each other, over the + // match threshold. Exactly the pair that must not re-form. + const off = Math.sqrt(1 - 0.91 ** 2); + const faceA = new Float32Array(DIM); + faceA[0] = 0.91; faceA[3] = off; + const faceB = new Float32Array(DIM); + faceB[0] = 0.91 * 0.7; faceB[1] = 0.91 * Math.sqrt(1 - 0.7 ** 2); faceB[3] = off; + + const rows = []; + for (const vec of [faceA, faceB]) { + const [p] = await db('photos').insert({ + event_id: eventId, filename: `${Math.random()}.jpg`, path: '/tmp/r.jpg', type: 'individual', + }).returning('id'); + const [f] = await db('photo_faces').insert({ + photo_id: typeof p === 'object' ? p.id : p, event_id: eventId, + bbox_x: 0, bbox_y: 0, bbox_w: 200, bbox_h: 200, det_score: 0.99, + embedding: clustering.packEmbedding(vec), model_version: 'test-v1', + created_at: new Date().toISOString(), + }).returning('id'); + rows.push({ id: typeof f === 'object' ? f.id : f, embedding: clustering.packEmbedding(vec), + model_version: 'test-v1', det_score: 0.99, bbox_w: 200, bbox_h: 200 }); + } + + // The premise: they are close enough to each other to cluster together. + expect(clustering.dot(faceA, faceB)).toBeGreaterThan(THRESHOLDS.face_match_threshold); + + const assignments = await clustering.assignFaces(eventId, rows, { thresholds: THRESHOLDS }); + expect(assignments[0].personId).not.toBe(assignments[1].personId); + }); + + it('leaves ordinary assignment alone when no separation applies', async () => { + const eventId = await seedEvent('sep-assign-clean'); + const base = new Float32Array(DIM); base[0] = 1; + const personId = await insertPerson(eventId, base); + + const [p] = await db('photos').insert({ + event_id: eventId, filename: 'x.jpg', path: '/tmp/x.jpg', type: 'individual', + }).returning('id'); + const photoId = typeof p === 'object' ? p.id : p; + const [f] = await db('photo_faces').insert({ + photo_id: photoId, event_id: eventId, + bbox_x: 0, bbox_y: 0, bbox_w: 200, bbox_h: 200, det_score: 0.99, + embedding: clustering.packEmbedding(base), model_version: 'test-v1', + created_at: new Date().toISOString(), + }).returning('id'); + + const assignments = await clustering.assignFaces( + eventId, [{ id: typeof f === 'object' ? f.id : f, embedding: clustering.packEmbedding(base), + model_version: 'test-v1', det_score: 0.99, bbox_w: 200, bbox_h: 200 }], + { thresholds: THRESHOLDS }, + ); + + // The whole point of the strict threshold: a constraint that fires when + // it should not would quietly wreck ordinary clustering. + expect(assignments[0].personId).toBe(personId); + }); + }); +}); diff --git a/backend/migrations/core/184_separations_keyed_on_embeddings.js b/backend/migrations/core/184_separations_keyed_on_embeddings.js new file mode 100644 index 00000000..21f73d13 --- /dev/null +++ b/backend/migrations/core/184_separations_keyed_on_embeddings.js @@ -0,0 +1,97 @@ +/** + * Make "not the same person" survive a re-scan (#1132). + * + * A separation — an explicit dismissal, or the implicit one a Split records — + * was stored as a pair of `event_people.id`. Those ids do not survive + * re-derivation, and the two paths differ in an important way: + * + * - `recluster()` deletes every person and re-assigns, so person ids die but + * `photo_faces.id` survives. + * - a full re-scan replaces a photo's faces entirely + * (`processPhotoFaces` deletes and re-inserts), so FACE ids die too. + * + * So neither person ids nor face ids are stable enough. The only thing that + * survives both is the embedding: the same photo through the same model + * produces the same vector. The separation is therefore keyed on the two + * CENTROIDS the pair had when the photographer separated them. + * + * That also answers the question the issue left open — what a separation means + * once its two sides have been split further or merged with a third cluster. + * It applies while both sides still LOOK like the clusters that were separated, + * and stops applying once they have drifted past recognition. A constraint on a + * pair that no longer exists should lapse, and this makes that automatic rather + * than a rule someone has to write. + * + * The person ids stay as a fast exact path within one clustering cycle. They + * are cheaper and unambiguous while they are still valid; the vectors are what + * carries the decision across a re-derivation. + * + * Backfill takes each row's current person centroids. A row whose people are + * already gone is dropped — it was dangling, and there is nothing to preserve. + */ + +exports.up = async function up(knex) { + if (!(await knex.schema.hasTable('event_people_merge_dismissals'))) return; + + // Each column guarded on its own. SQLite runs migrations outside a + // transaction and knex emits one ALTER per column, so a run that dies after + // the first would leave the table half-built — and a guard keyed on + // centroid_a alone would then skip the other two forever, with the backfill + // below failing on every retry. Not recoverable without hand-editing the + // schema, which is not something a deployment should ever need. + const addColumn = async (name, build) => { + if (await knex.schema.hasColumn('event_people_merge_dismissals', name)) return; + await knex.schema.alterTable('event_people_merge_dismissals', build); + }; + + await addColumn('centroid_a', (table) => table.binary('centroid_a')); + await addColumn('centroid_b', (table) => table.binary('centroid_b')); + // Which embedding space the vectors live in. A model change makes them + // meaningless rather than merely stale, exactly as it does for + // event_people.centroid. + await addColumn('model_version', (table) => table.string('model_version', 64)); + + // Backfill from the people the rows point at, while they still resolve. + const rows = await knex('event_people_merge_dismissals') + .whereNull('centroid_a') + .select('id', 'event_id', 'person_a_id', 'person_b_id'); + if (!rows.length) return; + + let filled = 0; + let dropped = 0; + for (const row of rows) { + const people = await knex('event_people') + .whereIn('id', [row.person_a_id, row.person_b_id]) + .select('id', 'centroid', 'model_version'); + const a = people.find((p) => p.id === row.person_a_id); + const b = people.find((p) => p.id === row.person_b_id); + + if (!a?.centroid || !b?.centroid) { + // Dangling already — the pair it named is gone, so there is no decision + // left to carry forward. + await knex('event_people_merge_dismissals').where({ id: row.id }).del(); + dropped += 1; + continue; + } + + await knex('event_people_merge_dismissals').where({ id: row.id }).update({ + centroid_a: a.centroid, + centroid_b: b.centroid, + model_version: a.model_version || b.model_version || null, + }); + filled += 1; + } + + console.log(` 184: keyed ${filled} separation(s) on embeddings, dropped ${dropped} dangling`); +}; + +exports.down = async function down(knex) { + if (!(await knex.schema.hasTable('event_people_merge_dismissals'))) return; + // Dropped one at a time for the same reason they are added one at a time. + for (const name of ['centroid_a', 'centroid_b', 'model_version']) { + if (!(await knex.schema.hasColumn('event_people_merge_dismissals', name))) continue; + await knex.schema.alterTable('event_people_merge_dismissals', (table) => { + table.dropColumn(name); + }); + } +}; diff --git a/backend/src/routes/adminArchives.js b/backend/src/routes/adminArchives.js index 8734917e..938461c4 100644 --- a/backend/src/routes/adminArchives.js +++ b/backend/src/routes/adminArchives.js @@ -434,6 +434,19 @@ router.delete('/:id', adminAuth, requirePermission('archives.delete'), requireEv } } + // Face data (#1074, #1132). This route deletes the event row directly and + // relies on the FK cascade, but SQLite only honours ON DELETE CASCADE with + // `PRAGMA foreign_keys = ON`, which PicPeak does not set — and + // event_people_merge_dismissals has no event FK at all, on either engine. + // archiveEvent's purge step is deliberately nonfatal, so an event can + // still be carrying face data when it reaches this permanent delete. + // Delete explicitly, the same way deleteEventCascade does. + await db('photo_faces').where('event_id', req.params.id).del(); + await db('event_people').where('event_id', req.params.id).del(); + if (await db.schema.hasTable('event_people_merge_dismissals')) { + await db('event_people_merge_dismissals').where('event_id', req.params.id).del(); + } + // Delete from database (cascade will delete photos and logs) await db('events').where('id', req.params.id).delete(); diff --git a/backend/src/routes/adminEvents/helpers.js b/backend/src/routes/adminEvents/helpers.js index 674129b0..532aeb97 100644 --- a/backend/src/routes/adminEvents/helpers.js +++ b/backend/src/routes/adminEvents/helpers.js @@ -249,6 +249,14 @@ async function deleteEventCascade(eventId, adminContext) { // photos, so the guarantee holds on both engines. await trx('photo_faces').where('event_id', eventId).del(); await trx('event_people').where('event_id', eventId).del(); + // Separations carry a COPY of each side's centroid since #1132, so this + // table holds biometric data too — and it deliberately has no event FK, so + // nothing else would ever reach it. hasTable rather than a catch: a failed + // statement aborts the surrounding transaction on Postgres, which would + // take the whole delete down on a pre-migration install. + if (await trx.schema.hasTable('event_people_merge_dismissals')) { + await trx('event_people_merge_dismissals').where('event_id', eventId).del(); + } await trx('photos').where('event_id', eventId).del(); // 5. Finally delete the event row diff --git a/backend/src/services/faceClustering.js b/backend/src/services/faceClustering.js index 73e238b0..8dd727b7 100644 --- a/backend/src/services/faceClustering.js +++ b/backend/src/services/faceClustering.js @@ -171,6 +171,34 @@ async function assignFacesLocked(eventId, faceRows, thresholds, trx) { modelVersion: p.model_version, })).filter((p) => p.centroid); + // Loaded once for the whole batch rather than per face: a photo with five + // faces would otherwise re-read the same handful of rows five times, and a + // backfill does that for every photo in the gallery. + // Through the caller's trx, not the global db: assignFaces runs inside + // processPhotoFaces' transaction, and on SQLite a second connection reading + // while that write transaction is open can block on the writer lock. + const { vectors: allSeparations } = await loadSeparations(eventId, trx); + + // Narrow to the separations this batch could possibly trip, before anything + // is projected. A separation binds only if the incoming FACE resolves to one + // of its sides, so if no face in the batch clears the threshold against + // either side, that separation cannot fire here — dropping it is exactly + // equivalent, not an approximation. + // + // This is what keeps a full scan affordable. assignFaces runs once per PHOTO, + // so projecting every person against every separation on every call would + // reintroduce the O(photos·people·separations·dims) cost the projection hoist + // just removed. One pass over the handful of vectors in this photo decides + // whether the event's people need projecting at all, and for almost every + // photo the answer is no. + const batchVectors = faceRows.map((f) => unpackEmbedding(f.embedding)).filter(Boolean); + const separations = allSeparations.filter((sep) => batchVectors.some( + (v) => dot(v, sep.a) >= thresholds.face_match_threshold + || dot(v, sep.b) >= thresholds.face_match_threshold, + )); + + for (const person of state) person.sep = projectOnSeparations(person.centroid, separations); + const assignments = []; for (const face of faceRows) { @@ -182,6 +210,8 @@ async function assignFacesLocked(eventId, faceRows, thresholds, trx) { continue; } + const faceSep = projectOnSeparations(embedding, separations); + let best = null; let bestScore = -Infinity; for (const person of state) { @@ -190,6 +220,34 @@ async function assignFacesLocked(eventId, faceRows, thresholds, trx) { if (person.modelVersion && face.model_version && person.modelVersion !== face.model_version) { continue; } + // Honour the photographer's separations here, not only in consolidate() + // (#1132). Without this the constraint was toothless across a re-scan: + // the faces come back with new ids, assignment puts them wherever the + // maths says, and the pair the photographer pulled apart is reformed + // before any later pass gets to object — a dismissed pair scores at + // least face_match_threshold by definition, that being the bottom of the + // suggestion band. + // + // Skipping a candidate can push a face to its SECOND-nearest centroid, + // or open a new person. That is the intended cost: a human said these + // are different people, and the alternative is silently overruling them. + // The threshold is strict enough that this only fires while the cluster + // still looks like the one that was separated. + if (separationForbidsProjected(faceSep, person.sep, { + modelVersion: face.model_version || person.modelVersion || null, + // BOTH sides get the ordinary match threshold here, not the strict + // one. Neither side of this comparison is a settled centroid: the + // candidate is a single face, and during a recluster the "person" it + // is being compared against is often a cluster of one — state starts + // empty and is rebuilt face by face. Holding either to 0.92 meant the + // constraint could not bind until well after the merge it was supposed + // to prevent had already happened. consolidate() and suggestMerges() + // keep the strict threshold, because there both sides really are + // established centroids. + threshold: thresholds.face_match_threshold, + })) { + continue; + } const score = dot(embedding, person.centroid); if (score > bestScore) { bestScore = score; @@ -201,6 +259,9 @@ async function assignFacesLocked(eventId, faceRows, thresholds, trx) { best.centroid = updateCentroid(best.centroid, best.count, embedding); best.count += 1; best.dirty = true; + // The centroid just moved, so its projection is stale — one re-project + // per ASSIGNED face, not per comparison. + best.sep = projectOnSeparations(best.centroid, separations); assignments.push({ faceId: face.id, personId: best.id }); } else { const [inserted] = await trx('event_people').insert({ @@ -226,6 +287,7 @@ async function assignFacesLocked(eventId, faceRows, thresholds, trx) { count: 1, dirty: false, modelVersion: face.model_version, + sep: faceSep, }); assignments.push({ faceId: face.id, personId }); } @@ -270,19 +332,147 @@ function pairKey(a, b) { } /** - * Pairs the photographer has explicitly kept apart (#1107). + * How closely a cluster must still resemble a separated side for the + * separation to bind it (#1132). + * + * Deliberately stricter than the auto-merge threshold. This is not asking "are + * these the same person" but "is this still the same CLUSTER the photographer + * pointed at" — a much narrower claim, and one that should lapse once the + * cluster has been reshaped enough that the original decision may no longer + * reflect what is in it. + */ +const SEPARATION_MATCH_THRESHOLD = 0.92; + +/** + * One vector's similarity to both sides of every separation. + * + * Hoisted out of the comparison so the maths is paid per VECTOR, not per pair. + * separationForbids is called from the innermost loop of assignment (every face + * against every person) and of consolidation (every person against every other) + * — projecting inside it made those O(F·P·S·D) and O(P²·S·D), which on a + * gallery with a few hundred people and a handful of dismissals is billions of + * float operations per re-scan. Projecting once per vector makes it + * O((F+P)·S·D) of arithmetic plus O(S) scalar comparisons per pair, which is + * the same order as the clustering it is guarding. + */ +function projectOnSeparations(vec, separations) { + if (!vec || !separations?.length) return []; + const out = []; + for (const sep of separations) { + if (!sep.a || !sep.b) continue; + out.push({ a: dot(vec, sep.a), b: dot(vec, sep.b), modelVersion: sep.modelVersion || null }); + } + return out; +} + +/** + * Does a stored separation forbid putting these two vectors together? + * + * A separation binds when each of its two sides still matches one of the + * candidates — in either orientation, since neither the stored pair nor the + * candidate pair has a meaningful order. + * + * `modelVersion` is the space the two candidates live in; every caller has + * already refused to compare across spaces before reaching here. A separation + * recorded under a different model is skipped for the same reason: its vectors + * are meaningless there, not merely stale. + * + * `threshold` is how closely a side must still be matched. It is the caller's + * because it depends on what is being compared: settled centroids can be held + * to SEPARATION_MATCH_THRESHOLD, whereas a single face — or a cluster of one + * part-way through a recluster — sits well below its own centroid and has to be + * judged at the ordinary match threshold. Holding those to 0.92 let exactly the + * case this feature exists for through. + * + * Takes PROJECTIONS, not vectors — see projectOnSeparations. The two arrays are + * positionally aligned because both come from the same separation list. + */ +function separationForbidsProjected(projX, projY, options = {}) { + const { modelVersion = null, threshold = SEPARATION_MATCH_THRESHOLD } = options; + if (!projX?.length || !projY?.length) return false; + + for (let i = 0; i < projX.length; i++) { + const px = projX[i]; + const py = projY[i]; + if (!px || !py) continue; + if (modelVersion && px.modelVersion && px.modelVersion !== modelVersion) continue; + + const xa = px.a; + const xb = px.b; + const ya = py.a; + const yb = py.b; + + // Each candidate must resolve to ONE side and the other candidate to the + // OTHER. Clearing the bar against both sides independently is not enough, + // and getting that wrong is not a corner case: a split leaves two halves + // that came out of one cluster, so its two stored sides are often similar + // to each other. Under the looser test, two faces that are plainly the + // SAME side each cleared the threshold against both — so the person the + // split was not even about got forbidden from clustering with itself and + // fragmented into singletons. + // + // Requiring a strict preference also makes the constraint lapse exactly + // when it becomes meaningless: if the two stored sides are so alike that a + // candidate cannot be told apart between them, there is no "these two" left + // to enforce. + const straight = xa >= threshold && yb >= threshold && xa > xb && yb > ya; + const crossed = xb >= threshold && ya >= threshold && xb > xa && ya > yb; + if (straight || crossed) return true; + } + return false; +} + +/** + * Vector-level convenience wrapper. Fine for one-off checks; the hot loops + * project once and call separationForbidsProjected directly. + */ +function separationForbids(vecX, vecY, separations, options = {}) { + if (!vecX || !vecY) return false; + return separationForbidsProjected( + projectOnSeparations(vecX, separations), + projectOnSeparations(vecY, separations), + options, + ); +} + +/** + * Pairs the photographer has explicitly kept apart (#1107, #1132). * * Read by BOTH the automatic pass and the suggestion list: "not the same * person" has to bind the thing that acts on its own even more than it binds * the thing that asks. Missing table (pre-migration) reads as "nothing * dismissed" rather than failing the merge that called this. + * + * Returns both identities. `ids` is the exact person-id pair set, valid only + * while those ids still mean what they meant — cheap and unambiguous within one + * clustering cycle. `vectors` is what survives re-derivation: person ids die on + * a recluster and face ids die on a full re-scan, so the embeddings are the only + * stable handle on "these two". + * + * Every row is returned carrying its own model_version rather than the load + * being filtered to one. recluster() hands the whole event to assignFaces in a + * single batch, and a partial upgrade or a failed re-scan can leave faces from + * two models in it — filtering on the first face's model would silently drop + * every separation belonging to the others. */ -async function loadDismissedPairs(eventId) { +async function loadSeparations(eventId, conn = db) { try { - const rows = await db('event_people_merge_dismissals') + const rows = await conn('event_people_merge_dismissals') .where({ event_id: eventId }) - .select('person_a_id', 'person_b_id'); - return new Set(rows.map((d) => pairKey(d.person_a_id, d.person_b_id))); + .select('person_a_id', 'person_b_id', 'centroid_a', 'centroid_b', 'model_version'); + + const ids = new Set(); + const vectors = []; + for (const row of rows) { + ids.add(pairKey(row.person_a_id, row.person_b_id)); + // Never compare across embedding spaces — a model change makes a stored + // centroid meaningless rather than merely stale, the same rule assignment + // and consolidation already apply to event_people.centroid. + const a = unpackEmbedding(row.centroid_a); + const b = unpackEmbedding(row.centroid_b); + if (a && b) vectors.push({ a, b, modelVersion: row.model_version || null }); + } + return { ids, vectors }; } catch (err) { // ONLY a missing table reads as "nothing dismissed" — that is a // pre-migration install, where by definition nothing has been dismissed. @@ -297,7 +487,7 @@ async function loadDismissedPairs(eventId) { `faceClustering: merge-dismissal table absent for event ${eventId} — treating as none`, { error: err.message } ); - return new Set(); + return { ids: new Set(), vectors: [] }; } } @@ -333,7 +523,8 @@ async function consolidate(eventId, options = {}) { // however far the centroids drift afterwards. Without this the automatic // pass silently overturns an explicit human decision the moment new faces // push the pair over the threshold — or the moment someone tunes it. - const dismissed = await loadDismissedPairs(eventId); + const separations = await loadSeparations(eventId); + for (const p of state) p.sep = projectOnSeparations(p.vec, separations.vectors); const merged = []; const absorbed = new Set(); @@ -357,15 +548,20 @@ async function consolidate(eventId, options = {}) { // to overrule. if (a.label && b.label && a.label !== b.label) continue; - if (dismissed.has(pairKey(a.id, b.id))) continue; + // Both identities: the exact pair while the ids still mean what they + // meant, and the embedding match that carries the decision across a + // re-derivation (#1132). + if (separations.ids.has(pairKey(a.id, b.id))) continue; + if (separationForbidsProjected(a.sep, b.sep, + { modelVersion: a.model_version || null })) continue; if (dot(a.vec, b.vec) >= mergeThreshold) { - // Re-read this one pair immediately before acting. The set above was - // loaded once for the whole pass, and a photographer pressing "Not the - // same" during it would otherwise be overruled by a decision that was - // already stale when it was made. Narrows the window to a single - // statement rather than the length of the pass; one extra query per - // pair that is actually about to merge, which is rare. + // Re-read this one pair immediately before acting. The set above was + // loaded once for the whole pass, and a photographer pressing "Not + // the same" during it would otherwise be overruled by a decision that + // was already stale when it was made. Narrows the window to a single + // statement rather than the length of the pass; one extra query per + // pair that is actually about to merge, which is rare. const justDismissed = await db('event_people_merge_dismissals') .where({ event_id: eventId, @@ -442,7 +638,8 @@ async function suggestMerges(eventId, options = {}) { if (state.length < 2) return []; - const dismissed = await loadDismissedPairs(eventId); + const separations = await loadSeparations(eventId); + for (const p of state) p.sep = projectOnSeparations(p.vec, separations.vectors); const pairs = []; for (let i = 0; i < state.length; i++) { @@ -454,7 +651,11 @@ async function suggestMerges(eventId, options = {}) { // assertion, not a question. if (a.label && b.label && a.label !== b.label) continue; - if (dismissed.has(pairKey(a.id, b.id))) continue; + // Exact pair while the ids still hold, plus the embedding match that + // carries the decision across a re-derivation (#1132). + if (separations.ids.has(pairKey(a.id, b.id))) continue; + if (separationForbidsProjected(a.sep, b.sep, + { modelVersion: a.model_version || null })) continue; const score = dot(a.vec, b.vec); if (score < floor || score >= mergeThreshold) continue; @@ -503,6 +704,117 @@ function isUniqueViolation(err) { return false; } +/** + * The centroids of two people, packed for storage on a separation row (#1132). + * + * Best-effort: a pre-migration install has no columns to write, and a person + * that vanished between the click and this read leaves the row keyed on ids + * alone — which is exactly what it was before, so the decision is no worse off + * than it used to be. + */ +async function separationSnapshot(eventId, personAId, personBId, conn = db) { + try { + const people = await conn('event_people') + .where({ event_id: eventId }) + .whereIn('id', [personAId, personBId]) + .select('id', 'centroid', 'model_version'); + const a = people.find((p) => p.id === personAId); + const b = people.find((p) => p.id === personBId); + if (!a?.centroid || !b?.centroid) return {}; + return { + centroid_a: a.centroid, + centroid_b: b.centroid, + model_version: a.model_version || b.model_version || null, + }; + } catch (err) { + logger.warn(`faceClustering: could not snapshot separation ${personAId}/${personBId}`, { + error: err.message, + }); + return {}; + } +} + +/** + * Re-anchor every separation in an event onto the people that are actually + * there, and drop the ones that no longer describe anything (#1132). + * + * Called after faces are destroyed — a hard photo delete recomputes or removes + * whole clusters. Two things have to happen, and neither can be decided from + * the person ids: a row that has already outlived a recluster names people who + * no longer exist, which is the normal state for this table rather than an + * exceptional one. + * + * 1. Each side is re-snapshotted from the live cluster that best matches it. + * The stored vector is a COPY of a centroid, so after a purge it is the + * one place a vector derived from the deleted photo would survive. Taking + * it from a live centroid means it only ever describes photos that are + * still here. + * + * 2. A side that matches NO live person means the row is inert — the + * constraint requires a candidate to match a side, so nothing can ever + * trip it again — and it is deleted. That is what bounds retention: an + * abandoned snapshot cannot sit in this table indefinitely, and nothing + * is lost by removing something that could never fire. + * + * Deliberately not keyed on which people the purge touched. A cluster can drift + * below the match threshold while still holding the deleted photo, and one + * stored side can be represented by several current people — checking the whole + * event sidesteps both, and it is a few hundred dot products on a path that + * runs when a photo is destroyed. + * + * Best-effort: failing to tidy a dismissal row must not fail the delete. + */ +async function refreshSeparationSnapshots(eventId, conn = db) { + if (!eventId) return; + try { + const rows = await conn('event_people_merge_dismissals') + .where({ event_id: eventId }) + .select('id', 'centroid_a', 'centroid_b'); + if (!rows.length) return; + + const live = (await conn('event_people') + .where({ event_id: eventId }) + .select('id', 'centroid')) + .map((p) => ({ id: p.id, vec: unpackEmbedding(p.centroid) })) + .filter((p) => p.vec); + + /** The live cluster this stored side still describes, if any. */ + const bestMatch = (vec) => { + if (!vec) return null; + let best = null; + let bestScore = SEPARATION_MATCH_THRESHOLD; + for (const person of live) { + const score = dot(vec, person.vec); + if (score >= bestScore) { bestScore = score; best = person; } + } + return best; + }; + + for (const row of rows) { + const matchA = bestMatch(unpackEmbedding(row.centroid_a)); + const matchB = bestMatch(unpackEmbedding(row.centroid_b)); + + // Both sides resolving to the SAME live cluster is the other way a row + // stops describing anything: there are no longer two things here to keep + // apart, so there is nothing left to enforce and no reason to keep the + // vectors. + if (!matchA || !matchB || matchA.id === matchB.id) { + await conn('event_people_merge_dismissals').where({ id: row.id }).del(); + continue; + } + await conn('event_people_merge_dismissals').where({ id: row.id }).update({ + centroid_a: packEmbedding(matchA.vec), + centroid_b: packEmbedding(matchB.vec), + }); + } + } catch (err) { + if (isMissingTable(err)) return; + logger.warn(`faceClustering: could not refresh separations for event ${eventId}`, { + error: err.message, + }); + } +} + /** * Remember that these two are NOT the same person, so the pair stops being * suggested. Normalized to (lower id, higher id) so the pair has one identity. @@ -510,11 +822,16 @@ function isUniqueViolation(err) { async function dismissMergeSuggestion(eventId, personAId, personBId) { const lo = Math.min(personAId, personBId); const hi = Math.max(personAId, personBId); + // Snapshot what the two clusters look like RIGHT NOW (#1132). The person ids + // stop meaning anything the next time clustering is re-derived; these vectors + // are what lets the decision outlive that. + const snapshot = await separationSnapshot(eventId, lo, hi); try { await db('event_people_merge_dismissals').insert({ event_id: eventId, person_a_id: lo, person_b_id: hi, + ...snapshot, created_at: new Date().toISOString(), }); } catch (err) { @@ -544,7 +861,7 @@ async function mergePeople(eventId, sourceIds, targetId) { // has them, and inherits from a source only where it does not. const target = await trx('event_people').where({ id: targetId }).first(); const sources = await trx('event_people').whereIn('id', ids) - .select('label', 'is_hidden', 'is_ignored', 'cover_face_id'); + .select('centroid', 'label', 'is_hidden', 'is_ignored', 'cover_face_id'); const inherited = {}; if (target && !target.label) { @@ -571,6 +888,49 @@ async function mergePeople(eventId, sourceIds, targetId) { await trx('event_people').where({ event_id: eventId }).whereIn('id', ids).del(); + // A separation between people who are now being merged is a decision the + // photographer has just reversed, and the newer decision wins. Leaving the + // row would be worse than untidy since #1132: it is keyed on the centroids + // too, so it outlives the ids it names and the next recluster would + // recognise those sides and pull the merge apart again — silently undoing + // an explicit human action. + // + // Matched the same way the constraint is enforced, not by id. A row that + // has already survived a recluster names people who are gone, and an + // id-only delete walks straight past precisely those rows — the ones with + // a live vector-keyed constraint still in them. hasTable rather than a + // catch, because a failed statement aborts the transaction on Postgres. + const mergedIds = [targetId, ...ids]; + if (await trx.schema.hasTable('event_people_merge_dismissals')) { + await trx('event_people_merge_dismissals') + .where({ event_id: eventId }) + .whereIn('person_a_id', mergedIds) + .whereIn('person_b_id', mergedIds) + .del(); + + const mergedVecs = [target, ...sources] + .map((p) => unpackEmbedding(p?.centroid)).filter(Boolean); + if (mergedVecs.length > 1) { + const rows = await trx('event_people_merge_dismissals') + .where({ event_id: eventId }) + .select('id', 'centroid_a', 'centroid_b'); + for (const row of rows) { + const sep = [{ + a: unpackEmbedding(row.centroid_a), + b: unpackEmbedding(row.centroid_b), + }]; + if (!sep[0].a || !sep[0].b) continue; + // Would this row have forbidden the merge that was just performed? + // Then it is the decision being reversed. + const forbids = mergedVecs.some((x, i) => mergedVecs.slice(i + 1) + .some((y) => separationForbids(x, y, sep))); + if (forbids) { + await trx('event_people_merge_dismissals').where({ id: row.id }).del(); + } + } + } + } + if (Object.keys(inherited).length) { await trx('event_people').where({ id: targetId }) .update({ ...inherited, updated_at: new Date().toISOString() }); @@ -608,24 +968,37 @@ async function splitPerson(eventId, personId, faceIds) { .whereIn('id', faces.map((f) => f.id)) .update({ person_id: newPersonId }); + // Centroids FIRST, then the separation. The order is load-bearing (#1132): + // at this point the new person has no centroid at all (it was inserted with + // none) and the original still carries the faces being split out, so a + // snapshot taken here would record one empty vector and one stale one — + // and the separation would bind the wrong pair, or nothing at all. + await recomputeCentroid(newPersonId, trx); + await recomputeCentroid(personId, trx); + // A split IS a "these are not the same person" decision, and it has to be - // recorded as one (#1107). Consolidation now runs automatically after - // every scan, and two clusters a photographer pulled apart are look-alikes - // by construction — their centroids usually sit above the merge threshold, - // so the very next scan would put them straight back together and the - // manual correction would look like it never happened. + // recorded as one (#1107). Consolidation runs automatically after every + // scan, and two clusters a photographer pulled apart are look-alikes by + // construction — their centroids usually sit above the merge threshold, so + // the very next scan would put them straight back together and the manual + // correction would look like it never happened. + // + // Keyed on the post-split centroids as well as the ids, so it also survives + // the re-derivation that kills both person and face ids (#1132). + const lo = Math.min(personId, newPersonId); + const hi = Math.max(personId, newPersonId); + const snapshot = await separationSnapshot(eventId, lo, hi, trx); await trx('event_people_merge_dismissals').insert({ event_id: eventId, - person_a_id: Math.min(personId, newPersonId), - person_b_id: Math.max(personId, newPersonId), + person_a_id: lo, + person_b_id: hi, + ...snapshot, created_at: new Date().toISOString(), }).catch((err) => { // Pre-migration install, or the pair was already separated once before. if (!isMissingTable(err) && !isUniqueViolation(err)) throw err; }); - await recomputeCentroid(newPersonId, trx); - await recomputeCentroid(personId, trx); return newPersonId; }); } @@ -807,6 +1180,14 @@ module.exports = { // must stop the pass. isUniqueViolation, isMissingTable, + // Separation constraints (#1132) — exported so the tests can drive the + // matching directly rather than only through a full clustering pass. + loadSeparations, + separationForbids, + separationForbidsProjected, + projectOnSeparations, + refreshSeparationSnapshots, + SEPARATION_MATCH_THRESHOLD, mergePeople, splitPerson, recomputeCentroid, diff --git a/backend/src/services/faceProcessor.js b/backend/src/services/faceProcessor.js index ee3f5c7e..53650498 100644 --- a/backend/src/services/faceProcessor.js +++ b/backend/src/services/faceProcessor.js @@ -402,11 +402,13 @@ async function purgeEvent(eventId) { * Safe to call for photos that were never scanned: it simply deletes nothing. */ async function purgePhotoFaces(photoId, trx = db) { - const affected = await trx('photo_faces') + const affectedRows = await trx('photo_faces') .where({ photo_id: photoId }) .whereNotNull('person_id') - .distinct('person_id') - .pluck('person_id'); + .distinct('person_id', 'event_id') + .select('person_id', 'event_id'); + const affected = [...new Set(affectedRows.map((r) => r.person_id))]; + const eventId = affectedRows[0]?.event_id; // Drop any in-flight claim as well. A worker holding this photo would // otherwise still satisfy its `face_status = 'processing'` commit guard and @@ -420,11 +422,19 @@ async function purgePhotoFaces(photoId, trx = db) { const removed = await trx('photo_faces').where({ photo_id: photoId }).del(); if (!removed) return { removed: 0 }; - const { recomputeCentroid: recompute } = require('./faceClustering'); + const { recomputeCentroid: recompute, refreshSeparationSnapshots } = require('./faceClustering'); for (const personId of affected) { // Deletes the person outright when it has no members left. await recompute(personId, trx); } + + // A separation row carries a COPY of each side's centroid (#1132), derived + // from the faces that person held — including the ones just deleted. Re-take + // each side from the live cluster it still describes, and drop the row if it + // describes nothing that is left. Otherwise deleting a photo would leave a + // vector built from it standing in a table nothing else touches. + await refreshSeparationSnapshots(eventId, trx); + return { removed, peopleTouched: affected.length }; }