feat(tags): TagMergeConflict + provenance helper, enrich rename collision
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user