diff --git a/alembic/versions/0100_drop_library_placement_run.py b/alembic/versions/0100_drop_library_placement_run.py new file mode 100644 index 0000000..5b7b36c --- /dev/null +++ b/alembic/versions/0100_drop_library_placement_run.py @@ -0,0 +1,109 @@ +"""Drop library_placement_run — the placement reconciler is removed. + +Milestone #421 built a sweep that compared each image's `artist_id` to the +name of the directory its file sat in, and called every mismatch a misplaced +file. On the operator's library that reported 33,789 of 63,605 images as +wrongly filed. + +That number was an artefact of the comparison, not a fact about the library: + +- **97.1%** of it was one artist's own folder spelled differently — + `Telepurte/` versus `telepurte/`. Same artist, same art, nothing wrong. +- Of the 1% that sat in a differently-named folder, querying `ImageProvenance` + — which records the post and source each file was actually downloaded from — + showed 87 where provenance agreed with the FOLDER and not the record, and 40 + genuinely posted by several creators. The sweep would have misfiled or + arbitrarily picked for roughly 41% of that set. + +The system already knows where every file came from. The reconciler inferred +it from a column and a directory name instead, and manufactured work out of a +naming convention. Operator's call, 2026-09-21: *"the current system +consistently records where items are and where they came from this is just +complicating something works and doesn't need fixing."* + +Rule #22 — no legacy to preserve. The table goes with the code. + +## What is deliberately kept + +`utils.paths.canonical_subdir` stays: new filesystem imports derive their +directory from the artist's slug, matching what the downloader has always +done. It is not part of this tool and removing it would be churn for no fix. +Run 1's 327 moved files (`InsoUwu/` -> `insouwu/`) also stay where they are — +same artist either way, and the gallery renders them correctly. + +Revision ID: 0100 +Revises: 0099 +Create Date: 2026-09-21 + +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + +revision: str = "0100" +down_revision: Union[str, None] = "0099" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.drop_index( + "ix_library_placement_run_artist_id", + table_name="library_placement_run", + ) + op.drop_index( + "ix_library_placement_run_status", table_name="library_placement_run", + ) + op.drop_table("library_placement_run") + + +def downgrade() -> None: + # Recreates the table only. The three runs it held (one applied, two + # planned-and-never-run) are not restored and are not worth restoring — + # the code that reads them is gone. + op.create_table( + "library_placement_run", + sa.Column("id", sa.Integer(), nullable=False), + sa.Column( + "status", sa.String(length=16), server_default="running", + nullable=False, + ), + sa.Column("artist_id", sa.Integer(), nullable=True), + sa.Column( + "started_at", sa.DateTime(timezone=True), + server_default=sa.text("now()"), nullable=False, + ), + sa.Column("finished_at", sa.DateTime(timezone=True), nullable=True), + sa.Column( + "planned_count", sa.Integer(), server_default="0", nullable=False, + ), + sa.Column( + "moved_count", sa.Integer(), server_default="0", nullable=False, + ), + sa.Column( + "refused_count", sa.Integer(), server_default="0", nullable=False, + ), + sa.Column( + "moves", postgresql.JSONB(astext_type=sa.Text()), + server_default=sa.text("'[]'::jsonb"), nullable=False, + ), + sa.Column( + "refusals", postgresql.JSONB(astext_type=sa.Text()), + server_default=sa.text("'[]'::jsonb"), nullable=False, + ), + sa.Column("error", sa.Text(), nullable=True), + sa.ForeignKeyConstraint( + ["artist_id"], ["artist.id"], + name="fk_library_placement_run_artist_id", ondelete="SET NULL", + ), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index( + "ix_library_placement_run_status", "library_placement_run", ["status"], + ) + op.create_index( + "ix_library_placement_run_artist_id", "library_placement_run", + ["artist_id"], + ) diff --git a/backend/app/api/cleanup.py b/backend/app/api/cleanup.py index d4b2196..68e0f00 100644 --- a/backend/app/api/cleanup.py +++ b/backend/app/api/cleanup.py @@ -29,8 +29,8 @@ from quart import Blueprint, jsonify, request from sqlalchemy import select from ..extensions import get_session -from ..models import LibraryAuditRun, LibraryPlacementRun -from ..services import cleanup_service, library_layout +from ..models import LibraryAuditRun +from ..services import cleanup_service from ._responses import error_response as _bad cleanup_bp = Blueprint("cleanup", __name__, url_prefix="/api/cleanup") @@ -196,113 +196,3 @@ async def audit_cancel(audit_id: int): ) await session.commit() return jsonify({"cancelled": True}) - - -@cleanup_bp.route("/layout", methods=["GET"]) -async def layout_survey(): - """Milestone #421 blast radius: which ImageRecord rows sit outside their - artist's canonical slug directory, per artist. - - Read-only. `?check_disk=1` additionally stats every destination to find - collisions with a file already there and sources that have gone missing — - the numbers the apply refuses on, at the cost of one stat per misplaced - row over NFS. It is OFF by default because a count-only pass answers "how - big is this" in seconds where the disk pass can run for minutes and time - the request out. - """ - check_disk = request.args.get("check_disk", "").lower() in ("1", "true", "yes") - async with get_session() as session: - report = await session.run_sync( - lambda s: library_layout.survey_layout( - s, IMAGES_ROOT, check_disk=check_disk, - ) - ) - return jsonify({**report.as_dict(), "checked_disk": check_disk}) - - -def _serialize_placement_run(run: LibraryPlacementRun, *, moves: bool = False) -> dict: - """`moves` is opt-in: an applied whole-library run carries tens of - thousands of entries, which is a fine thing to hold in Postgres and a - poor thing to put in every list response.""" - out = { - "id": run.id, - "status": run.status, - "artist_id": run.artist_id, - "started_at": run.started_at.isoformat() if run.started_at else None, - "finished_at": run.finished_at.isoformat() if run.finished_at else None, - "planned_count": run.planned_count, - "moved_count": run.moved_count, - "refused_count": run.refused_count, - "refusals": run.refusals or [], - "error": run.error, - } - if moves: - out["moves"] = run.moves or [] - return out - - -@cleanup_bp.route("/placement/runs", methods=["GET"]) -async def placement_runs(): - """Newest first. Without `moves`, so the list stays small.""" - try: - limit = min(int(request.args.get("limit", "25")), 100) - except ValueError: - return _bad("invalid_limit") - async with get_session() as session: - rows = (await session.execute( - select(LibraryPlacementRun) - .order_by(LibraryPlacementRun.id.desc()).limit(limit) - )).scalars().all() - return jsonify({"runs": [_serialize_placement_run(r) for r in rows]}) - - -@cleanup_bp.route("/placement/runs/", methods=["GET"]) -async def placement_run(run_id: int): - """One run WITH its moves — this is the preview the operator reads before - agreeing, and the record of what happened afterwards.""" - async with get_session() as session: - run = await session.get(LibraryPlacementRun, run_id) - if run is None: - return _bad("not_found", status=404) - return jsonify(_serialize_placement_run(run, moves=True)) - - -@cleanup_bp.route("/placement/plan", methods=["POST"]) -async def placement_plan(): - """Queue a planning run. `artist_id` scopes it to one artist, which is the - intended use: do one, look at the gallery, then continue or revert.""" - body = await request.get_json(silent=True) or {} - artist_id = body.get("artist_id") - if artist_id is not None and not isinstance(artist_id, int): - return _bad("invalid_artist_id") - from ..tasks.library_placement import plan_placement - plan_placement.delay(artist_id) - return jsonify({"status": "dispatched"}), 202 - - -@cleanup_bp.route("/placement/runs//apply", methods=["POST"]) -async def placement_apply(run_id: int): - """Execute a ready run. This renames files and rewrites rows.""" - async with get_session() as session: - run = await session.get(LibraryPlacementRun, run_id) - if run is None: - return _bad("not_found", status=404) - if run.status != "ready": - return _bad("not_ready", detail=f"run is {run.status}") - from ..tasks.library_placement import apply_placement - apply_placement.delay(run_id) - return jsonify({"status": "dispatched"}), 202 - - -@cleanup_bp.route("/placement/runs//revert", methods=["POST"]) -async def placement_revert(run_id: int): - """Put an applied run's files back. The reason the ledger is kept.""" - async with get_session() as session: - run = await session.get(LibraryPlacementRun, run_id) - if run is None: - return _bad("not_found", status=404) - if run.status != "applied": - return _bad("not_applied", detail=f"run is {run.status}") - from ..tasks.library_placement import revert_placement - revert_placement.delay(run_id) - return jsonify({"status": "dispatched"}), 202 diff --git a/backend/app/celery_app.py b/backend/app/celery_app.py index 5f176a2..d49c8ef 100644 --- a/backend/app/celery_app.py +++ b/backend/app/celery_app.py @@ -35,7 +35,6 @@ def make_celery() -> Celery: "backend.app.tasks.backup", "backend.app.tasks.admin", "backend.app.tasks.library_audit", - "backend.app.tasks.library_placement", "backend.app.tasks.translation", ], ) @@ -63,8 +62,6 @@ def make_celery() -> Celery: # 2026-06-07: a 2h audit blocked vacuum/backup/normalize for hours). "backend.app.tasks.maintenance.*": {"queue": "maintenance"}, "backend.app.tasks.backup.*": {"queue": "maintenance_long"}, - # 33k renames on NFS: long lane, same as backups. - "backend.app.tasks.library_placement.*": {"queue": "maintenance_long"}, "backend.app.tasks.admin.*": {"queue": "maintenance_long"}, "backend.app.tasks.library_audit.*": {"queue": "maintenance_long"}, # Translation backfill hits the LLM (~1–6s/item) → the long lane so it diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index b9bcf4a..9e085f4 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -22,7 +22,6 @@ from .import_batch import ImportBatch from .import_settings import ImportSettings from .import_task import ImportTask from .library_audit_run import LibraryAuditRun -from .library_placement_run import LibraryPlacementRun from .membership_sync import MembershipSync from .ml_settings import MLSettings from .patreon_failed_media import PatreonFailedMedia @@ -86,7 +85,6 @@ __all__ = [ "ImportTask", "ImportSettings", "LibraryAuditRun", - "LibraryPlacementRun", "MembershipSync", "MLSettings", "HeadAutoApplyRun", diff --git a/backend/app/models/library_placement_run.py b/backend/app/models/library_placement_run.py deleted file mode 100644 index da1fbfc..0000000 --- a/backend/app/models/library_placement_run.py +++ /dev/null @@ -1,99 +0,0 @@ -"""LibraryPlacementRun — one run of the placement reconciler (milestone #421). - -The library is keyed on the Artist row's `slug`, one directory per artist. -Every writer agrees on that now (`utils.paths.canonical_subdir`, task #4244), -but ~33,789 rows were written under older rules and sit in some other -artist's directory. This row is a run of the sweep that trues them up. - -State machine, mirroring LibraryAuditRun: - - running -> ready -> applied -> reverted - \\-> cancelled - (any) -> error - -## The `moves` column does three jobs - -`moves` is the plan: `[{"image_id": 1, "from": "...", "to": "..."}, ...]`. - - 1. **Preview.** It is what the operator reads before agreeing. - 2. **Apply.** The apply executes THIS list rather than re-deriving the set, - so the preview cannot describe a different set from the apply. That is - rule 93's guarantee reached the way LibraryAuditRun reaches it — the - plan is materialised, not recomputed. - 3. **Revert.** `from` is retained, so a batch that looks wrong in the - gallery goes back where it came from. - -## An applied run IS the undo ledger — it must never be pruned - -This is the trap lesson #4226 names: a record that answers both "what is the -current plan" and "what happened" gets deleted by whatever forgets the first. -A `ready` run is disposable state. An `applied` run is HISTORY, and it is the -only record of where 33,789 files used to be — delete it and the moves become -irreversible. - -No pruning exists for this table today, and that is deliberate. If retention -is ever added here, it may prune `ready`, `cancelled` and `error` runs; an -`applied` run is only safe to drop once someone decides the moves are settled -and undo is no longer wanted, which is an operator decision and not a -timer's. -""" - -from datetime import datetime -from typing import Any - -from sqlalchemy import DateTime, ForeignKey, Integer, String, Text, func, text -from sqlalchemy.dialects.postgresql import JSONB -from sqlalchemy.orm import Mapped, mapped_column - -from .base import Base - - -class LibraryPlacementRun(Base): - __tablename__ = "library_placement_run" - - id: Mapped[int] = mapped_column(Integer, primary_key=True) - status: Mapped[str] = mapped_column( - String(16), nullable=False, default="running", index=True, - server_default="running", - ) - # running | ready | applied | reverted | cancelled | error - - # Scope. NULL = the whole library; set = one artist, which is how this is - # meant to be used — do one artist, look at it in the gallery, continue or - # revert. ondelete SET NULL rather than CASCADE: deleting an artist must - # not destroy the record of where their files were moved. - artist_id: Mapped[int | None] = mapped_column( - ForeignKey("artist.id", ondelete="SET NULL"), nullable=True, index=True, - ) - - started_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), nullable=False, server_default=func.now(), - ) - finished_at: Mapped[datetime | None] = mapped_column( - DateTime(timezone=True), nullable=True, - ) - - planned_count: Mapped[int] = mapped_column( - Integer, nullable=False, default=0, server_default="0", - ) - moved_count: Mapped[int] = mapped_column( - Integer, nullable=False, default=0, server_default="0", - ) - refused_count: Mapped[int] = mapped_column( - Integer, nullable=False, default=0, server_default="0", - ) - - # [{"image_id": int, "from": str, "to": str}, ...] — see the module - # docstring. This is the plan, the audit trail and the undo, in that order - # of appearance and in one place. - moves: Mapped[list[dict[str, Any]]] = mapped_column( - JSONB, nullable=False, default=list, server_default=text("'[]'::jsonb"), - ) - # [{"image_id": int, "reason": str}, ...] — rows the apply declined to - # touch, with why. A refusal is an expected outcome, not an error: the - # world moves between plan and apply, and every gate fails closed. - refusals: Mapped[list[dict[str, Any]]] = mapped_column( - JSONB, nullable=False, default=list, server_default=text("'[]'::jsonb"), - ) - - error: Mapped[str | None] = mapped_column(Text, nullable=True) diff --git a/backend/app/services/library_layout.py b/backend/app/services/library_layout.py deleted file mode 100644 index c14c711..0000000 --- a/backend/app/services/library_layout.py +++ /dev/null @@ -1,434 +0,0 @@ -"""Milestone #421: where an image file BELONGS, and which rows are not there. - -The library is keyed on the Artist row's `slug` — one directory per artist. -It grew a second (and third, and fourth) home for many of them because -`Importer._copy_to_library` used to name the destination after the IMPORT -folder while the downloader wrote under the slug. That writer is fixed -(`utils.paths.canonical_subdir`, task #4244); this module is the other half — -finding the rows whose files are still in the old places, and saying where -each one goes. - -## Preview and apply share these predicates, they do not re-derive them - -`_misplaced_conditions` and `destination_for` are the whole decision. The -report (task #4245) and the move (task #4246) both spread them rather than -writing their own — the house shape for rule 93, snippet #3087. A preview -that computes its set differently from the apply is a preview that can lie, -and here the apply RENAMES the operator's art. - -## What writes and what does not - -`survey_layout` and `plan_placement` are read-only — they report and they -record a plan. `apply_run` and `revert_run` are the only functions here that -rename a file or rewrite a row, and each does both for one row at a time, -updating the row only after its rename lands. -""" -from __future__ import annotations - -from dataclasses import dataclass, field -from datetime import UTC, datetime -from pathlib import Path - -from sqlalchemy import func, select -from sqlalchemy.orm import Session - -from ..models import Artist, ImageRecord, LibraryPlacementRun -from ..utils.paths import canonical_subdir - -# Top-level directories under the images root that are STORES, not artists. -# A sweep that treats these as misplaced artwork would relocate the -# thumbnail cache, the attachment blobs, or the credential key. -# thumbs/ sha-addressed thumbnail cache (NOT path-keyed — see below) -# attachments/ sha-addressed non-media blobs -# cookies/, secrets/ credential material -# _backups/, _quarantine/ backup artifacts; files pulled out of the library -RESERVED_TOP_LEVEL = frozenset({ - "thumbs", "attachments", "cookies", "secrets", "_backups", "_quarantine", -}) - - -def canonical_dir(images_root: Path, slug: str) -> Path: - """The one directory an artist's files belong under.""" - return images_root / slug - - -def _misplaced_conditions(images_root: Path, artist_id: int, slug: str) -> list: - """Rows of `artist_id` whose file is NOT under that artist's canonical - directory. Spread into both halves — never restated. - - The prefix carries a trailing separator on purpose: without it, artist - `ara` would match every path under `arbuzbudesh/`, and the sweep would - report one artist's whole library as correctly placed while quietly - skipping another's. - - `startswith` compiles to LIKE, where `_` and `%` are wildcards, and this - does not escape them. That is safe ONLY because `utils.slug.slugify` - reduces a slug to `[a-z0-9-]` — neither character can reach the pattern. - Widen that charset and this needs `autoescape=True`, or `poch4n_art` - starts matching `poch4nXart` too. - """ - prefix = f"{canonical_dir(images_root, slug)}/" - return [ - ImageRecord.artist_id == artist_id, - ImageRecord.path.is_not(None), - ~ImageRecord.path.startswith(prefix), - ] - - -def destination_for(path: str, images_root: Path, slug: str) -> Path | None: - """Where `path`'s file belongs, or None when this row must not be moved. - - None means: the path is outside the images root, or its top-level segment - is a reserved store. Both are refusals rather than errors — a row pointing - somewhere unexpected is exactly what should NOT be relocated automatically. - - ## The one place this diverges from `canonical_subdir` - - A file sitting at the images ROOT with a known artist moves under that - artist's directory here, where `canonical_subdir` would leave it alone. - The two answer different questions. At import time an empty subdir means - no artist was resolved, so there is nothing to canonicalise against. Here - the row already CARRIES an artist_id, so a file at the root is an anomaly - with a known correct home — which is the whole point of the sweep. - - (The 660 unattributed files at the root have no artist_id at all and are - not reachable from these predicates; task #4247 decides those.) - """ - p = Path(path) - try: - rel_dir = p.parent.relative_to(images_root) - except ValueError: - return None - parts = rel_dir.parts - if parts and parts[0] in RESERVED_TOP_LEVEL: - return None - sub = canonical_subdir(str(rel_dir) if str(rel_dir) != "." else "", slug) - if not sub: - # At the root, with an artist — see the docstring above. - return canonical_dir(images_root, slug) / p.name - return images_root / sub / p.name - - -@dataclass -class ArtistLayout: - """One artist's verdict.""" - - artist_id: int - name: str - slug: str - canonical_rows: int = 0 - misplaced_rows: int = 0 - # The non-canonical top-level directories this artist's files sit in — - # "Conto", "StickySpoodge", … This is what makes the report readable as - # the family list the disk survey found. - stray_dirs: list[str] = field(default_factory=list) - collisions: list[str] = field(default_factory=list) - missing_files: int = 0 - unmovable: int = 0 - - -@dataclass -class LayoutReport: - artists: list[ArtistLayout] = field(default_factory=list) - total_rows: int = 0 - canonical_rows: int = 0 - misplaced_rows: int = 0 - collision_count: int = 0 - missing_files: int = 0 - unmovable: int = 0 - unattributed_rows: int = 0 - - def as_dict(self) -> dict: - return { - "total_rows": self.total_rows, - "canonical_rows": self.canonical_rows, - "misplaced_rows": self.misplaced_rows, - "collision_count": self.collision_count, - "missing_files": self.missing_files, - "unmovable": self.unmovable, - "unattributed_rows": self.unattributed_rows, - "artists": [ - { - "artist_id": a.artist_id, - "name": a.name, - "slug": a.slug, - "canonical_rows": a.canonical_rows, - "misplaced_rows": a.misplaced_rows, - "stray_dirs": a.stray_dirs, - "collisions": a.collisions, - "missing_files": a.missing_files, - "unmovable": a.unmovable, - } - for a in self.artists - if a.misplaced_rows or a.collisions - ], - } - - -def survey_layout( - session: Session, images_root: Path, *, check_disk: bool = True, -) -> LayoutReport: - """Read-only blast radius for the consolidation. - - `check_disk` stats every destination to find rows that would collide with - a file already there, and sources that have already gone missing. It is - the honest number and it is what the apply will refuse on, but it costs - one stat per misplaced row over NFS — turn it off when you only want - counts. - """ - report = LayoutReport() - report.total_rows = session.execute( - select(func.count(ImageRecord.id)) - ).scalar_one() - report.unattributed_rows = session.execute( - select(func.count(ImageRecord.id)).where(ImageRecord.artist_id.is_(None)) - ).scalar_one() - - artists = session.execute( - select(Artist).order_by(Artist.slug) - ).scalars().all() - - for artist in artists: - layout = ArtistLayout( - artist_id=artist.id, name=artist.name, slug=artist.slug, - ) - conds = _misplaced_conditions(images_root, artist.id, artist.slug) - owned = session.execute( - select(func.count(ImageRecord.id)) - .where(ImageRecord.artist_id == artist.id) - ).scalar_one() - rows = session.execute( - select(ImageRecord.id, ImageRecord.path).where(*conds) - ).all() - layout.misplaced_rows = len(rows) - layout.canonical_rows = owned - len(rows) - - strays: set[str] = set() - destinations: dict[str, int] = {} - for row_id, path in rows: - dest = destination_for(path, images_root, artist.slug) - if dest is None: - layout.unmovable += 1 - continue - try: - top = Path(path).parent.relative_to(images_root).parts - strays.add(top[0] if top else "") - except ValueError: - strays.add("") - key = str(dest) - if key in destinations: - layout.collisions.append(key) - else: - destinations[key] = row_id - if check_disk: - if not Path(path).exists(): - layout.missing_files += 1 - elif dest.exists(): - layout.collisions.append(key) - - layout.stray_dirs = sorted(strays) - report.artists.append(layout) - report.canonical_rows += layout.canonical_rows - report.misplaced_rows += layout.misplaced_rows - report.collision_count += len(layout.collisions) - report.missing_files += layout.missing_files - report.unmovable += layout.unmovable - - return report - - -# --- the reconciler: plan -> apply -> revert (#4246) ------------------------- -# -# The three verbs share one materialised plan rather than each deriving its -# own set. `plan_placement` writes `LibraryPlacementRun.moves`; `apply_run` -# executes THAT list; `revert_run` walks it backwards. A preview that can -# disagree with its apply is the failure this shape exists to prevent, and -# here the apply renames the operator's art. -# -# Every step fails CLOSED. The world moves between planning and applying — -# a download lands, a supersede rewrites a path, a file is deleted — so the -# apply re-checks each row against what the plan recorded and declines the -# ones that moved on, instead of trusting a plan that may be minutes old. - - -def plan_placement( - session: Session, images_root: Path, *, artist_id: int | None = None, -) -> LibraryPlacementRun: - """Build (and persist) the move plan. Touches no files. - - `artist_id` scopes the run to one artist, which is how this is meant to be - used: do one, look at the gallery, then continue or revert. None plans the - whole library. - """ - stmt = select(Artist).order_by(Artist.slug) - if artist_id is not None: - stmt = stmt.where(Artist.id == artist_id) - artists = session.execute(stmt).scalars().all() - - candidates: list[dict] = [] - wanted: dict[str, int] = {} - for artist in artists: - rows = session.execute( - select(ImageRecord.id, ImageRecord.path) - .where(*_misplaced_conditions(images_root, artist.id, artist.slug)) - ).all() - for row_id, path in rows: - dest = destination_for(path, images_root, artist.slug) - if dest is None or dest.exists(): - continue - key = str(dest) - candidates.append({"image_id": row_id, "from": path, "to": key}) - wanted[key] = wanted.get(key, 0) + 1 - - # Two rows wanting one destination: plan NEITHER. Which of them "wins" is - # not this sweep's call, and planning one of them would silently pick a - # winner by iteration order. Counting first and filtering after is what - # makes that true — claiming as we go would quietly keep whichever came - # first. - moves = [m for m in candidates if wanted[m["to"]] == 1] - - run = LibraryPlacementRun( - status="ready", artist_id=artist_id, moves=moves, - planned_count=len(moves), - ) - session.add(run) - session.flush() - return run - - -def _move_one(src: Path, dest: Path) -> str | None: - """Rename `src` to `dest`. Returns a refusal reason, or None on success. - - A rename within one filesystem, so no copy and no free space needed. The - destination check is not a race-free guarantee — nothing here is — but it - turns the common case of "something already landed there" into a refusal - instead of a silent overwrite. - """ - if not src.exists(): - return "source missing" - if dest.exists(): - return "destination occupied" - try: - dest.parent.mkdir(parents=True, exist_ok=True) - src.rename(dest) - except OSError as exc: - return f"rename failed: {exc}" - return None - - -def apply_run( - session: Session, run: LibraryPlacementRun, *, chunk: int = 0, -) -> LibraryPlacementRun: - """Execute a `ready` run's stored plan: file and row together, per row. - - The row is updated ONLY after its rename lands, so a refused or failed - move can never leave `ImageRecord.path` pointing at a file that is not - there. Refusals are recorded and the run continues — one row that moved - on since planning is not a reason to abandon the other 33,788. - - `chunk` commits progress every N moves. Set it for any real run: the - ledger is the ONLY record of where a file came from, so a worker that - dies at row 30,000 of 33,789 must not take the undo information for the - first 29,999 with it. Left at 0 (tests, small runs) everything persists - in one go at the end. - - Re-running a partially-applied plan is safe rather than clever: the rows - already moved no longer match their `from`, so they refuse as "row moved - since planning" instead of being moved twice. - """ - if run.status != "ready": - raise ValueError(f"run {run.id} is {run.status}, not ready") - - refusals: list[dict] = [] - moved: list[dict] = [] - - def _persist() -> None: - # Reassign rather than mutate: SQLAlchemy does not track in-place - # changes to a JSONB list, so an .append() alone would never reach - # the database and the ledger would silently stay empty. - run.moves = list(moved) - run.refusals = list(refusals) - run.moved_count = len(moved) - run.refused_count = len(refusals) - session.commit() - - # Snapshot the plan before iterating: `_persist` reassigns `run.moves`, - # and iterating the attribute while rewriting it would walk a list that - # changes underneath the loop. - plan = list(run.moves) - for done, move in enumerate(plan, start=1): - record = session.get(ImageRecord, move["image_id"]) - if record is None: - refusals.append({"image_id": move["image_id"], "reason": "row gone"}) - continue - if record.path != move["from"]: - # Something rewrote this row since the plan was built — a - # supersede, or an earlier run. The plan is stale for it. - refusals.append({ - "image_id": move["image_id"], "reason": "row moved since planning", - }) - continue - reason = _move_one(Path(move["from"]), Path(move["to"])) - if reason is not None: - refusals.append({"image_id": move["image_id"], "reason": reason}) - continue - record.path = move["to"] - moved.append(move) - if chunk and done % chunk == 0: - _persist() - - run.moves = moved - run.refusals = refusals - run.moved_count = len(moved) - run.refused_count = len(refusals) - run.status = "applied" - run.finished_at = datetime.now(UTC) - session.flush() - return run - - -def revert_run( - session: Session, run: LibraryPlacementRun, -) -> LibraryPlacementRun: - """Put an applied run's files back where they came from. - - This is why `from` is retained. It is the answer to "do one artist, look - at it, and undo if it reads wrong" — which is a cheaper way to settle - whether artist_id or the folder held the truth (#4257) than arguing it - from a sample. - - Refuses the same way the apply does: a file someone has since moved or - replaced stays where it is, and its row is left alone. - - A revert interrupted half way is resumable by re-running it: the rows - already put back no longer sit at `to`, so they refuse rather than move - twice. Unlike the apply this needs no chunked persistence — it consumes - the ledger rather than producing it, so a crash costs progress, not - information. - """ - if run.status != "applied": - raise ValueError(f"run {run.id} is {run.status}, not applied") - - refusals: list[dict] = [] - reverted = 0 - for move in run.moves: - record = session.get(ImageRecord, move["image_id"]) - if record is None or record.path != move["to"]: - refusals.append({ - "image_id": move["image_id"], "reason": "row changed since apply", - }) - continue - reason = _move_one(Path(move["to"]), Path(move["from"])) - if reason is not None: - refusals.append({"image_id": move["image_id"], "reason": reason}) - continue - record.path = move["from"] - reverted += 1 - - run.refusals = refusals - run.refused_count = len(refusals) - run.moved_count = run.moved_count - reverted - run.status = "reverted" - run.finished_at = datetime.now(UTC) - session.flush() - return run diff --git a/backend/app/tasks/library_placement.py b/backend/app/tasks/library_placement.py deleted file mode 100644 index 510795d..0000000 --- a/backend/app/tasks/library_placement.py +++ /dev/null @@ -1,125 +0,0 @@ -"""Placement reconciler tasks — plan, apply, revert (milestone #421). - -The service (`services.library_layout`) holds the decisions; this module is -only the async wrapper, matching `tasks.library_audit`: run on the -maintenance queue, mark the run `error` with a traceback if anything escapes, -and return a small summary dict so eager-mode tests can assert on it. - -Applying is a long run — 33,789 renames on the operator's library at the time -of writing — so `apply_placement` persists its ledger in chunks rather than -at the end. That ledger is the only record of where each file came from, and -a worker that dies two thirds of the way through must not take the undo -information for the first two thirds with it. -""" - -import logging -import traceback -from datetime import UTC, datetime -from pathlib import Path - -from sqlalchemy.exc import DBAPIError, OperationalError - -from ..celery_app import celery -from ..models import LibraryPlacementRun -from ..services import library_layout -from ._sync_engine import sync_session_factory as _sync_session_factory - -log = logging.getLogger(__name__) - -IMAGES_ROOT = Path("/images") - -# Commit the ledger every this many moves. Small enough that a crash loses -# seconds of work, large enough not to make a COMMIT per rename. -_APPLY_CHUNK = 200 - - -def _fail(session, run_id: int, message: str) -> None: - run = session.get(LibraryPlacementRun, run_id) - if run is not None: - run.status = "error" - run.error = message - run.finished_at = datetime.now(UTC) - session.commit() - - -@celery.task( - name="backend.app.tasks.library_placement.plan_placement", - autoretry_for=(OperationalError, DBAPIError), - retry_backoff=5, retry_backoff_max=60, retry_jitter=True, max_retries=3, - soft_time_limit=900, time_limit=1000, -) -def plan_placement(artist_id: int | None = None) -> dict: - """Build a move plan and leave it `ready` for the operator to read. - - Reads rows and stats destinations; moves nothing. - """ - SessionLocal = _sync_session_factory() - with SessionLocal() as session: - run = library_layout.plan_placement( - session, IMAGES_ROOT, artist_id=artist_id, - ) - session.commit() - return { - "run_id": run.id, "status": run.status, - "planned_count": run.planned_count, - } - - -@celery.task( - name="backend.app.tasks.library_placement.apply_placement", - soft_time_limit=7200, time_limit=7500, -) -def apply_placement(run_id: int) -> dict: - """Execute a `ready` run's stored plan. Renames files and rewrites rows. - - No autoretry: a retry would re-enter a half-applied plan on a schedule - nobody asked for. Re-running IS safe (the applied rows refuse as "row - moved since planning"), but that should be the operator's decision after - reading what happened, not the queue's. - """ - SessionLocal = _sync_session_factory() - with SessionLocal() as session: - run = session.get(LibraryPlacementRun, run_id) - if run is None: - return {"run_id": run_id, "status": "missing"} - if run.status != "ready": - return {"run_id": run_id, "status": run.status, "skipped": True} - try: - library_layout.apply_run(session, run, chunk=_APPLY_CHUNK) - session.commit() - except Exception: - log.exception("placement apply failed for run %s", run_id) - session.rollback() - _fail(session, run_id, traceback.format_exc()) - return {"run_id": run_id, "status": "error"} - return { - "run_id": run_id, "status": run.status, - "moved": run.moved_count, "refused": run.refused_count, - } - - -@celery.task( - name="backend.app.tasks.library_placement.revert_placement", - soft_time_limit=7200, time_limit=7500, -) -def revert_placement(run_id: int) -> dict: - """Put an applied run's files back where they came from.""" - SessionLocal = _sync_session_factory() - with SessionLocal() as session: - run = session.get(LibraryPlacementRun, run_id) - if run is None: - return {"run_id": run_id, "status": "missing"} - if run.status != "applied": - return {"run_id": run_id, "status": run.status, "skipped": True} - try: - library_layout.revert_run(session, run) - session.commit() - except Exception: - log.exception("placement revert failed for run %s", run_id) - session.rollback() - _fail(session, run_id, traceback.format_exc()) - return {"run_id": run_id, "status": "error"} - return { - "run_id": run_id, "status": run.status, - "refused": run.refused_count, - } diff --git a/frontend/src/components/settings/MaintenancePanel.vue b/frontend/src/components/settings/MaintenancePanel.vue index 0c44eaa..3931384 100644 --- a/frontend/src/components/settings/MaintenancePanel.vue +++ b/frontend/src/components/settings/MaintenancePanel.vue @@ -54,7 +54,6 @@ Self-healing and repair: missing files, thumbnails, database upkeep.

