Placement reconciler: put every image in its own artist's folder, one artist at a time #256

Merged

bvandeusen merged 5 commits from dev into main 2026-09-21 15:28:39 -04:00

Conversation 0 Commits 5 Files Changed 14

+1482 -3

bvandeusen commented 2026-09-21 15:28:33 -04:00

Owner

Copy Link

Copy Source

Five commits, milestone #421 step 3. dev tip 2dd9b95 green on both
workflows (runs 7197/7198).

What this is for

The survey (#4245) measured 33,789 of 63,605 images sitting in another
artist's folder, across 56 artists. Not a naming problem — the database has
one Artist row per artist and always did. _copy_to_library and _supersede
used to write to whatever folder the incoming FILE came from, so a larger
copy of a Telepurte piece found inside Ara/ moved the file there and left
the record correct. Both writers were fixed in #4244; this moves the backlog
home and gives the library a standing way to stay true.

Nothing here changes who an image belongs to

Folders remain human-readable artist names (conto/, big-bang/). No path
contains an id. The reconciler reads each record's artist, and puts the file
in that artist's folder. Attribution is never written.

Shape

LibraryPlacementRun (migration 0099) stores its plan as JSONB, and that one
structure does three jobs: the preview read before agreeing, the list the
apply executes (rather than re-deriving the set, so the two cannot
disagree), and — because from is retained — the undo.

The undo is the point. A 33,789-file operation becomes something to do one
artist at a time, look at in the gallery, and reverse if it reads wrong.

Everything fails closed. The apply re-checks each row against what the plan
recorded — source still there, destination still free, record unchanged —
because a download or supersede can land in between. A refusal is recorded
with its reason and the run continues. The record is updated only after its
rename lands, so a failed move can never leave path naming a file that is
not there. Two rows wanting one destination: neither is planned.

The ledger persists every 200 moves. A worker dying two thirds through must
not take the undo information for the first two thirds with it — that ledger
is the only record of where those files came from, which also means an
applied run is history, not state, and must never be pruned. The model
and the migration both say so.

Thumbnails are sha-addressed, not path-keyed, so they do not move (pinned by
a test).

UI

Maintenance → File placement. Check placement, then a per-artist table
with a Plan button; runs table with Review / Move files / Put back.

Runs are database rows, so no localStorage resurfacing (useMaintenanceTask)
is needed — a reload, another machine, or coming back tomorrow shows the same
state, because the state IS the row.

On deploy

0099 creates one empty table. Nothing moves until someone asks it to —
there is no beat entry and no automatic run. The intended first use is one
artist, then a look at the gallery.

Also in here

stubFetch extracted to frontend/test/stubFetch.js — the shape ledger
flagged it as byte-identical across five specs and this would have been the
sixth. The others keep their copies until each is next touched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR

Five commits, milestone #421 step 3. `dev` tip `2dd9b95` green on both workflows (runs 7197/7198). ## What this is for The survey (#4245) measured **33,789 of 63,605 images sitting in another artist's folder**, across 56 artists. Not a naming problem — the database has one Artist row per artist and always did. `_copy_to_library` and `_supersede` used to write to whatever folder the incoming FILE came from, so a larger copy of a Telepurte piece found inside `Ara/` moved the file there and left the record correct. Both writers were fixed in #4244; this moves the backlog home and gives the library a standing way to stay true. ## Nothing here changes who an image belongs to Folders remain human-readable artist names (`conto/`, `big-bang/`). No path contains an id. The reconciler reads each record's artist, and puts the file in that artist's folder. Attribution is never written. ## Shape `LibraryPlacementRun` (migration 0099) stores its plan as JSONB, and that one structure does three jobs: the **preview** read before agreeing, the list the **apply** executes (rather than re-deriving the set, so the two cannot disagree), and — because `from` is retained — the **undo**. The undo is the point. A 33,789-file operation becomes something to do one artist at a time, look at in the gallery, and reverse if it reads wrong. Everything fails closed. The apply re-checks each row against what the plan recorded — source still there, destination still free, record unchanged — because a download or supersede can land in between. A refusal is recorded with its reason and the run continues. The record is updated only after its rename lands, so a failed move can never leave `path` naming a file that is not there. Two rows wanting one destination: neither is planned. The ledger persists every 200 moves. A worker dying two thirds through must not take the undo information for the first two thirds with it — that ledger is the only record of where those files came from, which also means an **applied run is history, not state**, and must never be pruned. The model and the migration both say so. Thumbnails are sha-addressed, not path-keyed, so they do not move (pinned by a test). ## UI **Maintenance → File placement.** Check placement, then a per-artist table with a Plan button; runs table with Review / Move files / Put back. Runs are database rows, so no localStorage resurfacing (`useMaintenanceTask`) is needed — a reload, another machine, or coming back tomorrow shows the same state, because the state IS the row. ## On deploy 0099 creates one empty table. **Nothing moves until someone asks it to** — there is no beat entry and no automatic run. The intended first use is one artist, then a look at the gallery. ## Also in here `stubFetch` extracted to `frontend/test/stubFetch.js` — the shape ledger flagged it as byte-identical across five specs and this would have been the sixth. The others keep their copies until each is next touched. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR

bvandeusen added 5 commits 2026-09-21 15:28:33 -04:00

feat: placement reconciler — plan, apply, revert (4246, slice 3a) ... 9ccc460c69

Milestone #421 step 3, reframed on the operator's steer: not a one-off
migration but the system that keeps the tree true. The 33,789 misplaced rows
the survey found are just its first run.

The placement half was already done, verified by reading each writer rather
than assuming: downloads have always written `<root>/<slug>/<platform>/`
(gallery_dl.py:523), attach_in_place leaves files where the downloader put
them, and `_copy_to_library` / `_supersede` became canonical in #4244. So
nothing is written off-canon today; what remains is the backlog and a standing
check for future drift.

`LibraryPlacementRun` (migration 0099) holds the plan as JSONB, and that one
structure does three jobs: it is the PREVIEW the operator reads, the list the
APPLY executes (rather than re-deriving the set, so the two cannot disagree),
and — because `from` is retained — the UNDO.

The undo is the point. It makes a 33,789-file operation something to do one
artist at a time, look at in the gallery, and reverse if it reads wrong. That
settles whether artist_id or the folder held the truth (spike #4257) by doing
rather than by arguing it from a 50-row sample.

An applied run is therefore HISTORY, not state — lesson #4226's trap, since
it is the only record of where those files used to be. The model and the
migration both say so: any future retention here may prune ready/cancelled/
error runs, never an applied one.

Everything fails closed. The apply re-checks each row against what the plan
recorded — source still there, destination still free, row still pointing
where the plan said — because a download or a supersede can land in between.
A refusal is recorded with its reason and the run continues; one stale row is
not a reason to abandon the other 33,788. The row is updated only after its
rename lands, so a failed move can never leave `path` naming a file that is
not there.

Writing the collision test caught the code disagreeing with its own comment:
it claimed the first of two rows wanting one destination and skipped the
second, silently picking a winner by iteration order. Now it counts first and
filters after, so genuinely neither is planned.

Thumbnails are sha-addressed, not path-keyed, so they do not move — pinned by
a test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR

feat: placement reconciler tasks + API (4246, slice 3b) ... abe449b4f2

Three Celery tasks wrapping the 3a service, and the endpoints that drive
them. Routed to `maintenance_long` alongside backups: 33k renames on NFS have
no business in the quick lane where the self-healing sweeps live (the
2026-06-07 starvation).

A durability bug in 3a, found by thinking about what a crash costs rather
than by a failing test: `apply_run` wrote its ledger only at the end, so a
worker dying at row 30,000 of 33,789 would have taken the undo information
for the first 29,999 with it — and that ledger is the ONLY record of where
those files came from. It now persists every 200 moves. Two things fell out
of writing that:

- `_persist` reassigns `run.moves`, so the loop had to snapshot the plan
  first rather than iterate the attribute it rewrites.
- the reassignment is itself load-bearing: SQLAlchemy does not track in-place
  mutation of a JSONB list, so an `.append()` alone would never reach the
  database and the ledger would have stayed silently empty.

Re-running a partially-applied plan is safe — the moved rows no longer match
their `from` and refuse as "row moved since planning" — but `apply_placement`
deliberately has NO autoretry: re-entering a half-applied plan should be the
operator's call after reading what happened, not the queue's.

Endpoints gate on run state as well as the service does, so a stray POST
cannot re-apply an applied run. The list response omits `moves` (an applied
whole-library run carries tens of thousands of entries); the detail endpoint
includes them, because that detail IS the preview read before agreeing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR

test: placement API tests must commit, not flush (4246) ... db63377754

Five of the new tests failed in CI: every one that created a
LibraryPlacementRun and then read it back through the client. The ones that
touched no rows passed.

A flush stays inside the test's own transaction, and the app under test runs
on a separate session and connection — so the endpoint queried a database
where the row did not exist yet and got its 404 / empty list honestly.
`_seed_runs` in test_api_system_backup already commits for this reason; the
idiom was there to copy and I did not look first.

Recorded in the helper's docstring rather than just fixed, since the next
person writing a create-then-fetch API test will reach for flush too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR

test: import the placement task module so its names reach celery.tasks (4246) ... a4bdbcaca4

Three registration assertions failed: a task name only enters `celery.tasks`
when its module is imported, and nothing in the test process imported
`library_placement`. The API routes import it lazily inside the handlers, and
the registration test runs before any handler test triggers that.

`include=[...]` is what gets the module imported in a real WORKER, so
production registration was never in question — the test was asserting
something only observable after an import it never performed.

test_tasks_admin already carries the convention verbatim
(`import backend.app.tasks.admin  # noqa: F401 — register tasks`); I wrote the
assertion from what I meant instead of copying the idiom next to it. Same
mistake shape as the commit-vs-flush bounce one commit ago: the pattern was
already in the suite both times.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR

feat: File placement card — survey, plan per artist, apply, put back (4246, slice 3c) ... 2dd9b956d5

The UI half of the reconciler, in Maintenance. Survey shows how many images
sit in the wrong artist's folder and which folders they are in; each artist
gets its own Plan button; each run can be reviewed, applied, and put back.

Deliberately NOT using useMaintenanceTask. That composable stashes a task id
in localStorage so a result survives navigate-away, which is the right answer
when the only record is a Celery result. Here the runs are database rows — so
a reload, another machine, or coming back tomorrow simply shows the same
state, because the state IS the row. The card polls the runs endpoint instead.

Copy avoids the vocabulary this work has been tripping over: "folder", "put
back", "in the wrong folder" rather than artist_id, revert and canonical. The
one thing the operator most needs to know — nothing here changes who an image
belongs to — is what the blurb says first.

Three things I had assumed and checked instead: MaintenanceTile lives in
common/ not settings/; there is no generic ConfirmDialog (BackupCard uses a
purpose-built modal), so this uses a plain v-dialog; and `loadArtistNames`
did not exist — it does now, mapping id to name so a run row reads "Conto"
rather than "#47". The name deliberately is not denormalised into the run: it
belongs to the artist and would go stale on a rename.

Also extracted `stubFetch` to frontend/test/stubFetch.js. The shape ledger
flagged it as a byte-identical duplicate across five specs and this would
have been the sixth; the others keep their copies until each is next touched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR

bvandeusen merged commit 32d37cd453 into main 2026-09-21 15:28:39 -04:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate-bot bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/FabledCurator#256