US07-06: Validate Performance and Resource Bounds (#93)
This commit was merged in pull request #93.
This commit is contained in:
@@ -105,7 +105,7 @@ def exif_projection(decision: str) -> dict[str, list[str]]:
|
||||
import uuid
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy import column, func, select
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from photo_pipeline.models import Asset, ExifProjection, SafetyReview
|
||||
@@ -139,8 +139,8 @@ class SafetyService:
|
||||
|
||||
# -- reads ----------------------------------------------------------------
|
||||
def _latest_by_asset(self, session) -> dict[str, SafetyReview]:
|
||||
# Latest row per asset. Small local scale: order ascending, let later rows
|
||||
# overwrite. ponytail: a windowed query if safety_reviews ever grows huge.
|
||||
"""The current review per asset, as ORM rows. Only for small, known sets —
|
||||
every library-wide caller uses ``latest_reviews()`` in SQL instead."""
|
||||
latest: dict[str, SafetyReview] = {}
|
||||
for review in session.scalars(select(SafetyReview).order_by(SafetyReview.created_at)):
|
||||
latest[review.asset_id] = review
|
||||
@@ -148,58 +148,99 @@ class SafetyService:
|
||||
|
||||
def current_decision(self, asset_id: str) -> str | None:
|
||||
with self._session_factory() as session:
|
||||
review = self._latest_by_asset(session).get(asset_id)
|
||||
return review.decision if review else None
|
||||
latest = latest_reviews().subquery()
|
||||
return session.scalar(
|
||||
select(latest.c.decision).where(latest.c.asset_id == asset_id)
|
||||
)
|
||||
|
||||
def counts(self) -> dict[str, int]:
|
||||
"""Decision breakdown over canonical, active assets — the workflow totals."""
|
||||
"""Decision breakdown over canonical, active assets — the workflow totals.
|
||||
|
||||
Aggregated in SQL: the workflow home asks for this on every load, and
|
||||
materialising every asset and every review to count them cost hundreds of
|
||||
milliseconds at 25k assets and would scale linearly from there (US07-06).
|
||||
"""
|
||||
latest = latest_reviews().subquery()
|
||||
with self._session_factory() as session:
|
||||
assets = list(session.scalars(_eligible_assets_query()))
|
||||
latest = self._latest_by_asset(session)
|
||||
out = {SFW: 0, NSFW: 0, "deferred": 0, "undecided": 0, "scored": 0}
|
||||
for asset in assets:
|
||||
review = latest.get(asset.id)
|
||||
decision = review.decision if review else None
|
||||
if decision in (SFW, NSFW, "deferred"):
|
||||
out[decision] += 1
|
||||
else:
|
||||
out["undecided"] += 1
|
||||
if review and review.score is not None:
|
||||
out["scored"] += 1
|
||||
return out
|
||||
rows = session.execute(
|
||||
select(
|
||||
func.coalesce(latest.c.decision, "undecided"),
|
||||
func.count(),
|
||||
func.count(latest.c.score),
|
||||
)
|
||||
.select_from(Asset)
|
||||
.join(latest, latest.c.asset_id == Asset.id, isouter=True)
|
||||
.where(
|
||||
Asset.canonical_asset_id.is_(None),
|
||||
Asset.availability_state == "active",
|
||||
)
|
||||
.group_by(func.coalesce(latest.c.decision, "undecided"))
|
||||
).all()
|
||||
out = {SFW: 0, NSFW: 0, "deferred": 0, "undecided": 0, "scored": 0}
|
||||
for decision, total, scored in rows:
|
||||
if decision in (SFW, NSFW, "deferred"):
|
||||
out[decision] += int(total)
|
||||
else:
|
||||
# Anything that is not one of the three decisions is undecided —
|
||||
# including a score-only review, which is what "scored" counts.
|
||||
out["undecided"] += int(total)
|
||||
out["scored"] += int(scored)
|
||||
return out
|
||||
|
||||
def review_queue(self, state: str = "", limit: int = 100, offset: int = 0) -> dict:
|
||||
"""Assets for the review UI, filtered by ``state`` (undecided/sfw/nsfw/deferred)."""
|
||||
"""Assets for the review UI, filtered by ``state`` (undecided/sfw/nsfw/deferred).
|
||||
|
||||
Filtered, counted, and paged in SQL (US07-06): the queue for a large library
|
||||
is thousands of rows and the reviewer sees one page of it.
|
||||
"""
|
||||
latest = latest_reviews().subquery()
|
||||
projections = (
|
||||
select(ExifProjection.asset_id, ExifProjection.state.label("exif_state"))
|
||||
.where(ExifProjection.stage == "safety")
|
||||
.subquery()
|
||||
)
|
||||
effective = func.coalesce(latest.c.decision, "undecided")
|
||||
query = (
|
||||
select(
|
||||
Asset.id,
|
||||
Asset.current_path,
|
||||
latest.c.score,
|
||||
latest.c.decision,
|
||||
latest.c.exif_verified_at,
|
||||
projections.c.exif_state,
|
||||
)
|
||||
.select_from(Asset)
|
||||
.join(latest, latest.c.asset_id == Asset.id, isouter=True)
|
||||
.join(projections, projections.c.asset_id == Asset.id, isouter=True)
|
||||
.where(
|
||||
Asset.canonical_asset_id.is_(None),
|
||||
Asset.availability_state == "active",
|
||||
)
|
||||
)
|
||||
if state:
|
||||
query = query.where(effective == state)
|
||||
with self._session_factory() as session:
|
||||
assets = list(session.scalars(_eligible_assets_query().order_by(Asset.current_path)))
|
||||
latest = self._latest_by_asset(session)
|
||||
# One query, not one per asset: the reviewer needs to see a divergent
|
||||
# checkpoint, which is neither "verified" nor a plain failure (US07-03).
|
||||
projections = {
|
||||
row.asset_id: row.state
|
||||
for row in session.scalars(
|
||||
select(ExifProjection).where(ExifProjection.stage == "safety")
|
||||
)
|
||||
}
|
||||
rows = []
|
||||
for asset in assets:
|
||||
review = latest.get(asset.id)
|
||||
decision = review.decision if review else None
|
||||
effective = decision or "undecided"
|
||||
if state and state != effective:
|
||||
continue
|
||||
rows.append(
|
||||
{
|
||||
"asset_id": asset.id,
|
||||
"current_path": asset.current_path,
|
||||
"score": review.score if review else None,
|
||||
"decision": decision,
|
||||
"suggested": classify(review.score) if review and review.score is not None else None,
|
||||
"exif_verified": bool(review and review.exif_verified_at),
|
||||
"exif_state": projections.get(asset.id),
|
||||
}
|
||||
)
|
||||
return {"total": len(rows), "items": rows[offset : offset + limit]}
|
||||
total = int(
|
||||
session.scalar(select(func.count()).select_from(query.subquery())) or 0
|
||||
)
|
||||
rows = session.execute(
|
||||
query.order_by(Asset.current_path).limit(limit).offset(offset)
|
||||
).all()
|
||||
return {
|
||||
"total": total,
|
||||
"items": [
|
||||
{
|
||||
"asset_id": asset_id,
|
||||
"current_path": current_path,
|
||||
"score": score,
|
||||
"decision": decision,
|
||||
"suggested": classify(score) if score is not None else None,
|
||||
"exif_verified": bool(exif_verified_at),
|
||||
"exif_state": exif_state,
|
||||
}
|
||||
for asset_id, current_path, score, decision, exif_verified_at, exif_state in rows
|
||||
],
|
||||
}
|
||||
|
||||
def scorable_asset_ids(self) -> list[str]:
|
||||
"""Canonical active assets with a path — the items a scoring job enqueues."""
|
||||
@@ -300,6 +341,34 @@ class SafetyService:
|
||||
}
|
||||
|
||||
|
||||
def latest_reviews():
|
||||
"""One row per asset: its current safety review, chosen in SQL.
|
||||
|
||||
``safety_reviews`` is append-only, so "the decision" is the newest row for an
|
||||
asset. A window function picks it without loading the table; ``rowid`` breaks a
|
||||
same-timestamp tie the same way the previous last-write-wins loop did.
|
||||
"""
|
||||
ranked = (
|
||||
select(
|
||||
SafetyReview.asset_id,
|
||||
SafetyReview.decision,
|
||||
SafetyReview.score,
|
||||
SafetyReview.exif_verified_at,
|
||||
func.row_number()
|
||||
.over(
|
||||
partition_by=SafetyReview.asset_id,
|
||||
order_by=(SafetyReview.created_at.desc(), column("rowid").desc()),
|
||||
)
|
||||
.label("rank"),
|
||||
)
|
||||
.select_from(SafetyReview)
|
||||
.subquery()
|
||||
)
|
||||
return select(
|
||||
ranked.c.asset_id, ranked.c.decision, ranked.c.score, ranked.c.exif_verified_at
|
||||
).where(ranked.c.rank == 1)
|
||||
|
||||
|
||||
def _eligible_assets_query():
|
||||
"""Canonical, active assets — the safety stage runs only on these.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user