fix(cleanup): artist cascade preview counts posts and attachments (#3067)
CI / lint (push) Successful in 5s
CI / extension-version (push) Successful in 5s
CI / frontend-build (push) Successful in 23s
CI / backend-lint-and-test (push) Successful in 34s
CI / integration (push) Successful in 4m32s

`project_artist_cascade` is documented as "a read-only projection of what
delete_artist_cascade would touch" and drives the Tier-C confirm dialog,
but it counted only images, sources, thumbs, import_tasks and bytes. It
never counted posts or attachments, both of which the apply destroys.

That is silent in the worst case. `Post.artist_id` is ondelete=CASCADE, so
every post goes whether or not it carried an image — and FC has a large
body-only post population (#1288 measured 694 pixiv posts with text and no
images). Such an artist previewed as `images: 0`, reading as "empty, safe
to remove", while the apply destroyed every captured body, description,
external-link set and raw_metadata snapshot. The danger-zone card already
promised "every image, source, post, and attachment" — the copy was honest
and the numbers were not.

Root of the drift: the preview re-derived its own predicates instead of
sharing the apply's, the same shape as the 2026-06-08 fandom-tag deletion
that rule 93 exists for. So rather than bolt on two counts, both halves now
build from shared `_artist_{images,posts,attachments}_conditions` helpers,
following the `_unused_tag_conditions` / `_bare_post_conditions` style
already in the file. Sources keep no helper — the apply doesn't query them
either, it gets them from the Artist.sources ORM cascade.

The apply also now reports `posts_deleted` (counted before the delete,
since the CASCADE leaves nothing to count after). Rule 93's second half
asks for the apply to be tested, and parity is only assertable if both
halves state the number.

Adds a preview/apply parity test that runs both against one artist and
asserts the three pairs agree AND that the rows actually went, plus a
body-only-artist test covering the case that motivated this.

The confirm dialog's counts grid renders every key, so posts and
attachments surface there automatically; the prose summary line names
posts explicitly, since that is the number that changes how an artist
reads.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-26 22:23:29 -04:00
co-authored by Claude Opus 5
parent 39cf81aea6
commit 2ce467e347
3 changed files with 176 additions and 31 deletions
+80 -23
View File
@@ -48,6 +48,47 @@ log = logging.getLogger(__name__)
_VIDEO_DURATION_UNKNOWN = -1.0 _VIDEO_DURATION_UNKNOWN = -1.0
# -- artist-cascade predicates (rule 93: ONE definition, preview + apply) ---
# project_artist_cascade (preview) and delete_artist_cascade (apply) both build
# their queries from these. The preview used to re-derive its own — which is how
# it came to count images and stay silent about posts and attachments while the
# apply destroyed both. Same failure shape as the 2026-06-08 fandom-tag
# deletion, where a re-implemented delete predicate diverged from the preview's.
# Returned as condition LISTS spread into `.where(*conds)`, matching
# _unused_tag_conditions / _bare_post_conditions below.
def _artist_images_conditions(artist_id: int) -> list:
"""Images the cascade deletes (rows AND their on-disk files)."""
return [ImageRecord.artist_id == artist_id]
def _artist_posts_conditions(artist_id: int) -> list:
"""Posts the cascade destroys. The apply never names these — post.artist_id
is ondelete=CASCADE, so Postgres takes them when the artist row goes — which
is exactly why the preview has to name them: an artist whose posts are
body-only (no images) otherwise previews as `images: 0` and reads as an
empty artist, while every captured body/description/external-link set is
destroyed."""
return [Post.artist_id == artist_id]
def _artist_attachments_conditions(artist_id: int) -> list:
"""Attachments the cascade deletes. Matched by artist_id OR by the owning
post's artist: artist_id is nullable (_capture_attachment leaves it NULL
when no artist resolved), so neither arm alone covers every row. The
sha-addressed blobs are NOT unlinked (one blob backs many rows) — these are
row counts, and the bytes are not part of this operation's footprint."""
return [
or_(
PostAttachment.artist_id == artist_id,
PostAttachment.post_id.in_(
select(Post.id).where(*_artist_posts_conditions(artist_id))
),
)
]
def project_artist_cascade(session: Session, *, slug: str) -> dict: def project_artist_cascade(session: Session, *, slug: str) -> dict:
"""Read-only projection of what delete_artist_cascade would touch. """Read-only projection of what delete_artist_cascade would touch.
@@ -56,12 +97,17 @@ def project_artist_cascade(session: Session, *, slug: str) -> dict:
"artist": {"id": int, "name": str, "slug": str}, "artist": {"id": int, "name": str, "slug": str},
"projected": { "projected": {
"images": int, "images": int,
"posts": int, # hard-deleted by the post.artist_id CASCADE
"attachments": int, # rows deleted; the sha-addressed blobs stay
"sources": int, "sources": int,
"thumbs": int, # images with a thumbnail_path set "thumbs": int, # images with a thumbnail_path set
"import_tasks": int, # ImportTask rows referencing the artist's images "import_tasks": int, # ImportTask rows referencing the artist's images
"bytes_on_disk": int, # SUM(image_record.size_bytes) — column is NOT NULL "bytes_on_disk": int, # SUM(image_record.size_bytes) — column is NOT NULL
}, },
} }
Every count is built from the shared `_artist_*_conditions` predicates the
apply uses, so the two halves cannot drift (rule 93).
Raises LookupError if slug not found. No mutations. Raises LookupError if slug not found. No mutations.
""" """
from ..models.import_task import ImportTask from ..models.import_task import ImportTask
@@ -73,36 +119,49 @@ def project_artist_cascade(session: Session, *, slug: str) -> dict:
if artist is None: if artist is None:
raise LookupError(f"artist slug not found: {slug!r}") raise LookupError(f"artist slug not found: {slug!r}")
images_conds = _artist_images_conditions(artist.id)
images_count = session.execute( images_count = session.execute(
select(func.count(ImageRecord.id)) select(func.count(ImageRecord.id)).where(*images_conds)
.where(ImageRecord.artist_id == artist.id)
).scalar_one() ).scalar_one()
posts_count = session.execute(
select(func.count(Post.id))
.where(*_artist_posts_conditions(artist.id))
).scalar_one()
attachments_count = session.execute(
select(func.count(PostAttachment.id))
.where(*_artist_attachments_conditions(artist.id))
).scalar_one()
# Sources have no shared predicate: the apply never queries them either, it
# gets them from the Artist.sources ORM cascade. Counted directly here.
sources_count = session.execute( sources_count = session.execute(
select(func.count(Source.id)) select(func.count(Source.id))
.where(Source.artist_id == artist.id) .where(Source.artist_id == artist.id)
).scalar_one() ).scalar_one()
thumbs_count = session.execute( thumbs_count = session.execute(
select(func.count(ImageRecord.id)) select(func.count(ImageRecord.id))
.where(ImageRecord.artist_id == artist.id) .where(*images_conds)
.where(ImageRecord.thumbnail_path.is_not(None)) .where(ImageRecord.thumbnail_path.is_not(None))
).scalar_one() ).scalar_one()
import_tasks_count = session.execute( import_tasks_count = session.execute(
select(func.count(ImportTask.id)) select(func.count(ImportTask.id))
.where( .where(
ImportTask.result_image_id.in_( ImportTask.result_image_id.in_(
select(ImageRecord.id).where(ImageRecord.artist_id == artist.id) select(ImageRecord.id).where(*images_conds)
) )
) )
).scalar_one() ).scalar_one()
bytes_on_disk = session.execute( bytes_on_disk = session.execute(
select(func.coalesce(func.sum(ImageRecord.size_bytes), 0)) select(func.coalesce(func.sum(ImageRecord.size_bytes), 0))
.where(ImageRecord.artist_id == artist.id) .where(*images_conds)
).scalar_one() ).scalar_one()
return { return {
"artist": {"id": artist.id, "name": artist.name, "slug": artist.slug}, "artist": {"id": artist.id, "name": artist.name, "slug": artist.slug},
"projected": { "projected": {
"images": images_count, "images": images_count,
"posts": posts_count,
"attachments": attachments_count,
"sources": sources_count, "sources": sources_count,
"thumbs": thumbs_count, "thumbs": thumbs_count,
"import_tasks": import_tasks_count, "import_tasks": import_tasks_count,
@@ -291,12 +350,22 @@ def delete_artist_cascade(
"files_deleted": 0, "files_deleted": 0,
"thumbs_deleted": 0, "thumbs_deleted": 0,
"import_tasks_nulled": 0, "import_tasks_nulled": 0,
"posts_deleted": 0,
"attachments_deleted": 0, "attachments_deleted": 0,
"files_failed": 0, "files_failed": 0,
}, },
} }
artist_info = {"id": artist.id, "name": artist.name, "slug": artist.slug} artist_info = {"id": artist.id, "name": artist.name, "slug": artist.slug}
# Counted BEFORE the delete: Postgres takes these via the post.artist_id
# CASCADE when the artist row goes, so afterwards there is nothing left to
# count. Reported so the summary can be checked against the preview's
# `posts` — the parity rule 93 asks for is only testable if both halves
# actually state the number.
posts_deleted = session.execute(
select(func.count(Post.id)).where(*_artist_posts_conditions(artist.id))
).scalar_one()
images_deleted = 0 images_deleted = 0
files_deleted = 0 files_deleted = 0
thumbs_deleted = 0 thumbs_deleted = 0
@@ -305,7 +374,7 @@ def delete_artist_cascade(
while True: while True:
rows = session.execute( rows = session.execute(
select(ImageRecord) select(ImageRecord)
.where(ImageRecord.artist_id == artist.id) .where(*_artist_images_conditions(artist.id))
.limit(500) .limit(500)
).scalars().all() ).scalars().all()
if not rows: if not rows:
@@ -342,24 +411,11 @@ def delete_artist_cascade(
# _repoint_post_links guards the identical collision class in the reconcile # _repoint_post_links guards the identical collision class in the reconcile
# path; this is its artist-cascade counterpart. # path; this is its artist-cascade counterpart.
# #
# Matched by artist_id OR by the owning post's artist: artist_id is nullable # Which rows count as the artist's — and why the blobs are left on disk —
# and _capture_attachment leaves it NULL when no artist resolved, so neither # is _artist_attachments_conditions, shared with the preview.
# predicate alone covers every row this cascade is about to strand.
#
# The sha-addressed BLOBS are deliberately left on disk. One blob backs many
# rows (attachment_store.store is sha-addressed + idempotent), so unlinking
# needs a refcount pass over the surviving rows — that belongs to the
# attachment-reclamation sweep, not here, and this Tier-C op must not delete
# bytes its own preview never disclosed.
attachments_deleted = session.execute( attachments_deleted = session.execute(
delete(PostAttachment).where( delete(PostAttachment)
or_( .where(*_artist_attachments_conditions(artist.id))
PostAttachment.artist_id == artist.id,
PostAttachment.post_id.in_(
select(Post.id).where(Post.artist_id == artist.id)
),
)
)
).rowcount or 0 ).rowcount or 0
session.commit() session.commit()
@@ -373,6 +429,7 @@ def delete_artist_cascade(
"files_deleted": files_deleted, "files_deleted": files_deleted,
"thumbs_deleted": thumbs_deleted, "thumbs_deleted": thumbs_deleted,
"import_tasks_nulled": import_tasks_nulled, "import_tasks_nulled": import_tasks_nulled,
"posts_deleted": posts_deleted,
"attachments_deleted": attachments_deleted, "attachments_deleted": attachments_deleted,
"files_failed": files_failed, "files_failed": files_failed,
}, },
@@ -50,14 +50,19 @@ const projected = ref(null)
const projectedCounts = computed(() => projected.value?.projected || null) const projectedCounts = computed(() => projected.value?.projected || null)
const modalDescription = computed( // `posts` is named here, not left to the counts grid below it: an artist whose
() => projected.value // posts are body-only previews as `images: 0`, and a summary line that says
? `Artist “${props.artistName}” — ` // only "0 images" reads as "this artist is empty" while the apply destroys
+ `${projected.value.projected.images} images, ` // every captured post body (#3067). Attachments stay in the grid — the grid
+ `${projected.value.projected.sources} sources, ` // renders every key, so this line carries only what changes the read.
+ `${Math.round(projected.value.projected.bytes_on_disk / 1_048_576)} MiB on disk` const modalDescription = computed(() => {
: '', const p = projectedCounts.value
) return p
? `Artist “${props.artistName}” — ${p.images} images, `
+ `${p.posts} posts, ${p.sources} sources, `
+ `${Math.round(p.bytes_on_disk / 1_048_576)} MiB on disk`
: ''
})
async function onClick() { async function onClick() {
loading.value = true loading.value = true
+83
View File
@@ -68,6 +68,8 @@ def test_project_artist_cascade_returns_zeroes_for_empty_artist(db_sync):
assert result["artist"]["slug"] == "empty" assert result["artist"]["slug"] == "empty"
assert result["projected"] == { assert result["projected"] == {
"images": 0, "images": 0,
"posts": 0,
"attachments": 0,
"sources": 0, "sources": 0,
"thumbs": 0, "thumbs": 0,
"import_tasks": 0, "import_tasks": 0,
@@ -92,6 +94,87 @@ def test_project_artist_cascade_counts_images_and_thumbs_and_bytes(db_sync, tmp_
assert result["projected"]["bytes_on_disk"] == 3500 assert result["projected"]["bytes_on_disk"] == 3500
def test_project_artist_cascade_counts_posts_and_attachments(db_sync, tmp_path):
"""The body-only artist: zero images, but posts and attachments that the
apply destroys. Previewing this as `images: 0` alone is what made a
content-only artist read as an empty one (#3067)."""
a = _make_artist(db_sync, slug="bodyonly")
p1 = Post(artist_id=a.id, external_post_id="bo-1", description="a body")
p2 = Post(artist_id=a.id, external_post_id="bo-2", description="another")
db_sync.add_all([p1, p2])
db_sync.flush()
db_sync.add(PostAttachment(
post_id=p1.id, artist_id=a.id, sha256="b0d1".ljust(64, "0"),
path="/store/b0d1/a.pdf", original_filename="a.pdf",
ext=".pdf", size_bytes=5,
))
# artist_id NULL, reachable only through its post — the second arm of
# _artist_attachments_conditions.
db_sync.add(PostAttachment(
post_id=p2.id, artist_id=None, sha256="b0d2".ljust(64, "0"),
path="/store/b0d2/b.pdf", original_filename="b.pdf",
ext=".pdf", size_bytes=5,
))
db_sync.commit()
projected = cleanup_service.project_artist_cascade(
db_sync, slug="bodyonly",
)["projected"]
assert projected["images"] == 0
assert projected["posts"] == 2
assert projected["attachments"] == 2
def test_artist_cascade_preview_matches_apply(db_sync, tmp_path):
"""Rule 93: the preview's numbers must be what the apply actually does.
Guards the drift directly rather than trusting that both halves happen to
use the same predicate — the preview and apply are asserted against each
other on one artist carrying all three row kinds.
"""
a = _make_artist(db_sync, slug="parity")
for i in range(3):
f = tmp_path / f"par{i}.jpg"
f.write_bytes(b"x")
_make_image(
db_sync, artist=a, path=str(f), sha256=f"{i:064x}", size=10,
)
posts = [
Post(artist_id=a.id, external_post_id=f"par-{i}") for i in range(4)
]
db_sync.add_all(posts)
db_sync.flush()
for i, p in enumerate(posts[:2]):
db_sync.add(PostAttachment(
post_id=p.id, artist_id=a.id, sha256=f"par{i}".ljust(64, "0"),
path=f"/store/par{i}/f.zip", original_filename="f.zip",
ext=".zip", size_bytes=9,
))
db_sync.commit()
artist_id = a.id
projected = cleanup_service.project_artist_cascade(
db_sync, slug="parity",
)["projected"]
summary = cleanup_service.delete_artist_cascade(
db_sync, artist_id=artist_id, images_root=tmp_path,
)["summary"]
assert projected["images"] == summary["images_deleted"] == 3
assert projected["posts"] == summary["posts_deleted"] == 4
assert projected["attachments"] == summary["attachments_deleted"] == 2
# And the apply really did remove them — a matching pair of numbers is
# worth nothing if neither half touched the DB.
assert db_sync.execute(
select(func.count(Post.id)).where(Post.artist_id == artist_id)
).scalar_one() == 0
assert db_sync.execute(
select(func.count(PostAttachment.id))
.where(PostAttachment.artist_id == artist_id)
).scalar_one() == 0
def test_project_artist_cascade_raises_on_unknown_slug(db_sync): def test_project_artist_cascade_raises_on_unknown_slug(db_sync):
with pytest.raises(LookupError): with pytest.raises(LookupError):
cleanup_service.project_artist_cascade(db_sync, slug="nope") cleanup_service.project_artist_cascade(db_sync, slug="nope")