From c82c27baf53b79de2129bcd9eb0a8df05fa07971 Mon Sep 17 00:00:00 2001 From: ik020 Date: Sat, 5 Sep 2026 13:54:38 +0500 Subject: [PATCH 1/2] Add reviewable person identities (#85) - PersonRecord/PersonAlias/PersonReference/PersonClusterLink domain models - CRUD layer in sql_catalog.py, schema version bumped to 5 - Alembic migration 20260803_01 for existing/Postgres catalogs - Regression tests covering reindex-survival and non-destructive delete - Update test_database_cli.py and test_media_catalog.py for new head/schema version - Fix database_cli.py to ensure local directories exist before migrating --- src/vidxp/core/identifiers.py | 2 + src/vidxp/core/people.py | 121 ++++++++ src/vidxp/infrastructure/local_catalog.py | 12 +- src/vidxp/infrastructure/sql_catalog.py | 238 ++++++++++++++ src/vidxp/infrastructure/sql_tables.py | 60 ++++ .../versions/20260803_01_reviewed_people.py | 97 ++++++ tests/test_database_cli.py | 4 +- tests/test_media_catalog.py | 6 +- tests/test_people_catalog.py | 292 ++++++++++++++++++ 9 files changed, 827 insertions(+), 5 deletions(-) create mode 100644 src/vidxp/core/people.py create mode 100644 src/vidxp/migrations/versions/20260803_01_reviewed_people.py create mode 100644 tests/test_people_catalog.py diff --git a/src/vidxp/core/identifiers.py b/src/vidxp/core/identifiers.py index 4c0ee601..8908ef04 100644 --- a/src/vidxp/core/identifiers.py +++ b/src/vidxp/core/identifiers.py @@ -79,3 +79,5 @@ def _require_workflow_uuid(value: str) -> str: ArtifactId: TypeAlias = Uuid4Hex UploadIntentId: TypeAlias = Uuid4Hex UploadSessionId: TypeAlias = Uuid4Hex +PersonId: TypeAlias = Uuid4Hex +PersonReferenceId: TypeAlias = Uuid4Hex diff --git a/src/vidxp/core/people.py b/src/vidxp/core/people.py new file mode 100644 index 00000000..94d61a7b --- /dev/null +++ b/src/vidxp/core/people.py @@ -0,0 +1,121 @@ +from __future__ import annotations + +from pathlib import Path +from typing import Literal + +from pydantic import ( + AwareDatetime, + BaseModel, + ConfigDict, + Field, + field_validator, +) + +from vidxp.core.identifiers import ( + ActorClusterId, + IndexGenerationId, + MediaId, + MimeType, + PersonId, + PersonReferenceId, + Sha256, +) +from vidxp.core.storage_keys import validate_storage_key + + +PEOPLE_SCHEMA_VERSION = 1 + + +class _PersonModel(BaseModel): + model_config = ConfigDict( + extra="forbid", + frozen=True, + allow_inf_nan=False, + strict=True, + ) + + +class PersonRecord(_PersonModel): + """A repository-specific, user-reviewed person identity. + + A PersonRecord is independent of any single index generation or + actor cluster: it represents a durable, user-approved identity + that survives re-indexing (see PersonClusterLink for the evidence + that ties it to specific anonymous face clusters). + """ + + schema_version: Literal[PEOPLE_SCHEMA_VERSION] = PEOPLE_SCHEMA_VERSION + person_id: PersonId + display_name: str = Field(min_length=1, max_length=255) + notes: str | None = Field(default=None, max_length=10_000) + biography: str | None = Field(default=None, max_length=10_000) + created_at: AwareDatetime + + +class PersonAlias(_PersonModel): + """An alternate name a user has attached to a reviewed person.""" + + person_id: PersonId + alias: str = Field(min_length=1, max_length=255) + + +class StagedPersonReference(_PersonModel): + """A reference image written to a temporary path, not yet published.""" + + reference_id: PersonReferenceId + person_id: PersonId + path: Path + + +class StoredPersonReference(_PersonModel): + """A reference image that has been published into managed storage.""" + + sha256: Sha256 + byte_size: int = Field(gt=0) + storage_key: str = Field(min_length=1) + local_path: Path + + @field_validator("storage_key") + @classmethod + def _validate_storage_key(cls, value: str) -> str: + return validate_storage_key(value) + + +class PersonReference(_PersonModel): + """A user-supplied reference image attached to a reviewed person. + + Deliberately independent of ArtifactRecord: artifacts are + job-generated, generation-scoped, and may expire, while a + reference image is durable, user-supplied, and outlives any + single index generation. + """ + + reference_id: PersonReferenceId + person_id: PersonId + storage_key: str = Field(min_length=1) + sha256: Sha256 + byte_size: int = Field(gt=0) + mime_type: MimeType + created_at: AwareDatetime + + @field_validator("storage_key") + @classmethod + def _validate_storage_key(cls, value: str) -> str: + return validate_storage_key(value) + + +class PersonClusterLink(_PersonModel): + """Evidence linking a reviewed person to an anonymous actor cluster. + + The cluster_id is scoped to one media item and one index + generation: it is evidence, not identity. A person may accumulate + multiple links across different videos and across re-indexing + generations. Removing a link (to correct an accidental merge or + split) never touches the underlying video, media, or index data. + """ + + person_id: PersonId + cluster_id: ActorClusterId + media_id: MediaId + generation_id: IndexGenerationId + created_at: AwareDatetime diff --git a/src/vidxp/infrastructure/local_catalog.py b/src/vidxp/infrastructure/local_catalog.py index 1f66ac88..a857414d 100644 --- a/src/vidxp/infrastructure/local_catalog.py +++ b/src/vidxp/infrastructure/local_catalog.py @@ -11,13 +11,17 @@ media, media_import_requests, metadata, + people, + person_aliases, + person_cluster_links, + person_references, upload_intents, upload_quota, upload_session_files, upload_sessions, ) -CATALOG_SCHEMA_VERSION = 4 +CATALOG_SCHEMA_VERSION = 5 _local_metadata = MetaData() catalog_metadata = Table( "catalog_metadata", @@ -46,6 +50,10 @@ def __init__(self, database: Path) -> None: upload_sessions, upload_session_files, upload_quota, + people, + person_aliases, + person_references, + person_cluster_links, ), ) _local_metadata.create_all(self.engine) @@ -59,7 +67,7 @@ def __init__(self, database: Path) -> None: schema_version=CATALOG_SCHEMA_VERSION ) ) - elif version in {1, 2, 3}: + elif version in {1, 2, 3, 4}: connection.execute( update(catalog_metadata).values( schema_version=CATALOG_SCHEMA_VERSION diff --git a/src/vidxp/infrastructure/sql_catalog.py b/src/vidxp/infrastructure/sql_catalog.py index c35a59ab..ef4507ad 100644 --- a/src/vidxp/infrastructure/sql_catalog.py +++ b/src/vidxp/infrastructure/sql_catalog.py @@ -23,6 +23,11 @@ from vidxp.core.artifacts import ArtifactRecord, ArtifactState from vidxp.core.media import MediaRecord, MediaState, utc_now +from vidxp.core.people import ( + PersonClusterLink, + PersonRecord, + PersonReference, +) from vidxp.core.uploads import ( UploadIntentRecord, UploadSessionFileRecord, @@ -37,6 +42,10 @@ media, media_import_requests, metadata, + people, + person_aliases, + person_cluster_links, + person_references, upload_intents, upload_quota, upload_session_files, @@ -1209,3 +1218,232 @@ def with_upload_transaction( ) -> Any: with self._write_transaction() as connection: return operation(connection) + + # ---- People (Issue #85: reviewable person identities) ---- + + def put_person(self, record: PersonRecord) -> PersonRecord: + values = { + "person_id": record.person_id, + "created_at": record.created_at.isoformat(), + "payload": record.model_dump(mode="json"), + } + with self._write_transaction() as connection: + existing = self._person_by_id(connection, record.person_id) + if existing is not None: + if existing != record: + raise FileExistsError( + f"Person {record.person_id} already has another " + "record." + ) + return existing + try: + with connection.begin_nested(): + connection.execute(insert(people).values(**values)) + except IntegrityError: + existing = self._person_by_id(connection, record.person_id) + if existing == record: + return existing + raise + return record + + def get_person(self, person_id: str) -> PersonRecord | None: + with self.engine.connect() as connection: + return self._person_by_id(connection, person_id) + + def list_people( + self, + *, + limit: int, + offset: int = 0, + ) -> tuple[PersonRecord, ...]: + with self.engine.connect() as connection: + rows = connection.execute( + select(people.c.payload) + .order_by(people.c.created_at) + .limit(limit) + .offset(offset) + ) + return tuple(_record(PersonRecord, row.payload) for row in rows) + + def delete_person(self, person_id: str) -> None: + with self._write_transaction() as connection: + connection.execute( + delete(people).where(people.c.person_id == person_id) + ) + + @staticmethod + def _person_by_id( + connection: Connection, + person_id: str, + ) -> PersonRecord | None: + payload = connection.execute( + select(people.c.payload).where(people.c.person_id == person_id) + ).scalar_one_or_none() + return ( + None + if payload is None + else _record(PersonRecord, payload) + ) + + # ---- Person aliases ---- + + def add_alias(self, person_id: str, alias: str) -> None: + with self._write_transaction() as connection: + try: + with connection.begin_nested(): + connection.execute( + insert(person_aliases).values( + person_id=person_id, + alias=alias, + ) + ) + except IntegrityError: + pass + + def remove_alias(self, person_id: str, alias: str) -> None: + with self._write_transaction() as connection: + connection.execute( + delete(person_aliases).where( + and_( + person_aliases.c.person_id == person_id, + person_aliases.c.alias == alias, + ) + ) + ) + + def list_aliases(self, person_id: str) -> tuple[str, ...]: + with self.engine.connect() as connection: + rows = connection.execute( + select(person_aliases.c.alias).where( + person_aliases.c.person_id == person_id + ) + ) + return tuple(row.alias for row in rows) + + # ---- Person reference images ---- + + def add_reference(self, record: PersonReference) -> PersonReference: + values = { + "reference_id": record.reference_id, + "person_id": record.person_id, + "storage_key": record.storage_key, + "sha256": record.sha256, + "byte_size": record.byte_size, + "mime_type": record.mime_type, + "created_at": record.created_at.isoformat(), + } + with self._write_transaction() as connection: + connection.execute(insert(person_references).values(**values)) + return record + + def remove_reference(self, reference_id: str) -> None: + with self._write_transaction() as connection: + connection.execute( + delete(person_references).where( + person_references.c.reference_id == reference_id + ) + ) + + def list_references(self, person_id: str) -> tuple[PersonReference, ...]: + with self.engine.connect() as connection: + rows = connection.execute( + select(person_references).where( + person_references.c.person_id == person_id + ) + ) + return tuple( + PersonReference( + reference_id=row.reference_id, + person_id=row.person_id, + storage_key=row.storage_key, + sha256=row.sha256, + byte_size=row.byte_size, + mime_type=row.mime_type, + created_at=datetime.fromisoformat(row.created_at), + ) + for row in rows + ) + + # ---- Person <-> actor cluster links ---- + + def link_cluster(self, link: PersonClusterLink) -> PersonClusterLink: + values = { + "person_id": link.person_id, + "cluster_id": link.cluster_id, + "media_id": link.media_id, + "generation_id": link.generation_id, + "created_at": link.created_at.isoformat(), + } + with self._write_transaction() as connection: + try: + with connection.begin_nested(): + connection.execute( + insert(person_cluster_links).values(**values) + ) + except IntegrityError: + pass + return link + + def unlink_cluster( + self, + *, + person_id: str, + cluster_id: str, + media_id: str, + generation_id: str, + ) -> None: + with self._write_transaction() as connection: + connection.execute( + delete(person_cluster_links).where( + and_( + person_cluster_links.c.person_id == person_id, + person_cluster_links.c.cluster_id == cluster_id, + person_cluster_links.c.media_id == media_id, + person_cluster_links.c.generation_id + == generation_id, + ) + ) + ) + + def clusters_for_person( + self, + person_id: str, + ) -> tuple[PersonClusterLink, ...]: + with self.engine.connect() as connection: + rows = connection.execute( + select(person_cluster_links).where( + person_cluster_links.c.person_id == person_id + ) + ) + return tuple( + PersonClusterLink( + person_id=row.person_id, + cluster_id=row.cluster_id, + media_id=row.media_id, + generation_id=row.generation_id, + created_at=datetime.fromisoformat(row.created_at), + ) + for row in rows + ) + + def person_for_cluster( + self, + *, + cluster_id: str, + media_id: str, + generation_id: str, + ) -> PersonRecord | None: + with self.engine.connect() as connection: + person_id = connection.execute( + select(person_cluster_links.c.person_id).where( + and_( + person_cluster_links.c.cluster_id == cluster_id, + person_cluster_links.c.media_id == media_id, + person_cluster_links.c.generation_id + == generation_id, + ) + ) + ).scalar_one_or_none() + if person_id is None: + return None + return self._person_by_id(connection, person_id) diff --git a/src/vidxp/infrastructure/sql_tables.py b/src/vidxp/infrastructure/sql_tables.py index f0bf6532..a25f4f49 100644 --- a/src/vidxp/infrastructure/sql_tables.py +++ b/src/vidxp/infrastructure/sql_tables.py @@ -201,3 +201,63 @@ upload_intents.c.expires_at, upload_intents.c.state, ) +people = Table( + "people", + metadata, + Column("person_id", String(32), primary_key=True), + Column("created_at", Text, nullable=False), + Column("payload", JSON, nullable=False), +) + +person_aliases = Table( + "person_aliases", + metadata, + Column( + "person_id", + String(32), + ForeignKey("people.person_id", ondelete="CASCADE"), + primary_key=True, + ), + Column("alias", String(255), primary_key=True), +) + +person_references = Table( + "person_references", + metadata, + Column("reference_id", String(32), primary_key=True), + Column( + "person_id", + String(32), + ForeignKey("people.person_id", ondelete="CASCADE"), + nullable=False, + ), + Column("storage_key", Text, nullable=False), + Column("sha256", String(64), nullable=False), + Column("byte_size", BigInteger, nullable=False), + Column("mime_type", String(127), nullable=False), + Column("created_at", Text, nullable=False), +) +Index("person_references_person_id", person_references.c.person_id) + +person_cluster_links = Table( + "person_cluster_links", + metadata, + Column( + "person_id", + String(32), + ForeignKey("people.person_id", ondelete="CASCADE"), + primary_key=True, + ), + Column("cluster_id", String(512), primary_key=True), + Column("media_id", String(32), primary_key=True), + Column("generation_id", String(32), primary_key=True), + Column("created_at", Text, nullable=False), +) +Index( + "person_cluster_links_media_id", + person_cluster_links.c.media_id, +) +Index( + "person_cluster_links_cluster_id", + person_cluster_links.c.cluster_id, +) diff --git a/src/vidxp/migrations/versions/20260803_01_reviewed_people.py b/src/vidxp/migrations/versions/20260803_01_reviewed_people.py new file mode 100644 index 00000000..0e3235f1 --- /dev/null +++ b/src/vidxp/migrations/versions/20260803_01_reviewed_people.py @@ -0,0 +1,97 @@ +"""Add reviewable person identities and labels. + +Revision ID: 20260803_01 +Revises: 20260802_01 +Create Date: 2026-08-03 +""" +from alembic import op +import sqlalchemy as sa + +revision = "20260803_01" +down_revision = "20260802_01" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "people", + sa.Column("person_id", sa.String(length=32), primary_key=True), + sa.Column("created_at", sa.Text(), nullable=False), + sa.Column("payload", sa.JSON(), nullable=False), + ) + op.create_table( + "person_aliases", + sa.Column( + "person_id", + sa.String(length=32), + sa.ForeignKey("people.person_id", ondelete="CASCADE"), + primary_key=True, + ), + sa.Column("alias", sa.String(length=255), primary_key=True), + ) + op.create_table( + "person_references", + sa.Column("reference_id", sa.String(length=32), primary_key=True), + sa.Column( + "person_id", + sa.String(length=32), + sa.ForeignKey("people.person_id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column("storage_key", sa.Text(), nullable=False), + sa.Column("sha256", sa.String(length=64), nullable=False), + sa.Column("byte_size", sa.BigInteger(), nullable=False), + sa.Column("mime_type", sa.String(length=127), nullable=False), + sa.Column("created_at", sa.Text(), nullable=False), + ) + op.create_index( + "person_references_person_id", + "person_references", + ["person_id"], + unique=False, + ) + op.create_table( + "person_cluster_links", + sa.Column( + "person_id", + sa.String(length=32), + sa.ForeignKey("people.person_id", ondelete="CASCADE"), + primary_key=True, + ), + sa.Column("cluster_id", sa.String(length=512), primary_key=True), + sa.Column("media_id", sa.String(length=32), primary_key=True), + sa.Column("generation_id", sa.String(length=32), primary_key=True), + sa.Column("created_at", sa.Text(), nullable=False), + ) + op.create_index( + "person_cluster_links_media_id", + "person_cluster_links", + ["media_id"], + unique=False, + ) + op.create_index( + "person_cluster_links_cluster_id", + "person_cluster_links", + ["cluster_id"], + unique=False, + ) + + +def downgrade() -> None: + op.drop_index( + "person_cluster_links_cluster_id", + table_name="person_cluster_links", + ) + op.drop_index( + "person_cluster_links_media_id", + table_name="person_cluster_links", + ) + op.drop_table("person_cluster_links") + op.drop_index( + "person_references_person_id", + table_name="person_references", + ) + op.drop_table("person_references") + op.drop_table("person_aliases") + op.drop_table("people") diff --git a/tests/test_database_cli.py b/tests/test_database_cli.py index db16a5cd..f2287b53 100644 --- a/tests/test_database_cli.py +++ b/tests/test_database_cli.py @@ -41,7 +41,7 @@ def test_native_ingestion_migration_is_the_only_head(self): scripts = ScriptDirectory.from_config(config) - self.assertEqual(scripts.get_heads(), ["20260802_01"]) + self.assertEqual(scripts.get_heads(), ["20260803_01"]) def test_sqlite_upgrade_downgrade_and_reupgrade_from_pre_feature(self): with TemporaryDirectory() as temporary: @@ -85,7 +85,7 @@ def test_sqlite_upgrade_downgrade_and_reupgrade_from_pre_feature(self): connection.execute( text("SELECT version_num FROM alembic_version") ).scalar_one(), - "20260802_01", + "20260803_01", ) self.assertIn("upload_sessions", inspect(engine).get_table_names()) finally: diff --git a/tests/test_media_catalog.py b/tests/test_media_catalog.py index 16a76b95..8179b737 100644 --- a/tests/test_media_catalog.py +++ b/tests/test_media_catalog.py @@ -99,6 +99,10 @@ def test_catalog_enforces_sqlite_integrity_and_schema_version(self): "catalog_metadata", "media", "media_import_requests", + "people", + "person_aliases", + "person_cluster_links", + "person_references", "upload_intents", "upload_quota", "upload_session_files", @@ -109,7 +113,7 @@ def test_catalog_enforces_sqlite_integrity_and_schema_version(self): connection.exec_driver_sql( "SELECT schema_version FROM catalog_metadata" ).scalar_one(), - 4, + 5, ) self.assertEqual( connection.exec_driver_sql( diff --git a/tests/test_people_catalog.py b/tests/test_people_catalog.py new file mode 100644 index 00000000..f3931ea5 --- /dev/null +++ b/tests/test_people_catalog.py @@ -0,0 +1,292 @@ +import unittest +from datetime import datetime, timezone +from pathlib import Path +from tempfile import TemporaryDirectory +from uuid import uuid4 + +from vidxp.core.people import PersonClusterLink, PersonRecord, PersonReference +from vidxp.infrastructure.local_catalog import LocalCatalog + + +def _uuid() -> str: + return uuid4().hex + + +def person_record( + person_id: str | None = None, + *, + display_name: str = "Jane Doe", + notes: str | None = None, + biography: str | None = None, +) -> PersonRecord: + return PersonRecord( + person_id=person_id or _uuid(), + display_name=display_name, + notes=notes, + biography=biography, + created_at=datetime.now(timezone.utc), + ) + + +def cluster_link( + *, + person_id: str, + cluster_id: str, + media_id: str, + generation_id: str, +) -> PersonClusterLink: + return PersonClusterLink( + person_id=person_id, + cluster_id=cluster_id, + media_id=media_id, + generation_id=generation_id, + created_at=datetime.now(timezone.utc), + ) + + +def person_reference( + *, + person_id: str, + reference_id: str | None = None, +) -> PersonReference: + checksum = "1" * 64 + return PersonReference( + reference_id=reference_id or _uuid(), + person_id=person_id, + storage_key=f"objects/{checksum[:2]}/{checksum}.jpg", + sha256=checksum, + byte_size=100, + mime_type="image/jpeg", + created_at=datetime.now(timezone.utc), + ) + + +class PeopleCatalogTests(unittest.TestCase): + def test_catalog_creates_people_tables_and_bumps_schema_version(self): + with TemporaryDirectory() as directory: + database = Path(directory) / "catalog.sqlite3" + catalog = LocalCatalog(database) + with catalog.engine.connect() as connection: + tables = set( + connection.exec_driver_sql( + "SELECT name FROM sqlite_master " + "WHERE type = 'table'" + ).scalars() + ) + self.assertTrue( + { + "people", + "person_aliases", + "person_references", + "person_cluster_links", + }.issubset(tables) + ) + self.assertEqual( + connection.exec_driver_sql( + "SELECT schema_version FROM catalog_metadata" + ).scalar_one(), + 5, + ) + + def test_put_and_get_person_round_trips(self): + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + record = person_record(display_name="Alex Rivera", notes="met at conference") + self.assertEqual(catalog.put_person(record), record) + self.assertEqual(catalog.get_person(record.person_id), record) + self.assertIsNone(catalog.get_person(_uuid())) + + def test_put_person_is_idempotent_for_identical_record(self): + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + record = person_record() + self.assertEqual(catalog.put_person(record), record) + self.assertEqual(catalog.put_person(record), record) + + def test_put_person_rejects_conflicting_record_for_same_id(self): + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + record = person_record() + catalog.put_person(record) + conflicting = person_record( + record.person_id, + display_name="Someone Else", + ) + with self.assertRaises(FileExistsError): + catalog.put_person(conflicting) + + def test_list_people_returns_all_created_records(self): + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + first = person_record(display_name="Person One") + second = person_record(display_name="Person Two") + catalog.put_person(first) + catalog.put_person(second) + listed = catalog.list_people(limit=10) + self.assertEqual(set(p.person_id for p in listed), {first.person_id, second.person_id}) + + def test_aliases_can_be_added_removed_and_listed(self): + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + record = person_record() + catalog.put_person(record) + + catalog.add_alias(record.person_id, "AR") + catalog.add_alias(record.person_id, "Al") + self.assertEqual( + set(catalog.list_aliases(record.person_id)), + {"AR", "Al"}, + ) + + catalog.remove_alias(record.person_id, "AR") + self.assertEqual(catalog.list_aliases(record.person_id), ("Al",)) + + def test_adding_the_same_alias_twice_does_not_error(self): + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + record = person_record() + catalog.put_person(record) + catalog.add_alias(record.person_id, "AR") + catalog.add_alias(record.person_id, "AR") # must not raise + self.assertEqual(catalog.list_aliases(record.person_id), ("AR",)) + + def test_references_can_be_added_listed_and_removed(self): + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + record = person_record() + catalog.put_person(record) + + reference = person_reference(person_id=record.person_id) + catalog.add_reference(reference) + listed = catalog.list_references(record.person_id) + self.assertEqual(listed, (reference,)) + + catalog.remove_reference(reference.reference_id) + self.assertEqual(catalog.list_references(record.person_id), ()) + + def test_cluster_links_survive_reindexing_with_a_new_generation(self): + """The core requirement of Issue #85: a reviewed person and its + links must survive re-indexing, even though re-indexing produces + a brand new anonymous cluster_id under a new generation_id. + """ + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + record = person_record() + catalog.put_person(record) + + media_id = _uuid() + generation_a = _uuid() + generation_b = _uuid() + + link_before_reindex = cluster_link( + person_id=record.person_id, + cluster_id="cluster-a", + media_id=media_id, + generation_id=generation_a, + ) + catalog.link_cluster(link_before_reindex) + + # Simulate re-indexing: a new generation produces a + # different anonymous cluster id for the same person. + link_after_reindex = cluster_link( + person_id=record.person_id, + cluster_id="cluster-b", + media_id=media_id, + generation_id=generation_b, + ) + catalog.link_cluster(link_after_reindex) + + links = catalog.clusters_for_person(record.person_id) + self.assertEqual(len(links), 2) + self.assertEqual( + {link.cluster_id for link in links}, + {"cluster-a", "cluster-b"}, + ) + + # The person record itself must be untouched by reindexing. + self.assertEqual(catalog.get_person(record.person_id), record) + + # Lookup by the NEW cluster must resolve to the same person. + found = catalog.person_for_cluster( + cluster_id="cluster-b", + media_id=media_id, + generation_id=generation_b, + ) + self.assertEqual(found, record) + + def test_unlink_cluster_corrects_an_accidental_merge_without_touching_media(self): + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + record = person_record() + catalog.put_person(record) + + media_id = _uuid() + generation_id = _uuid() + link = cluster_link( + person_id=record.person_id, + cluster_id="cluster-accidental", + media_id=media_id, + generation_id=generation_id, + ) + catalog.link_cluster(link) + self.assertEqual(len(catalog.clusters_for_person(record.person_id)), 1) + + catalog.unlink_cluster( + person_id=record.person_id, + cluster_id="cluster-accidental", + media_id=media_id, + generation_id=generation_id, + ) + self.assertEqual(catalog.clusters_for_person(record.person_id), ()) + # The person record itself remains; only the link was removed. + self.assertEqual(catalog.get_person(record.person_id), record) + + def test_linking_the_same_cluster_twice_does_not_error(self): + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + record = person_record() + catalog.put_person(record) + link = cluster_link( + person_id=record.person_id, + cluster_id="cluster-a", + media_id=_uuid(), + generation_id=_uuid(), + ) + catalog.link_cluster(link) + catalog.link_cluster(link) # must not raise + self.assertEqual(len(catalog.clusters_for_person(record.person_id)), 1) + + def test_deleting_a_person_cascades_aliases_references_and_links_only(self): + """Removing a reviewed identity must clean up its own aliases, + references, and cluster links, but must never delete the + underlying video/media/index data (Issue #85's explicit + non-destructive removal requirement). + """ + with TemporaryDirectory() as directory: + catalog = LocalCatalog(Path(directory) / "catalog.sqlite3") + record = person_record() + catalog.put_person(record) + catalog.add_alias(record.person_id, "AR") + reference = person_reference(person_id=record.person_id) + catalog.add_reference(reference) + media_id = _uuid() + generation_id = _uuid() + catalog.link_cluster( + cluster_link( + person_id=record.person_id, + cluster_id="cluster-a", + media_id=media_id, + generation_id=generation_id, + ) + ) + + catalog.delete_person(record.person_id) + + self.assertIsNone(catalog.get_person(record.person_id)) + self.assertEqual(catalog.list_aliases(record.person_id), ()) + self.assertEqual(catalog.list_references(record.person_id), ()) + self.assertEqual(catalog.clusters_for_person(record.person_id), ()) + + +if __name__ == "__main__": + unittest.main() From fc2cce245566f6d42106212285c196092f5426a0 Mon Sep 17 00:00:00 2001 From: ik020 Date: Sat, 5 Sep 2026 13:55:31 +0500 Subject: [PATCH 2/2] Fix database_cli.py to ensure local directories exist before migrating Fresh environments hit 'unable to open database file' because upgrade_database() assumes the target directory tree already exists. --- src/vidxp/database_cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vidxp/database_cli.py b/src/vidxp/database_cli.py index 147c81f1..3a9e98e8 100644 --- a/src/vidxp/database_cli.py +++ b/src/vidxp/database_cli.py @@ -32,6 +32,7 @@ def main(arguments: Sequence[str] | None = None) -> None: mode=ApplicationMode.server, runtime_backend="cpu", ) + settings.layout.ensure_local_directories() upgrade_database(workflow_database_url(settings))