- @@ -81,7 +80,6 @@ import MLBackfillCard from './MLBackfillCard.vue' import ThumbnailBackfillCard from './ThumbnailBackfillCard.vue' import ArchiveReextractCard from './ArchiveReextractCard.vue' import MissingFileRepairCard from './MissingFileRepairCard.vue' -import PlacementCard from './PlacementCard.vue' import GpuTriageCard from './GpuTriageCard.vue' import DbMaintenanceCard from './DbMaintenanceCard.vue' import VideoEmbeddingCard from './VideoEmbeddingCard.vue' diff --git a/frontend/src/components/settings/PlacementCard.vue b/frontend/src/components/settings/PlacementCard.vue deleted file mode 100644 index 462c255..0000000 --- a/frontend/src/components/settings/PlacementCard.vue +++ /dev/null @@ -1,312 +0,0 @@ - - - diff --git a/frontend/src/stores/cleanup.js b/frontend/src/stores/cleanup.js index 73b17d7..82813a2 100644 --- a/frontend/src/stores/cleanup.js +++ b/frontend/src/stores/cleanup.js @@ -71,66 +71,10 @@ export const useCleanupStore = defineStore('cleanup', () => { return await api.post(`/api/cleanup/audit/${id}/cancel`) } - // --- placement reconciler (milestone #421) -------------------------------- - // - // Runs are server-side rows, so the DATABASE is the durable state here — - // no localStorage resurfacing (useMaintenanceTask) is needed. Reload the - // page, open it on another machine, and the run and its status are simply - // there. That also means a plan survives being walked away from for a day. - - const placementRuns = ref([]) - const layout = ref(null) - - // The survey: which rows sit outside their artist's directory. check_disk - // additionally stats every destination (collisions, missing sources) and - // costs one stat per misplaced row over NFS, so it is opt-in. - async function loadLayout(checkDisk = false) { - layout.value = await api.get('/api/cleanup/layout', { - params: checkDisk ? { check_disk: 1 } : {}, - }) - return layout.value - } - - // id -> name, so a run row can say "Conto" instead of "#47". The runs - // endpoint carries artist_id alone: the name belongs to the artist, and - // denormalising it into every run would go stale the moment one is renamed. - async function loadArtistNames() { - const rows = await api.get('/api/artists/names') - return Object.fromEntries((rows || []).map(a => [a.id, a.name])) - } - - async function loadPlacementRuns(limit = 25) { - const body = await api.get('/api/cleanup/placement/runs', { params: { limit } }) - placementRuns.value = body.runs || [] - return placementRuns.value - } - - // Detail carries `moves` — the plan the operator reads before agreeing. - async function getPlacementRun(id) { - return await api.get(`/api/cleanup/placement/runs/${id}`) - } - - async function planPlacement(artistId = null) { - return await api.post('/api/cleanup/placement/plan', { - body: artistId === null ? {} : { artist_id: artistId }, - }) - } - - async function applyPlacement(id) { - return await api.post(`/api/cleanup/placement/runs/${id}/apply`) - } - - async function revertPlacement(id) { - return await api.post(`/api/cleanup/placement/runs/${id}/revert`) - } - return { defaults, recentRuns, loadDefaults, previewMinDim, deleteMinDim, startAudit, getAudit, loadHistory, latestAuditForRule, applyAudit, cancelAudit, - placementRuns, layout, - loadLayout, loadArtistNames, loadPlacementRuns, getPlacementRun, - planPlacement, applyPlacement, revertPlacement, } }) diff --git a/frontend/test/placement.spec.js b/frontend/test/placement.spec.js deleted file mode 100644 index af5fc1a..0000000 --- a/frontend/test/placement.spec.js +++ /dev/null @@ -1,111 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' -import { setActivePinia, createPinia } from 'pinia' -import { useCleanupStore } from '../src/stores/cleanup.js' -import { stubFetch } from './stubFetch.js' - - -describe('placement reconciler store (milestone #421)', () => { - beforeEach(() => setActivePinia(createPinia())) - afterEach(() => vi.restoreAllMocks()) - - it('loadLayout leaves the disk check off by default', async () => { - const s = useCleanupStore() - let seen = '' - stubFetch((url) => { - seen = url - return { status: 200, body: { total_rows: 10, misplaced_rows: 2, artists: [] } } - }) - await s.loadLayout() - // One stat per misplaced row over NFS is the cost; it must be opt-in. - expect(seen).not.toContain('check_disk') - expect(s.layout.misplaced_rows).toBe(2) - }) - - it('loadLayout asks for the disk check when requested', async () => { - const s = useCleanupStore() - let seen = '' - stubFetch((url) => { - seen = url - return { status: 200, body: { total_rows: 0, misplaced_rows: 0, artists: [] } } - }) - await s.loadLayout(true) - expect(seen).toContain('check_disk=1') - }) - - it('planPlacement scopes to an artist when given one', async () => { - const s = useCleanupStore() - let sent = null - stubFetch((url, init) => { - sent = JSON.parse(init.body) - return { status: 202, body: { status: 'dispatched' } } - }) - await s.planPlacement(47) - expect(sent).toEqual({ artist_id: 47 }) - }) - - it('planPlacement sends no scope for the whole library', async () => { - const s = useCleanupStore() - let sent = null - stubFetch((url, init) => { - sent = JSON.parse(init.body) - return { status: 202, body: { status: 'dispatched' } } - }) - await s.planPlacement() - // Not `{artist_id: null}` — the endpoint rejects a non-integer, and an - // absent key is how "whole library" is spelled. - expect(sent).toEqual({}) - }) - - it('loadPlacementRuns keeps the rows for the table', async () => { - const s = useCleanupStore() - stubFetch(() => ({ - status: 200, - body: { runs: [{ id: 3, status: 'ready', planned_count: 12 }] }, - })) - await s.loadPlacementRuns() - expect(s.placementRuns).toHaveLength(1) - expect(s.placementRuns[0].status).toBe('ready') - }) - - it('getPlacementRun carries the moves — it is the preview', async () => { - const s = useCleanupStore() - stubFetch(() => ({ - status: 200, - body: { - id: 3, status: 'ready', planned_count: 1, - moves: [{ image_id: 9, from: '/images/Conto/x.png', to: '/images/conto/x.png' }], - }, - })) - const run = await s.getPlacementRun(3) - expect(run.moves[0].from).toBe('/images/Conto/x.png') - expect(run.moves[0].to).toBe('/images/conto/x.png') - }) - - it('loadArtistNames maps id to name for the run rows', async () => { - const s = useCleanupStore() - stubFetch(() => ({ - status: 200, - body: [{ id: 47, name: 'Conto', slug: 'conto' }], - })) - expect(await s.loadArtistNames()).toEqual({ 47: 'Conto' }) - }) - - it('loadArtistNames survives an empty roster', async () => { - const s = useCleanupStore() - stubFetch(() => ({ status: 200, body: [] })) - expect(await s.loadArtistNames()).toEqual({}) - }) - - it('applyPlacement and revertPlacement post to their own run', async () => { - const s = useCleanupStore() - const urls = [] - stubFetch((url) => { - urls.push(url) - return { status: 202, body: { status: 'dispatched' } } - }) - await s.applyPlacement(5) - await s.revertPlacement(5) - expect(urls[0]).toContain('/api/cleanup/placement/runs/5/apply') - expect(urls[1]).toContain('/api/cleanup/placement/runs/5/revert') - }) -}) diff --git a/frontend/test/stubFetch.js b/frontend/test/stubFetch.js deleted file mode 100644 index 80dc8fd..0000000 --- a/frontend/test/stubFetch.js +++ /dev/null @@ -1,24 +0,0 @@ -import { vi } from 'vitest' - -// The canonical fetch stub for store specs. -// -// `handler(url, init)` returns `{ status, body }`; body is JSON-encoded, and -// `ok` is derived from the status so a store's error path can be exercised by -// returning 4xx/5xx. Returns the vi.fn so a caller can assert on calls. -// -// Extracted 2026-09-21 from six specs carrying byte-identical copies -// (adminStore, credentials, dbMaintenance, gallery, suggestions, -// galleryRelatedStrip). Those still hold their own; migrate each the next -// time it is touched rather than in one sweep. -export function stubFetch (handler) { - globalThis.fetch = vi.fn(async (url, init) => { - const { status, body } = handler(url, init) - return { - ok: status >= 200 && status < 300, - status, - statusText: String(status), - text: async () => (body == null ? '' : JSON.stringify(body)), - } - }) - return globalThis.fetch -} diff --git a/tests/test_api_placement.py b/tests/test_api_placement.py deleted file mode 100644 index 3fd56b4..0000000 --- a/tests/test_api_placement.py +++ /dev/null @@ -1,167 +0,0 @@ -"""The placement reconciler's task + API surface (milestone #421, slice 3b). - -The move logic itself is covered in tests/test_library_layout.py; this module -covers the wrapper — that the tasks are registered and routed, that the -endpoints gate on run state, and that a list response stays small. -""" - -import pytest -from sqlalchemy import select - -import backend.app.tasks.library_placement # noqa: F401 — register tasks -from backend.app.celery_app import celery -from backend.app.models import Artist, LibraryPlacementRun - -pytestmark = pytest.mark.integration - -_TASKS = ( - "backend.app.tasks.library_placement.plan_placement", - "backend.app.tasks.library_placement.apply_placement", - "backend.app.tasks.library_placement.revert_placement", -) - - -@pytest.mark.parametrize("name", _TASKS) -def test_placement_tasks_are_registered(name): - assert name in celery.tasks - - -def test_placement_runs_on_the_long_maintenance_lane(): - """33k renames must not sit in the quick lane, which is where the - self-healing sweeps live (the 2026-06-07 starvation).""" - routes = celery.conf.task_routes - assert routes["backend.app.tasks.library_placement.*"] == { - "queue": "maintenance_long" - } - - -def _run(db, status="ready", moves=None, artist_id=None): - """Adds the row; the caller awaits the COMMIT. - - Commit, not flush: the app under test runs on its own session and - connection, so a flush that stays inside this test's transaction is - invisible to the endpoint — the row simply is not there yet. Same reason - `_seed_runs` in test_api_system_backup commits.""" - run = LibraryPlacementRun( - status=status, artist_id=artist_id, moves=moves or [], - planned_count=len(moves or []), - ) - db.add(run) - return run - - -@pytest.mark.asyncio -async def test_runs_list_omits_the_moves(client, db): - """An applied whole-library run carries tens of thousands of entries. - Fine in Postgres, wrong in every list response.""" - run = LibraryPlacementRun( - status="applied", - moves=[{"image_id": 1, "from": "/images/A/x.png", "to": "/images/a/x.png"}], - planned_count=1, moved_count=1, - ) - db.add(run) - await db.commit() - - resp = await client.get("/api/cleanup/placement/runs") - assert resp.status_code == 200 - body = await resp.get_json() - assert body["runs"][0]["planned_count"] == 1 - assert "moves" not in body["runs"][0] - - -@pytest.mark.asyncio -async def test_run_detail_carries_the_moves(client, db): - """The detail IS the preview the operator reads before agreeing.""" - run = LibraryPlacementRun( - status="ready", - moves=[{"image_id": 7, "from": "/images/Conto/x.png", "to": "/images/conto/x.png"}], - planned_count=1, - ) - db.add(run) - await db.commit() - - resp = await client.get(f"/api/cleanup/placement/runs/{run.id}") - assert resp.status_code == 200 - body = await resp.get_json() - assert body["moves"][0]["from"] == "/images/Conto/x.png" - assert body["moves"][0]["to"] == "/images/conto/x.png" - - -@pytest.mark.asyncio -async def test_run_detail_404s_for_an_unknown_run(client): - resp = await client.get("/api/cleanup/placement/runs/999999") - assert resp.status_code == 404 - - -@pytest.mark.asyncio -async def test_plan_rejects_a_non_integer_artist(client): - resp = await client.post( - "/api/cleanup/placement/plan", json={"artist_id": "conto"}, - ) - assert resp.status_code == 400 - assert (await resp.get_json())["error"] == "invalid_artist_id" - - -@pytest.mark.asyncio -async def test_plan_accepts_an_artist_scope(client, db, monkeypatch): - sent = {} - from backend.app.tasks import library_placement - - monkeypatch.setattr( - library_placement.plan_placement, "delay", - lambda artist_id=None: sent.update(artist_id=artist_id), - ) - artist = Artist(name="Conto", slug="conto") - db.add(artist) - await db.commit() - - resp = await client.post( - "/api/cleanup/placement/plan", json={"artist_id": artist.id}, - ) - assert resp.status_code == 202 - assert sent["artist_id"] == artist.id - - -@pytest.mark.asyncio -async def test_apply_refuses_a_run_that_is_not_ready(client, db): - """The gate is here as well as in the service — an applied run must not - be re-applied by a stray POST.""" - run = _run(db, status="applied") - await db.commit() - - resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/apply") - assert resp.status_code == 400 - assert (await resp.get_json())["error"] == "not_ready" - - -@pytest.mark.asyncio -async def test_revert_refuses_a_run_that_was_never_applied(client, db): - run = _run(db, status="ready") - await db.commit() - - resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/revert") - assert resp.status_code == 400 - assert (await resp.get_json())["error"] == "not_applied" - - -@pytest.mark.asyncio -async def test_apply_dispatches_for_a_ready_run(client, db, monkeypatch): - sent = {} - from backend.app.tasks import library_placement - - monkeypatch.setattr( - library_placement.apply_placement, "delay", - lambda run_id: sent.update(run_id=run_id), - ) - run = _run(db, status="ready") - await db.commit() - - resp = await client.post(f"/api/cleanup/placement/runs/{run.id}/apply") - assert resp.status_code == 202 - assert sent["run_id"] == run.id - # Dispatch only — the endpoint must not have moved anything itself. - still = (await db.execute( - select(LibraryPlacementRun.status) - .where(LibraryPlacementRun.id == run.id) - )).scalar_one() - assert still == "ready" diff --git a/tests/test_library_layout.py b/tests/test_library_layout.py deleted file mode 100644 index 1594082..0000000 --- a/tests/test_library_layout.py +++ /dev/null @@ -1,403 +0,0 @@ -"""Milestone #421 — the shared predicate behind the consolidation. - -`destination_for` is pure and tested without a database; `survey_layout` gets -the integration treatment because the counts are the number the apply is -checked against. -""" - -from pathlib import Path - -import pytest -from sqlalchemy import select - -from backend.app.models import Artist, ImageRecord -from backend.app.services.library_layout import ( - RESERVED_TOP_LEVEL, - _misplaced_conditions, - canonical_dir, - destination_for, - survey_layout, -) - -ROOT = Path("/images") - - -# --- destination_for (pure) ------------------------------------------------- - - -def test_destination_rewrites_only_the_artist_segment(): - assert destination_for( - "/images/Conto/patreon/2026-01_a_Post/x.png", ROOT, "conto" - ) == Path("/images/conto/patreon/2026-01_a_Post/x.png") - - -def test_destination_is_identity_for_a_row_already_in_place(): - p = "/images/conto/patreon/x.png" - assert destination_for(p, ROOT, "conto") == Path(p) - - -def test_destination_pulls_a_root_level_row_under_its_artist(): - """Diverges from canonical_subdir deliberately: the row CARRIES an - artist_id, so a file at the root is an anomaly with a known home.""" - assert destination_for("/images/loose.png", ROOT, "conto") == Path( - "/images/conto/loose.png" - ) - - -def test_destination_refuses_paths_outside_the_images_root(): - assert destination_for("/srv/elsewhere/x.png", ROOT, "conto") is None - - -@pytest.mark.parametrize("reserved", sorted(RESERVED_TOP_LEVEL)) -def test_destination_refuses_the_reserved_stores(reserved): - """Relocating these would move the thumbnail cache, the attachment blobs - or the credential key into an artist folder.""" - assert destination_for(f"/images/{reserved}/aa/x.png", ROOT, "conto") is None - - -def test_destination_is_idempotent(): - once = destination_for("/images/Conto/patreon/x.png", ROOT, "conto") - assert destination_for(str(once), ROOT, "conto") == once - - -# --- the predicate ---------------------------------------------------------- - - -def test_canonical_prefix_carries_a_separator(): - """Without the trailing slash, artist `ara` matches every path under - `arbuzbudesh/` — one artist reads as fully placed while another's rows - are silently skipped.""" - conds = _misplaced_conditions(ROOT, 1, "ara") - rendered = str(conds[-1].compile(compile_kwargs={"literal_binds": True})) - assert "/images/ara/" in rendered - - -# --- survey_layout (integration) -------------------------------------------- -# -# Marked per-test rather than with a module-level `pytestmark`: the -# destination_for cases above are pure and belong in the fast unit lane. - - -def _artist(db, name, slug): - a = Artist(name=name, slug=slug) - db.add(a) - db.flush() - return a - - -def _image(db, path, artist=None, n=0): - rec = ImageRecord( - path=path, sha256=f"{n:064d}", size_bytes=1, mime="image/png", - width=10, height=10, origin="imported_filesystem", - integrity_status="unknown", - artist_id=artist.id if artist else None, - ) - db.add(rec) - db.flush() - return rec - - -@pytest.mark.integration -def test_survey_splits_canonical_from_misplaced(db_sync): - conto = _artist(db_sync, "Conto", "conto") - _image(db_sync, "/images/conto/patreon/a.png", conto, 1) - _image(db_sync, "/images/Conto/patreon/b.png", conto, 2) - _image(db_sync, "/images/Conto/patreon/c.png", conto, 3) - - report = survey_layout(db_sync, ROOT, check_disk=False) - - assert report.misplaced_rows == 2 - assert report.canonical_rows == 1 - row = next(a for a in report.artists if a.slug == "conto") - assert row.stray_dirs == ["Conto"] - - -@pytest.mark.integration -def test_survey_does_not_confuse_a_prefix_sharing_artist(db_sync): - """`ara` vs `arbuzbudesh` — the reason the predicate anchors on a - separator. Both are real artists in the operator's library.""" - ara = _artist(db_sync, "Ara", "ara") - arbuz = _artist(db_sync, "ArbuzBudesh", "arbuzbudesh") - _image(db_sync, "/images/ara/x.png", ara, 4) - _image(db_sync, "/images/arbuzbudesh/y.png", arbuz, 5) - - report = survey_layout(db_sync, ROOT, check_disk=False) - - assert report.misplaced_rows == 0 - assert report.canonical_rows == 2 - - -@pytest.mark.integration -def test_survey_counts_two_rows_landing_on_one_destination(db_sync): - """A collision is the case the apply must refuse, so the report has to - surface it rather than promise a move that cannot happen.""" - sticky = _artist(db_sync, "StickySpoodge", "stickyspoodge") - _image(db_sync, "/images/StickySpoodge/p/dup.png", sticky, 6) - _image(db_sync, "/images/Stickyspoodge/p/dup.png", sticky, 7) - - report = survey_layout(db_sync, ROOT, check_disk=False) - - assert report.collision_count == 1 - row = next(a for a in report.artists if a.slug == "stickyspoodge") - assert row.collisions == ["/images/stickyspoodge/p/dup.png"] - assert row.stray_dirs == ["StickySpoodge", "Stickyspoodge"] - - -@pytest.mark.integration -def test_survey_reports_unattributed_rows_without_moving_them(db_sync): - """The 660 loose root files have no artist_id, so no predicate reaches - them. They are counted, and left for task #4247.""" - _image(db_sync, "/images/orphan.png", None, 8) - - report = survey_layout(db_sync, ROOT, check_disk=False) - - assert report.unattributed_rows == 1 - assert report.misplaced_rows == 0 - - -@pytest.mark.integration -def test_survey_refuses_a_row_under_a_reserved_store(db_sync): - thumbs = _artist(db_sync, "Thumbsy", "thumbsy") - _image(db_sync, "/images/thumbs/aa/weird.png", thumbs, 9) - - report = survey_layout(db_sync, ROOT, check_disk=False) - - assert report.unmovable == 1 - row = next(a for a in report.artists if a.slug == "thumbsy") - assert row.misplaced_rows == 1 - assert row.collisions == [] - - -@pytest.mark.integration -def test_survey_counts_a_missing_source_file(db_sync, tmp_path): - """check_disk is what separates "would move" from "can move".""" - gone = _artist(db_sync, "Gone", "gone") - _image(db_sync, str(tmp_path / "Gone" / "missing.png"), gone, 10) - - report = survey_layout(db_sync, tmp_path, check_disk=True) - - assert report.missing_files == 1 - - -@pytest.mark.integration -def test_survey_flags_a_destination_that_already_exists(db_sync, tmp_path): - occupied = _artist(db_sync, "Occupied", "occupied") - src = tmp_path / "Occupied" / "x.png" - src.parent.mkdir(parents=True) - src.write_bytes(b"src") - dest = canonical_dir(tmp_path, "occupied") / "x.png" - dest.parent.mkdir(parents=True) - dest.write_bytes(b"already here") - _image(db_sync, str(src), occupied, 11) - - report = survey_layout(db_sync, tmp_path, check_disk=True) - - assert report.collision_count == 1 - assert dest.read_bytes() == b"already here" # read-only: nothing moved - - -@pytest.mark.integration -def test_survey_is_read_only(db_sync, tmp_path): - a = _artist(db_sync, "Reader", "reader") - src = tmp_path / "Reader" / "x.png" - src.parent.mkdir(parents=True) - src.write_bytes(b"x") - rec = _image(db_sync, str(src), a, 12) - before = rec.path - - survey_layout(db_sync, tmp_path, check_disk=True) - - db_sync.expire_all() - assert db_sync.get(ImageRecord, rec.id).path == before - assert src.exists() - assert db_sync.execute( - select(ImageRecord.path).where(ImageRecord.id == rec.id) - ).scalar_one() == before - - -# --- plan / apply / revert (#4246) ------------------------------------------ - - -def _staged(db, tmp_path, slug, stray, name="x.png", n=100): - """An artist with one file sitting in `stray`'s directory.""" - artist = _artist(db, slug.title(), slug) - src = tmp_path / stray / name - src.parent.mkdir(parents=True, exist_ok=True) - src.write_bytes(b"pixels") - rec = _image(db, str(src), artist, n) - return artist, rec, src - - -@pytest.mark.integration -def test_plan_records_where_each_file_came_from(db_sync, tmp_path): - from backend.app.services.library_layout import plan_placement - - _, rec, src = _staged(db_sync, tmp_path, "conto", "Conto", n=20) - run = plan_placement(db_sync, tmp_path) - - assert run.status == "ready" - assert run.planned_count == 1 - assert run.moves == [{ - "image_id": rec.id, - "from": str(src), - "to": str(tmp_path / "conto" / "x.png"), - }] - # Planning touches nothing. - assert src.exists() - assert db_sync.get(ImageRecord, rec.id).path == str(src) - - -@pytest.mark.integration -def test_apply_moves_file_and_row_together(db_sync, tmp_path): - from backend.app.services.library_layout import apply_run, plan_placement - - _, rec, src = _staged(db_sync, tmp_path, "conto", "Conto", n=21) - run = apply_run(db_sync, plan_placement(db_sync, tmp_path)) - - dest = tmp_path / "conto" / "x.png" - assert run.status == "applied" - assert run.moved_count == 1 and run.refused_count == 0 - assert dest.exists() and not src.exists() - db_sync.expire_all() - assert db_sync.get(ImageRecord, rec.id).path == str(dest) - - -@pytest.mark.integration -def test_revert_puts_it_back(db_sync, tmp_path): - """The whole reason `from` is retained: do one artist, look, undo.""" - from backend.app.services.library_layout import ( - apply_run, - plan_placement, - revert_run, - ) - - _, rec, src = _staged(db_sync, tmp_path, "conto", "Conto", n=22) - run = revert_run(db_sync, apply_run(db_sync, plan_placement(db_sync, tmp_path))) - - assert run.status == "reverted" - assert src.exists() - assert not (tmp_path / "conto" / "x.png").exists() - db_sync.expire_all() - assert db_sync.get(ImageRecord, rec.id).path == str(src) - - -@pytest.mark.integration -def test_apply_refuses_a_row_that_moved_since_planning(db_sync, tmp_path): - """A supersede or an earlier run can rewrite a path between plan and - apply. The stale entry is declined, not forced.""" - from backend.app.services.library_layout import apply_run, plan_placement - - _, rec, src = _staged(db_sync, tmp_path, "conto", "Conto", n=23) - run = plan_placement(db_sync, tmp_path) - - elsewhere = tmp_path / "conto" / "already-here.png" - elsewhere.parent.mkdir(parents=True, exist_ok=True) - elsewhere.write_bytes(b"pixels") - rec.path = str(elsewhere) - db_sync.flush() - - run = apply_run(db_sync, run) - - assert run.moved_count == 0 and run.refused_count == 1 - assert run.refusals[0]["reason"] == "row moved since planning" - assert src.exists() # untouched - - -@pytest.mark.integration -def test_apply_never_overwrites_an_occupied_destination(db_sync, tmp_path): - from backend.app.services.library_layout import apply_run, plan_placement - - _, rec, src = _staged(db_sync, tmp_path, "conto", "Conto", n=24) - run = plan_placement(db_sync, tmp_path) - - squatter = tmp_path / "conto" / "x.png" - squatter.parent.mkdir(parents=True, exist_ok=True) - squatter.write_bytes(b"someone else") - - run = apply_run(db_sync, run) - - assert run.refused_count == 1 - assert run.refusals[0]["reason"] == "destination occupied" - assert squatter.read_bytes() == b"someone else" - db_sync.expire_all() - assert db_sync.get(ImageRecord, rec.id).path == str(src) - - -@pytest.mark.integration -def test_apply_leaves_the_row_alone_when_the_source_is_gone(db_sync, tmp_path): - from backend.app.services.library_layout import apply_run, plan_placement - - _, rec, src = _staged(db_sync, tmp_path, "conto", "Conto", n=25) - run = plan_placement(db_sync, tmp_path) - src.unlink() - - run = apply_run(db_sync, run) - - assert run.refusals[0]["reason"] == "source missing" - db_sync.expire_all() - # The row still points at the missing file rather than at a file that - # was never created — a broken row is recoverable, a lying one is not. - assert db_sync.get(ImageRecord, rec.id).path == str(src) - - -@pytest.mark.integration -def test_plan_scopes_to_one_artist(db_sync, tmp_path): - """Per-artist scope is what makes this incremental instead of one - irreversible sweep.""" - from backend.app.services.library_layout import plan_placement - - conto, _, _ = _staged(db_sync, tmp_path, "conto", "Conto", n=26) - _staged(db_sync, tmp_path, "maewix", "Maewix", name="y.png", n=27) - - run = plan_placement(db_sync, tmp_path, artist_id=conto.id) - - assert run.planned_count == 1 - assert run.artist_id == conto.id - assert "Conto" in run.moves[0]["from"] - - -@pytest.mark.integration -def test_plan_skips_both_rows_when_two_want_one_destination(db_sync, tmp_path): - """Which of two colliding rows 'wins' is not this sweep's call.""" - from backend.app.services.library_layout import plan_placement - - artist = _artist(db_sync, "Sticky", "sticky") - for stray, n in (("StickySpoodge", 28), ("Stickyspoodge", 29)): - p = tmp_path / stray / "dup.png" - p.parent.mkdir(parents=True, exist_ok=True) - p.write_bytes(b"pixels") - _image(db_sync, str(p), artist, n) - - run = plan_placement(db_sync, tmp_path) - - assert run.planned_count == 0 - - -@pytest.mark.integration -def test_thumbnails_do_not_move(db_sync, tmp_path): - """Thumbs are sha-addressed (`thumbs//.jpg`), not path-keyed, so - a placement move must not touch them. Pinned so nobody 'fixes' it.""" - from backend.app.services.library_layout import apply_run, plan_placement - - artist, rec, _ = _staged(db_sync, tmp_path, "conto", "Conto", n=30) - thumb = tmp_path / "thumbs" / "ab" / "abc.jpg" - thumb.parent.mkdir(parents=True, exist_ok=True) - thumb.write_bytes(b"thumb") - rec.thumbnail_path = str(thumb) - db_sync.flush() - - apply_run(db_sync, plan_placement(db_sync, tmp_path)) - - db_sync.expire_all() - assert thumb.exists() - assert db_sync.get(ImageRecord, rec.id).thumbnail_path == str(thumb) - - -@pytest.mark.integration -def test_apply_refuses_a_run_that_is_not_ready(db_sync, tmp_path): - from backend.app.services.library_layout import apply_run, plan_placement - - _staged(db_sync, tmp_path, "conto", "Conto", n=31) - run = apply_run(db_sync, plan_placement(db_sync, tmp_path)) - with pytest.raises(ValueError): - apply_run(db_sync, run)