fix(tags): correct directory image_count — fandom leg must correlate the outer tag
The directory card count regressed to a globally-inflated number (~every card showed the same ~469): the fandom leg used a doubly-nested correlated subquery — image_tag.tag_id IN (SELECT member.id WHERE member.fandom_id == Tag.id) — whose inner predicate did not correlate the outer Tag, so it matched EVERY character that has any fandom and counted all their images for every tag. The gallery scope and cleanup count were unaffected (they pass a literal tag id, a single-level subquery), which is why only the card diverged from the gallery. Rewrite the count as a single-level correlated scalar subquery: join `member` (the tag applied to the image) and match image_tag.tag_id == Tag.id (direct) OR member.fandom_id == Tag.id (a character of this fandom). Strengthen the directory test with a second unrelated fandom/character so a non-correlating fandom leg fails (count would read 4 instead of 3). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -55,21 +55,20 @@ class TagDirectoryService:
|
|||||||
fandom = aliased(Tag)
|
fandom = aliased(Tag)
|
||||||
member = aliased(Tag)
|
member = aliased(Tag)
|
||||||
# image_count aggregates the tag's images INCLUDING, for a fandom, every
|
# image_count aggregates the tag's images INCLUDING, for a fandom, every
|
||||||
# image carrying one of its characters (member.fandom_id == Tag.id) — the
|
# image carrying one of its characters. `member` is the tag actually on
|
||||||
# member subquery is empty for non-fandom tags, so they stay direct-only.
|
# the image; an image counts for the outer Tag if that applied tag IS the
|
||||||
# DISTINCT so an image with both the fandom and ≥1 of its characters
|
# Tag (direct) OR is a character whose fandom_id is the Tag (the fandom
|
||||||
# counts once; a correlated scalar subquery keeps it 1 row per tag with
|
# leg). Both legs correlate the OUTER Tag.id at a SINGLE level — a nested
|
||||||
# no group_by (the fandom self-join below is 1:1).
|
# `tag_id IN (SELECT ... WHERE member.fandom_id == Tag.id)` does NOT
|
||||||
|
# correlate and silently counts every fandom-character globally (~every
|
||||||
|
# tag collapses to the same inflated number). DISTINCT so an image with
|
||||||
|
# the fandom AND ≥1 of its characters counts once; a correlated scalar
|
||||||
|
# subquery keeps it 1 row per tag with no group_by.
|
||||||
count_col = (
|
count_col = (
|
||||||
select(func.count(image_tag.c.image_record_id.distinct()))
|
select(func.count(image_tag.c.image_record_id.distinct()))
|
||||||
.where(
|
.select_from(image_tag)
|
||||||
or_(
|
.join(member, member.id == image_tag.c.tag_id)
|
||||||
image_tag.c.tag_id == Tag.id,
|
.where(or_(image_tag.c.tag_id == Tag.id, member.fandom_id == Tag.id))
|
||||||
image_tag.c.tag_id.in_(
|
|
||||||
select(member.id).where(member.fandom_id == Tag.id)
|
|
||||||
),
|
|
||||||
)
|
|
||||||
)
|
|
||||||
.correlate(Tag)
|
.correlate(Tag)
|
||||||
.scalar_subquery()
|
.scalar_subquery()
|
||||||
.label("image_count")
|
.label("image_count")
|
||||||
|
|||||||
@@ -47,20 +47,35 @@ async def test_directory_fandom_counts_and_previews_via_characters(db):
|
|||||||
char = Tag(name="Ichigo", kind=TagKind.character, fandom_id=fandom.id)
|
char = Tag(name="Ichigo", kind=TagKind.character, fandom_id=fandom.id)
|
||||||
db.add(char)
|
db.add(char)
|
||||||
await db.flush()
|
await db.flush()
|
||||||
|
# A SECOND, unrelated fandom + character + image. The count for "Bleach"
|
||||||
|
# must NOT pick this up — guards against the count's fandom leg failing to
|
||||||
|
# correlate the outer tag and counting EVERY fandom-character globally
|
||||||
|
# (which inflates every card to the same number).
|
||||||
|
other_fandom = Tag(name="Naruto", kind=TagKind.fandom)
|
||||||
|
db.add(other_fandom)
|
||||||
|
await db.flush()
|
||||||
|
other_char = Tag(name="Sasuke", kind=TagKind.character, fandom_id=other_fandom.id)
|
||||||
|
db.add(other_char)
|
||||||
|
await db.flush()
|
||||||
# img0: character only; img1: fandom direct; img2: BOTH (dedup → counts once)
|
# img0: character only; img1: fandom direct; img2: BOTH (dedup → counts once)
|
||||||
img0, img1, img2 = await _img(db, 0), await _img(db, 1), await _img(db, 2)
|
img0, img1, img2 = await _img(db, 0), await _img(db, 1), await _img(db, 2)
|
||||||
|
other_img = await _img(db, 3)
|
||||||
await db.execute(image_tag.insert().values([
|
await db.execute(image_tag.insert().values([
|
||||||
{"image_record_id": img0.id, "tag_id": char.id, "source": "manual"},
|
{"image_record_id": img0.id, "tag_id": char.id, "source": "manual"},
|
||||||
{"image_record_id": img1.id, "tag_id": fandom.id, "source": "manual"},
|
{"image_record_id": img1.id, "tag_id": fandom.id, "source": "manual"},
|
||||||
{"image_record_id": img2.id, "tag_id": char.id, "source": "manual"},
|
{"image_record_id": img2.id, "tag_id": char.id, "source": "manual"},
|
||||||
{"image_record_id": img2.id, "tag_id": fandom.id, "source": "manual"},
|
{"image_record_id": img2.id, "tag_id": fandom.id, "source": "manual"},
|
||||||
|
{"image_record_id": other_img.id, "tag_id": other_char.id, "source": "manual"},
|
||||||
]))
|
]))
|
||||||
await db.flush()
|
await db.flush()
|
||||||
svc = TagDirectoryService(db)
|
svc = TagDirectoryService(db)
|
||||||
page = await svc.list_tags(kind="fandom", q=None, cursor=None, limit=10)
|
page = await svc.list_tags(kind="fandom", q=None, cursor=None, limit=10)
|
||||||
card = next(c for c in page.cards if c["name"] == "Bleach")
|
card = next(c for c in page.cards if c["name"] == "Bleach")
|
||||||
assert card["image_count"] == 3
|
assert card["image_count"] == 3 # NOT 4 — Naruto's image must be excluded
|
||||||
assert len(card["preview_thumbnails"]) == 3
|
assert len(card["preview_thumbnails"]) == 3
|
||||||
|
# The unrelated fandom counts only its own one image.
|
||||||
|
other = next(c for c in page.cards if c["name"] == "Naruto")
|
||||||
|
assert other["image_count"] == 1
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|||||||
Reference in New Issue
Block a user