feat(tags): system-tag UI markers + full protection sweep (step 4 of #128)
UI: shield marker + tooltip on TagChip and TagCard; system tags hide rename/merge/delete affordances (chip kebab entirely — set-fandom never applies to their general kind; remove stays, un-tagging is normal use). Aliases stay available: mapping model outputs ONTO a system tag is useful. Directory cards carry is_system. Every destructive path that could take out a system row is now guarded, found by sweeping run 1891s off-by-three failures — each one was a surface that would have eaten the seeded tags: - prune-unused: predicate exempts is_system (they ship with zero applications and matched every unused condition) - reset-content: predicate exempts is_system AND keeps their applications — hygiene flags describe the file, not content tagging - admin tag DELETE: refused with system_tag error - normalize_existing_tags: scan excludes is_system — canonicalization would recase wip -> Wip behind TagService.rename's guard, breaking the name-keyed presentation lookup Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CDgx8bQS5YrGRK76v8HUnM
This commit is contained in:
@@ -156,6 +156,10 @@ async def tag_delete(tag_id: int):
|
|||||||
)
|
)
|
||||||
except LookupError:
|
except LookupError:
|
||||||
return _bad("not_found", status=404)
|
return _bad("not_found", status=404)
|
||||||
|
except ValueError as exc:
|
||||||
|
# System tags (#128) — the training-hygiene machinery keys on
|
||||||
|
# these rows.
|
||||||
|
return _bad("system_tag", detail=str(exc))
|
||||||
return jsonify(result)
|
return jsonify(result)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -17,7 +17,7 @@ from datetime import UTC, datetime, timedelta
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from sqlalchemy import delete, func, or_, select, update
|
from sqlalchemy import and_, delete, func, or_, select, update
|
||||||
from sqlalchemy.orm import Session, aliased
|
from sqlalchemy.orm import Session, aliased
|
||||||
|
|
||||||
from ..models import (
|
from ..models import (
|
||||||
@@ -203,6 +203,9 @@ def _unused_tag_conditions() -> list:
|
|||||||
Tag.id.not_in(used_via_series),
|
Tag.id.not_in(used_via_series),
|
||||||
Tag.id.not_in(used_via_chapter),
|
Tag.id.not_in(used_via_chapter),
|
||||||
Tag.id.not_in(used_via_fandom),
|
Tag.id.not_in(used_via_fandom),
|
||||||
|
# System tags (#128) ship with zero applications and must survive a
|
||||||
|
# prune — the training-hygiene machinery keys on the rows.
|
||||||
|
Tag.is_system.is_(False),
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
@@ -402,6 +405,8 @@ def delete_tag(session: Session, *, tag_id: int) -> dict:
|
|||||||
tag = session.get(Tag, tag_id)
|
tag = session.get(Tag, tag_id)
|
||||||
if tag is None:
|
if tag is None:
|
||||||
raise LookupError(f"tag id not found: {tag_id}")
|
raise LookupError(f"tag id not found: {tag_id}")
|
||||||
|
if tag.is_system:
|
||||||
|
raise ValueError(f"'{tag.name}' is a system tag and cannot be deleted")
|
||||||
associations_count = count_tag_associations(session, tag_id=tag_id)
|
associations_count = count_tag_associations(session, tag_id=tag_id)
|
||||||
info = {"id": tag.id, "name": tag.name, "kind": tag.kind.value}
|
info = {"id": tag.id, "name": tag.name, "kind": tag.kind.value}
|
||||||
session.delete(tag)
|
session.delete(tag)
|
||||||
@@ -731,7 +736,10 @@ def reset_content_tagging(session: Session, *, dry_run: bool = False) -> dict:
|
|||||||
heads retrain from whatever the operator re-tags. (The API route gates the
|
heads retrain from whatever the operator re-tags. (The API route gates the
|
||||||
live run behind a preview-derived confirm token for exactly this reason.)
|
live run behind a preview-derived confirm token for exactly this reason.)
|
||||||
|
|
||||||
PRESERVED: fandom + series tags and their series_page ordering. CASCADE on
|
PRESERVED: fandom + series tags and their series_page ordering, AND the
|
||||||
|
system hygiene tags (#128) WITH their applications — the reset re-tags
|
||||||
|
CONTENT concepts, while wip/banner flags describe the file itself and
|
||||||
|
re-flagging hundreds of banners by hand would be pure loss. CASCADE on
|
||||||
image_tag / tag_alias / tag_suggestion_rejection clears each deleted tag's
|
image_tag / tag_alias / tag_suggestion_rejection clears each deleted tag's
|
||||||
applications + metadata. Tag.fandom_id is SET NULL, so deleting character
|
applications + metadata. Tag.fandom_id is SET NULL, so deleting character
|
||||||
tags never touches the fandom rows. Irreversible except via DB backup
|
tags never touches the fandom rows. Irreversible except via DB backup
|
||||||
@@ -744,7 +752,9 @@ def reset_content_tagging(session: Session, *, dry_run: bool = False) -> dict:
|
|||||||
"sample_names": [first 50],
|
"sample_names": [first 50],
|
||||||
and on live runs "deleted": total}
|
and on live runs "deleted": total}
|
||||||
"""
|
"""
|
||||||
predicate = Tag.kind.in_(RESETTABLE_TAG_KINDS)
|
predicate = and_(
|
||||||
|
Tag.kind.in_(RESETTABLE_TAG_KINDS), Tag.is_system.is_(False)
|
||||||
|
)
|
||||||
rows = session.execute(
|
rows = session.execute(
|
||||||
select(Tag.id, Tag.name, Tag.kind).where(predicate)
|
select(Tag.id, Tag.name, Tag.kind).where(predicate)
|
||||||
).all()
|
).all()
|
||||||
|
|||||||
@@ -107,6 +107,7 @@ class TagDirectoryService:
|
|||||||
"kind": tag.kind.value if hasattr(tag.kind, "value") else tag.kind,
|
"kind": tag.kind.value if hasattr(tag.kind, "value") else tag.kind,
|
||||||
"fandom_id": tag.fandom_id,
|
"fandom_id": tag.fandom_id,
|
||||||
"fandom_name": fandom_name,
|
"fandom_name": fandom_name,
|
||||||
|
"is_system": tag.is_system,
|
||||||
"image_count": int(image_count),
|
"image_count": int(image_count),
|
||||||
"preview_thumbnails": previews.get(tag.id, []),
|
"preview_thumbnails": previews.get(tag.id, []),
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -806,6 +806,10 @@ async def normalize_existing_tags(
|
|||||||
rows = (
|
rows = (
|
||||||
await session.execute(
|
await session.execute(
|
||||||
select(Tag.id, Tag.name, Tag.kind, Tag.fandom_id)
|
select(Tag.id, Tag.name, Tag.kind, Tag.fandom_id)
|
||||||
|
# System tags (#128) are exempt: their names are the keys the
|
||||||
|
# hygiene machinery matches on, and canonicalization would recase
|
||||||
|
# 'wip' → 'Wip' (this path bypasses TagService.rename's guard).
|
||||||
|
.where(Tag.is_system.is_(False))
|
||||||
)
|
)
|
||||||
).all()
|
).all()
|
||||||
groups = _group_existing_tags(rows)
|
groups = _group_existing_tags(rows)
|
||||||
|
|||||||
@@ -13,9 +13,15 @@
|
|||||||
<div class="fc-tagcard__name">
|
<div class="fc-tagcard__name">
|
||||||
<template v-if="!editing">
|
<template v-if="!editing">
|
||||||
{{ card.name }}
|
{{ card.name }}
|
||||||
|
<v-icon
|
||||||
|
v-if="card.is_system" size="14" icon="mdi-shield-outline"
|
||||||
|
class="fc-tagcard__system"
|
||||||
|
title="System tag — tagged items are excluded from training other concepts"
|
||||||
|
/>
|
||||||
<span v-if="card.kind === 'character' && card.fandom_name"
|
<span v-if="card.kind === 'character' && card.fandom_name"
|
||||||
class="fc-tagcard__fandom">— {{ card.fandom_name }}</span>
|
class="fc-tagcard__fandom">— {{ card.fandom_name }}</span>
|
||||||
<v-icon
|
<v-icon
|
||||||
|
v-if="!card.is_system"
|
||||||
class="fc-tagcard__edit" size="14" icon="mdi-pencil"
|
class="fc-tagcard__edit" size="14" icon="mdi-pencil"
|
||||||
@click.stop="startEdit"
|
@click.stop="startEdit"
|
||||||
/>
|
/>
|
||||||
@@ -58,12 +64,18 @@
|
|||||||
prepend-icon="mdi-tag-multiple"
|
prepend-icon="mdi-tag-multiple"
|
||||||
@click="$emit('aliases', card)"
|
@click="$emit('aliases', card)"
|
||||||
/>
|
/>
|
||||||
|
<!-- System tags (#128): merge-away and delete are refused
|
||||||
|
server-side — the hygiene machinery keys on the row — so
|
||||||
|
don't offer them. Aliases stay (mapping model outputs ONTO
|
||||||
|
a system tag is useful). -->
|
||||||
<v-list-item
|
<v-list-item
|
||||||
|
v-if="!card.is_system"
|
||||||
title="Merge with…"
|
title="Merge with…"
|
||||||
prepend-icon="mdi-call-merge"
|
prepend-icon="mdi-call-merge"
|
||||||
@click="$emit('merge-with', card)"
|
@click="$emit('merge-with', card)"
|
||||||
/>
|
/>
|
||||||
<v-list-item
|
<v-list-item
|
||||||
|
v-if="!card.is_system"
|
||||||
title="Delete tag"
|
title="Delete tag"
|
||||||
prepend-icon="mdi-delete"
|
prepend-icon="mdi-delete"
|
||||||
base-color="error"
|
base-color="error"
|
||||||
@@ -133,6 +145,7 @@ function submit() {
|
|||||||
.fc-tagcard__body { padding: 8px 12px; }
|
.fc-tagcard__body { padding: 8px 12px; }
|
||||||
.fc-tagcard__name { font-weight: 600; }
|
.fc-tagcard__name { font-weight: 600; }
|
||||||
.fc-tagcard__fandom { font-weight: 400; opacity: 0.7; }
|
.fc-tagcard__fandom { font-weight: 400; opacity: 0.7; }
|
||||||
|
.fc-tagcard__system { opacity: 0.6; margin-left: 2px; }
|
||||||
.fc-tagcard__meta {
|
.fc-tagcard__meta {
|
||||||
display: flex; align-items: center; justify-content: space-between;
|
display: flex; align-items: center; justify-content: space-between;
|
||||||
margin-top: 4px;
|
margin-top: 4px;
|
||||||
|
|||||||
@@ -10,14 +10,23 @@
|
|||||||
@click:close="$emit('remove', tag.id)"
|
@click:close="$emit('remove', tag.id)"
|
||||||
>
|
>
|
||||||
<v-icon start size="x-small">{{ iconFor(tag.kind) }}</v-icon>
|
<v-icon start size="x-small">{{ iconFor(tag.kind) }}</v-icon>
|
||||||
{{ tag.name }}<span
|
{{ tag.name }}<v-icon
|
||||||
|
v-if="tag.is_system" end size="x-small" class="fc-tag-chip__system"
|
||||||
|
title="System tag — tagged items are excluded from training other concepts"
|
||||||
|
>mdi-shield-outline</v-icon><span
|
||||||
v-if="tag.fandom_id" class="fc-tag-chip__fandom"
|
v-if="tag.fandom_id" class="fc-tag-chip__fandom"
|
||||||
:title="tag.fandom_name ? `Fandom: ${tag.fandom_name}` : 'Has a fandom'"
|
:title="tag.fandom_name ? `Fandom: ${tag.fandom_name}` : 'Has a fandom'"
|
||||||
>→<template v-if="fandomLabel"> {{ fandomLabel }}</template></span>
|
>→<template v-if="fandomLabel"> {{ fandomLabel }}</template></span>
|
||||||
</v-chip>
|
</v-chip>
|
||||||
<!-- Modal-safe kebab is baked into KebabMenu (this chip lives in the
|
<!-- Modal-safe kebab is baked into KebabMenu (this chip lives in the
|
||||||
teleported image modal — #711). -->
|
teleported image modal — #711). System tags hide it entirely: rename
|
||||||
<KebabMenu class="fc-tag-chip__kebab" :label="`More actions for ${tag.name}`">
|
is refused server-side (the hygiene machinery keys on the row) and
|
||||||
|
set-fandom never applies to their general kind. Remove (✕) stays —
|
||||||
|
un-tagging an image is normal use. -->
|
||||||
|
<KebabMenu
|
||||||
|
v-if="!tag.is_system"
|
||||||
|
class="fc-tag-chip__kebab" :label="`More actions for ${tag.name}`"
|
||||||
|
>
|
||||||
<v-list-item @click="$emit('rename', tag)">
|
<v-list-item @click="$emit('rename', tag)">
|
||||||
<v-list-item-title>Rename…</v-list-item-title>
|
<v-list-item-title>Rename…</v-list-item-title>
|
||||||
</v-list-item>
|
</v-list-item>
|
||||||
@@ -64,6 +73,7 @@ function iconFor (k) { return KIND_ICONS[k] || 'mdi-tag' }
|
|||||||
outline: 2px solid rgb(var(--v-theme-accent));
|
outline: 2px solid rgb(var(--v-theme-accent));
|
||||||
outline-offset: 1px;
|
outline-offset: 1px;
|
||||||
}
|
}
|
||||||
|
.fc-tag-chip__system { opacity: 0.6; margin-left: 2px; }
|
||||||
.fc-tag-chip__kebab { opacity: 0.7; }
|
.fc-tag-chip__kebab { opacity: 0.7; }
|
||||||
.fc-tag-chip:hover .fc-tag-chip__kebab { opacity: 1; }
|
.fc-tag-chip:hover .fc-tag-chip__kebab { opacity: 1; }
|
||||||
.fc-tag-chip__fandom { opacity: 0.7; font-size: 0.85em; }
|
.fc-tag-chip__fandom { opacity: 0.7; font-size: 0.85em; }
|
||||||
|
|||||||
@@ -7,9 +7,14 @@ mechanism keys on these exact rows. Merge-INTO stays allowed: adopting an
|
|||||||
operator's old hygiene tag into the system row is the intended move.
|
operator's old hygiene tag into the system row is the intended move.
|
||||||
"""
|
"""
|
||||||
import pytest
|
import pytest
|
||||||
from sqlalchemy import select
|
from sqlalchemy import insert, select
|
||||||
|
|
||||||
from backend.app.models import Tag, TagKind
|
from backend.app.models import ImageRecord, Tag, TagKind
|
||||||
|
from backend.app.models.tag import image_tag
|
||||||
|
from backend.app.services.cleanup_service import (
|
||||||
|
prune_unused_tags,
|
||||||
|
reset_content_tagging,
|
||||||
|
)
|
||||||
|
|
||||||
pytestmark = pytest.mark.integration
|
pytestmark = pytest.mark.integration
|
||||||
|
|
||||||
@@ -65,6 +70,72 @@ async def test_merge_away_system_tag_refused(client, db):
|
|||||||
assert "system tag" in body["error"]
|
assert "system tag" in body["error"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_admin_delete_system_tag_refused(client, db):
|
||||||
|
wip = await _system_tag(db, "wip")
|
||||||
|
resp = await client.delete(f"/api/admin/tags/{wip.id}")
|
||||||
|
assert resp.status_code == 400
|
||||||
|
body = await resp.get_json()
|
||||||
|
assert body["error"] == "system_tag"
|
||||||
|
still = (await db.execute(
|
||||||
|
select(Tag.id).where(Tag.id == wip.id)
|
||||||
|
)).scalar_one_or_none()
|
||||||
|
assert still == wip.id
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_prune_unused_skips_system_tags(db):
|
||||||
|
"""The seeded system tags have zero applications and would match every
|
||||||
|
'unused' condition — the shared predicate must exempt them (preview AND
|
||||||
|
live delete, same conditions)."""
|
||||||
|
preview = await db.run_sync(
|
||||||
|
lambda s: prune_unused_tags(s, dry_run=True)
|
||||||
|
)
|
||||||
|
assert preview["count"] == 0
|
||||||
|
assert preview["sample_names"] == []
|
||||||
|
await db.run_sync(lambda s: prune_unused_tags(s, dry_run=False))
|
||||||
|
remaining = (await db.execute(
|
||||||
|
select(Tag.name).where(Tag.is_system.is_(True))
|
||||||
|
)).scalars().all()
|
||||||
|
assert set(remaining) == SYSTEM_NAMES
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_reset_content_preserves_system_tags_and_applications(db):
|
||||||
|
"""Reset re-tags CONTENT concepts; hygiene flags describe the file itself
|
||||||
|
and survive with their applications."""
|
||||||
|
wip = await _system_tag(db, "wip")
|
||||||
|
plain = Tag(name="doomed concept", kind=TagKind.general)
|
||||||
|
db.add(plain)
|
||||||
|
img = ImageRecord(
|
||||||
|
path="/images/r.jpg", sha256="f" * 64, size_bytes=1, mime="image/jpeg",
|
||||||
|
width=1, height=1, origin="imported_filesystem",
|
||||||
|
integrity_status="unknown",
|
||||||
|
)
|
||||||
|
db.add(img)
|
||||||
|
await db.flush()
|
||||||
|
for tid in (wip.id, plain.id):
|
||||||
|
await db.execute(insert(image_tag).values(
|
||||||
|
image_record_id=img.id, tag_id=tid, source="manual",
|
||||||
|
))
|
||||||
|
await db.commit()
|
||||||
|
|
||||||
|
result = await db.run_sync(
|
||||||
|
lambda s: reset_content_tagging(s, dry_run=False)
|
||||||
|
)
|
||||||
|
assert result["deleted"] == 1 # only the plain general tag
|
||||||
|
names_left = (await db.execute(
|
||||||
|
select(Tag.name).where(Tag.kind == TagKind.general)
|
||||||
|
)).scalars().all()
|
||||||
|
assert "doomed concept" not in names_left
|
||||||
|
assert "wip" in names_left
|
||||||
|
wip_apps = (await db.execute(
|
||||||
|
select(image_tag.c.image_record_id)
|
||||||
|
.where(image_tag.c.tag_id == wip.id)
|
||||||
|
)).scalars().all()
|
||||||
|
assert wip_apps == [img.id]
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_merge_into_system_tag_allowed(client, db):
|
async def test_merge_into_system_tag_allowed(client, db):
|
||||||
wip = await _system_tag(db, "wip")
|
wip = await _system_tag(db, "wip")
|
||||||
|
|||||||
Reference in New Issue
Block a user