diff --git a/backend/app/services/tag_service.py b/backend/app/services/tag_service.py index 85a72c9..441b9b5 100644 --- a/backend/app/services/tag_service.py +++ b/backend/app/services/tag_service.py @@ -3,17 +3,39 @@ from collections.abc import Sequence from dataclasses import dataclass -from sqlalchemy import and_, case, func, select -from sqlalchemy.dialects.postgresql import insert +from sqlalchemy import and_, case, exists, func, select, text, update +from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.ext.asyncio import AsyncSession from ..models import Tag, TagKind, image_tag +from ..models.tag_allowlist import TagAllowlist +from ..models.tag_reference_embedding import TagReferenceEmbedding class TagValidationError(ValueError): """Raised when tag construction breaks the kind/fandom rules.""" +class TagMergeConflict(TagValidationError): + """Rename hit a same-(kind, fandom) name collision. Carries the + colliding target and the merge preview the 409 needs.""" + + def __init__( + self, + message: str, + *, + target_id: int, + target_name: str, + source_image_count: int, + will_alias: bool, + ): + super().__init__(message) + self.target_id = target_id + self.target_name = target_name + self.source_image_count = source_image_count + self.will_alias = will_alias + + @dataclass(frozen=True) class TagAutocompleteHit: id: int @@ -136,7 +158,7 @@ class TagService: async def add_to_image(self, image_id: int, tag_id: int, source: str = "manual") -> None: """Idempotent: re-adding an existing tag does nothing.""" - stmt = insert(image_tag).values( + stmt = pg_insert(image_tag).values( image_record_id=image_id, tag_id=tag_id, source=source ) stmt = stmt.on_conflict_do_nothing( @@ -163,12 +185,42 @@ class TagService: ) return (await self.session.execute(stmt)).scalars().all() - async def rename(self, tag_id: int, new_name: str) -> Tag: - """Rename a tag. Raises TagValidationError if the new name collides - with an existing tag of the same (kind, fandom_id). + async def _keep_as_alias(self, tag_id: int) -> bool: + """A merged-away tag's old name must survive as an alias iff the ML + pipeline has ever applied it OR could re-emit it (allowlisted / has + a centroid) — otherwise the proactive apply_allowlist_tags worker + would silently regenerate it. Purely-manual, ML-unknown tags are + deleted outright (no DB bloat).""" + is_machine = await self.session.scalar( + select( + exists().where( + and_( + image_tag.c.tag_id == tag_id, + image_tag.c.source.in_( + ("ml_auto", "ml_accepted", "auto") + ), + ) + ) + ) + ) + if is_machine: + return True + allowlisted = await self.session.scalar( + select(exists().where(TagAllowlist.tag_id == tag_id)) + ) + if allowlisted: + return True + has_centroid = await self.session.scalar( + select( + exists().where(TagReferenceEmbedding.tag_id == tag_id) + ) + ) + return bool(has_centroid) - Tag merge (consolidating two tags) lands in FC-2c; until then, - rename refuses on collision. + async def rename(self, tag_id: int, new_name: str) -> Tag: + """Rename a tag. Raises TagMergeConflict if the new name collides + with an existing tag of the same (kind, fandom_id) — the API turns + that into a 409 merge hint. """ new_name = new_name.strip() if not new_name: @@ -190,9 +242,18 @@ class TagService: ) clash = (await self.session.execute(clash_stmt)).scalar_one_or_none() if clash is not None: - raise TagValidationError( - f"A {tag.kind} tag named {new_name!r} already exists " - f"(merge lands in FC-2c)" + source_image_count = await self.session.scalar( + select(func.count()) + .select_from(image_tag) + .where(image_tag.c.tag_id == tag_id) + ) + will_alias = await self._keep_as_alias(tag_id) + raise TagMergeConflict( + f"A {tag.kind} tag named {new_name!r} already exists", + target_id=clash.id, + target_name=clash.name, + source_image_count=int(source_image_count or 0), + will_alias=will_alias, ) tag.name = new_name diff --git a/tests/test_tag_merge.py b/tests/test_tag_merge.py new file mode 100644 index 0000000..679bb98 --- /dev/null +++ b/tests/test_tag_merge.py @@ -0,0 +1,78 @@ +import pytest + +from backend.app.models import Tag, TagKind, image_tag +from backend.app.models.tag_allowlist import TagAllowlist +from backend.app.services.tag_service import ( + TagMergeConflict, + TagService, + TagValidationError, +) + +pytestmark = pytest.mark.integration + +_IMG_SEQ = 0 + + +async def _img(db): + """Insert a minimal valid ImageRecord (all NOT NULL cols satisfied: + path, sha256, size_bytes, mime, origin) and return its id.""" + global _IMG_SEQ + _IMG_SEQ += 1 + from backend.app.models import ImageRecord + + rec = ImageRecord( + path=f"/tmp/fc_merge_{_IMG_SEQ}.png", + sha256=f"{_IMG_SEQ:064d}", + size_bytes=1, + mime="image/png", + origin="uploaded", + ) + db.add(rec) + await db.flush() + return rec.id + + +@pytest.mark.asyncio +async def test_rename_collision_raises_merge_conflict_with_payload(db): + svc = TagService(db) + target = await svc.find_or_create("Existing", TagKind.character) + source = await svc.find_or_create("Dupe", TagKind.character) + img = await _img(db) + await svc.add_to_image(img, source.id, source="manual") + + with pytest.raises(TagMergeConflict) as ei: + await svc.rename(source.id, "Existing") + exc = ei.value + assert exc.target_id == target.id + assert exc.target_name == "Existing" + assert exc.source_image_count == 1 + assert exc.will_alias is False # purely manual, ML-unknown + + +@pytest.mark.asyncio +async def test_merge_conflict_is_a_tag_validation_error(db): + assert issubclass(TagMergeConflict, TagValidationError) + + +@pytest.mark.asyncio +async def test_will_alias_true_when_machine_sourced(db): + svc = TagService(db) + await svc.find_or_create("Canon", TagKind.general) + source = await svc.find_or_create("Auto", TagKind.general) + img = await _img(db) + await svc.add_to_image(img, source.id, source="ml_auto") + with pytest.raises(TagMergeConflict) as ei: + await svc.rename(source.id, "Canon") + assert ei.value.will_alias is True + + +@pytest.mark.asyncio +async def test_will_alias_true_when_allowlisted(db): + svc = TagService(db) + await svc.find_or_create("Canon2", TagKind.general) + source = await svc.find_or_create("Allowed", TagKind.general) + db.add(TagAllowlist(tag_id=source.id)) + await db.flush() + with pytest.raises(TagMergeConflict) as ei: + await svc.rename(source.id, "Canon2") + assert ei.value.will_alias is True