Eine Bildmeldung fragt, ob das Herunternehmen die Bildart kennt (#2058)

Das Tor vor der Meldung leitete sich aus `@mirrored` ab, also aus der Liste,
die eine Contract-Freigabe löscht. Sobald eine Bildart ihren Umzug in die
gemeinsame Tabelle beendet, hätte es „ja" gesagt, während nichts diese Art
herunternehmen kann: die Meldung wird angenommen, und die Bestätigung wirft
der Administratorin eine Fehlerseite hin. Tor, Einfrieren, Auftauen und Löschen
lesen jetzt dieselbe Karte `@takedown`, und ein Test friert jede Art wirklich
ein, die das Tor zulässt. Die drei Freigaben, die der Umzug einer Art braucht,
stehen jetzt einmal und eindeutig in `docs/architecture/images.md`.

Diesen Text hat ein KI-Agent in meinem Namen geschrieben. Ich weiß, dass das problematisch ist.

Claude-Session: https://claude.ai/code/session_01UGPe7bHVS11M1W7fNehSku

Author: wintermeyer

docs/architecture/images.md

@@ -345,8 +345,9 @@ that already knows.
 `bin/vutuv eval "Vutuv.Release.check_image_rows()"`) is the gate before the
 cut: it counts every picture against its row *and* against its file on
 disk — the quarantine tree while the picture is `"pending"`, the served tree
-otherwise — prints one line per kind plus the members behind each class of
-mismatch, and **fails the command** when anything is outstanding. Both of
+otherwise — prints one line per kind plus a bounded sample of the ids behind
+each class of mismatch (`Backfill.check/1`'s own doc names what each class is
+sampled by), and **fails the command** when anything is outstanding. Both of
 those come from `check/1` itself rather than from either entry point, because
 the first version put the printing in the mix task alone: it fell out of step
 with the shape `check/1` returns and crashed on every invocation, while the
@@ -421,9 +422,9 @@ default is one nobody knows how to take offline.
 
 A post photo, an organization image, a job-posting picture and a review cover
 each kept a table and an uploader of their own, so the `image` report type and
-the freeze knew one kind only. They move in one release per kind, smallest
-first — a **job-posting picture** went first (#2054) precisely to settle the
-shape.
+the freeze knew one kind only. They move one kind at a time and three releases
+per kind (the sequence is spelled out below), smallest first — a **job-posting
+picture** went first (#2054) precisely to settle the shape.
 
 **Three of the four are the same shape; the review cover is not.** A post
 photo, an organization image and a job-posting picture each have a row of their
@@ -432,7 +433,9 @@ is `cover` / `cover_status` / `cover_moderation` **columns on the review row**
 with no token and no table of its own (`Vutuv.Posts.PostReview`), which is the
 profile picture's shape, not this one — #2055 lands on `member_columns/0`'s side
 of the fence, and `Vutuv.Images.Backfill`'s `%{cols: …}` source is what it
-extends. What #2052 and #2053 copy from here is everything below.
+extends. One thing it does share with the three: a report cannot name one of
+its pictures until the kind has a strategy in `Vutuv.Images`'s `@takedown`.
+What #2052 and #2053 copy from here is everything below.
 
 **The token is the join key, not a pointer.** #2013 added
 `users.avatar_image_id` because a member row had no stable handle of its own;
@@ -500,17 +503,36 @@ it, and `Vutuv.Moderation` refuses a report that names one
 (`Vutuv.Images.takedown_ready?/1`), because a case opened on a row nothing
 consults would go through an uphold that takes nothing offline. A member
 reports the posting instead, which is all there was before the row existed.
-
-**Retiring `job_posting_images` is not this release.** It is the deploy after,
-and it has to remove the double write in the same step — the mirror, the
-`forget/2` calls, the `@gallery_sources` entry in the backfill and
-`Vutuv.Images.mirrored?/1`'s answer — because a migration may drop only what
-the currently deployed release has stopped using. Before it: run
-`mix vutuv.images.backfill --only job_posting_image` and read the check. The
-release in between is the one that moves the proxy, the form and the freeze
-onto the row, which is what makes the old table unread in the first place; and
-that release, not this one, is where the kind decides whether it wants a
-**bridge** the way `member_image/2` has one.
+That gate reads `@takedown`, the map naming which kinds have a takedown at all,
+so it turns yes in step 2 below and not a moment earlier (issue #2057).
+
+**A gallery kind moves in three releases, and #2054 was the first.** Each is
+N-1 safe on its own and no two can be merged; this milestone has already paid
+for an off-by-one in that count once, in #2027.
+
+1. **Expand**: what #2054 shipped for `job_posting_image`. Every path that
+   touches the picture writes and drops the `images` row beside the old one,
+   while every reader, and the truth, stay in `job_posting_images`. Nothing
+   here can take such a picture offline yet: `freeze/1` raises for the row and
+   a report cannot name it. Between this release and the next, an operator runs
+   `mix vutuv.images.backfill --only <kind>` and reads its check; after step 2
+   the two copies can no longer be compared, so this is the last chance.
+2. **Move the readers, the writes and the takedown onto the row.** The proxy,
+   the edit form and the AI gate read the `images` row, and the context writes
+   that row directly, so the mirror goes out in the same change: the
+   `write_mirrored/2`, `mirror/2` and `forget/2` calls and the kind's entry in
+   `@mirrored` (`Vutuv.Images.mirror_source/1`, `mirrored?/1`). *The backfill
+   needs nothing removed*, at this step or any other: `Backfill.kinds/0` is
+   `Vutuv.Images.mirrored_kinds/0` plus the profile kinds, and every gallery
+   source builds itself from `mirror_source/1`, so it holds no per-kind list of
+   its own. The kind also gets its takedown here: a strategy in `@takedown` and
+   the `freeze`/`unfreeze`/`purge` clauses that go with it, which is what opens
+   the report form on these pictures. No migration in this release: the old
+   table is still standing and the release one step back is still reading and
+   writing it.
+3. **Contract**: the migration that drops `job_posting_images`, and nothing
+   else. Step 2 is what stopped using the table, and step 2 is what serves
+   while this migration runs.
 
 **This kind needs no bridge.** #2027 needed one because it moved every reader
 onto the row in the same deploy as the row's first appearance, so a picture the

docs/architecture/moderation.md

@@ -143,6 +143,16 @@ report type for the picture itself: `fetch_content/2` resolves it through
 cover, and the ⋯ menu on a profile offers "Report the profile picture" beside
 the profile's own Report whenever that picture has a row.
 
+**Only a picture the freeze can act on may be reported** (issue #2057). A row
+in `images` is no longer proof of that: #2015 is moving four more picture kinds
+into the table one at a time, and each arrives with a row a release before
+anything can take it offline. So `reportable_by?/2` asks
+`Vutuv.Images.takedown_ready?/1`, which reads the same `@takedown` map the
+freeze itself dispatches on; the three releases of a kind's move are in
+[images.md](images.md). A kind it refuses keeps the affordance it had before
+its row existed: the member reports the post, the posting or the page the
+picture sits on.
+
 **Its freeze is a file move, not a column write.** `Vutuv.Images.freeze/1`
 stamps `frozen_at`, clears the member row's four columns for that kind, and
 moves every derived version, the private original and anything still in AI

lib/vutuv/images.ex

@@ -25,8 +25,10 @@ defmodule Vutuv.Images do
   a mirror, written by `mirror/2` and dropped by `forget/2`, joined on the
   `token` both carry rather than by a pointer. Nothing here reads it yet, which
   is why `freeze/1` raises for one and `takedown_ready?/1` answers false: a case
-  opened on a row nobody consults would take nothing offline. Moving the readers
-  across, and only then retiring the old table, is the deploy after.
+  opened on a row nobody consults would take nothing offline. **Such a kind
+  moves in three releases**: the mirror, then the one that moves the readers and
+  wires the takedown, then the migration that retires the old table.
+  `docs/architecture/images.md` spells the three out.
 
   `serving/1` is the second thing here that is not a column: how a kind reaches
   a reader decides what its off switch is, and a kind nobody has declared
@@ -66,7 +68,9 @@ defmodule Vutuv.Images do
   # compares the two and fails the build on it.
   #
   # An entry goes when that kind's contract release retires its old table,
-  # together with the double write; `takedown_ready?/1` below flips with it.
+  # together with the double write. It does **not** take the report gate with
+  # it: that reads `@takedown` below, which the release before that one, the one
+  # that moves the readers and wires the takedown, is what extends.
   # #2052 (post photos) and #2053 (organization images) add one entry each;
   # #2055 (review covers) does **not** — a review's cover is columns on the
   # review row with no token and no table, which is the `@profile_columns`
@@ -649,33 +653,44 @@ defmodule Vutuv.Images do
     :ok
   end
 
+  # Which takedown a kind gets, and (by the presence of a key) whether it has
+  # one at all. `takedown_ready?/1` and the three functions below all guard on
+  # `is_map_key(@takedown, kind)`, so the gate that lets a report name a picture
+  # and the code that takes it offline read one map and cannot answer
+  # differently. A kind arrives here in the same change as its strategy's
+  # clauses (issue #2057).
+  #
+  # The gate used to be derived from `@mirrored` instead, and the two agreed by
+  # arithmetic rather than by meaning: that entry is what a **contract** release
+  # deletes, so the gate would have opened on the deploy that retires a kind's
+  # old table whether or not anything had wired that kind's freeze, and the
+  # first report accepted on it would have raised in front of the admin who
+  # upheld it.
+  @takedown Map.new(@profile_kinds, &{&1, :profile})
+
   @doc """
   Whether a copyright case can act on this picture at all — what
   `Vutuv.Moderation` asks before letting a report name it.
 
-  A gallery kind that has only just arrived here (#2054 and its siblings ship
-  the **expand** half: the row exists, every URL and every gate still reads the
-  old table) has no takedown path yet, and `freeze/1` below has no clause for
-  it. Answering "yes" would open a case whose uphold raises, so a report is
-  refused until the kind's own contract release wires the freeze — which is all
-  there was before the row existed.
-
-  Derived from `mirrored?/1` rather than listed a second time, so the two
-  cannot drift: "this picture's truth is still elsewhere" and "nothing here can
-  take it offline" are one fact, and the contract release that deletes the
-  mirror entry flips both at once.
+  True for exactly the kinds `freeze/1` below can act on (`@takedown`), because
+  that is the fact the gate needs: a report accepted on a kind nothing can take
+  offline opens a case whose uphold raises. A gallery kind that has only just
+  arrived here (#2054 and its siblings ship the **expand** half: the row exists,
+  every URL and every gate still reads the old table) is therefore refused until
+  the release that wires its takedown, which leaves it the affordance it had
+  before the row existed: reporting the posting or the page the picture sits on.
   """
-  def takedown_ready?(%Image{kind: kind}), do: kind in @kinds and not mirrored?(kind)
+  def takedown_ready?(%Image{kind: kind}), do: is_map_key(@takedown, kind)
 
-  # A kind whose row exists but whose freeze does not. Loud rather than
+  # A kind whose row exists but whose takedown does not. Loud rather than
   # half-done: the alternative is a stamped `frozen_at` no reader consults and
   # files nothing moved, which reads from the case page exactly like a
   # completed takedown.
   defp no_takedown_path!(%Image{kind: kind}, action) do
     raise ArgumentError, """
-    cannot #{action} an image of kind #{inspect(kind)}: this release mirrors it \
-    into the images table but no reader consults the row yet, so nothing would \
-    go offline. Wire the kind's freeze before letting a case reach it.\
+    cannot #{action} an image of kind #{inspect(kind)}: nothing here can take a \
+    picture of that kind offline, so nothing would go offline. Give the kind a \
+    strategy in Vutuv.Images' @takedown before letting a case reach it.\
     """
   end
 
@@ -703,7 +718,14 @@ defmodule Vutuv.Images do
   already invisible and a job `reconcile_holds/0` finishes. The other order
   would leave files in a hold that nothing knows to bring back.
   """
-  def freeze(%Image{kind: kind} = image) when kind in @profile_kinds do
+  def freeze(%Image{kind: kind} = image) when is_map_key(@takedown, kind),
+    do: freeze_by(@takedown[kind], image)
+
+  def freeze(%Image{} = image), do: no_takedown_path!(image, "freeze")
+
+  # The profile strategy: the files are served straight off disk (`serving/1`
+  # answers `:static`), so taking the picture offline means moving them.
+  defp freeze_by(:profile, %Image{} = image) do
     # `is_nil(frozen_at)` so a second pass — `reconcile_holds/0` finishing an
     # interrupted move — re-asserts the freeze without moving the moment it
     # happened, which is what the case and the statement of reasons quote.
@@ -720,8 +742,6 @@ defmodule Vutuv.Images do
     :ok
   end
 
-  def freeze(%Image{} = image), do: no_takedown_path!(image, "freeze")
-
   @doc """
   Puts a frozen picture back exactly where it was: every file returns to the
   tree it came from under the name it had, the member row gets its four columns
@@ -738,7 +758,12 @@ defmodule Vutuv.Images do
   only once the member row names the files again, so a half-finished restore is
   still a hold for `reconcile_holds/0` to find.
   """
-  def unfreeze(%Image{kind: kind} = image) when kind in @profile_kinds do
+  def unfreeze(%Image{kind: kind} = image) when is_map_key(@takedown, kind),
+    do: unfreeze_by(@takedown[kind], image)
+
+  def unfreeze(%Image{} = image), do: no_takedown_path!(image, "unfreeze")
+
+  defp unfreeze_by(:profile, %Image{} = image) do
     {_count, _} =
       Repo.update_all(from(i in Image, where: i.id == ^image.id),
         set: [frozen_at: nil, updated_at: now()]
@@ -756,8 +781,6 @@ defmodule Vutuv.Images do
     :ok
   end
 
-  def unfreeze(%Image{} = image), do: no_takedown_path!(image, "unfreeze")
-
   @doc """
   Deletes this picture for good — every derived version, the private original
   and the held copies — and forgets the row. What an upheld copyright case does,
@@ -767,7 +790,12 @@ defmodule Vutuv.Images do
   nothing points at (which `reconcile_holds/0` collects), never a member row
   naming files that are gone.
   """
-  def purge(%Image{kind: kind} = image) when kind in @profile_kinds do
+  def purge(%Image{kind: kind} = image) when is_map_key(@takedown, kind),
+    do: purge_by(@takedown[kind], image)
+
+  def purge(%Image{} = image), do: no_takedown_path!(image, "purge")
+
+  defp purge_by(:profile, %Image{} = image) do
     case owner(image) do
       %User{} = user ->
         config = @profile_columns[image.kind]
@@ -785,8 +813,6 @@ defmodule Vutuv.Images do
     :ok
   end
 
-  def purge(%Image{} = image), do: no_takedown_path!(image, "purge")
-
   @doc """
   Finishes every move a dying slot left half-done, in both directions — the
   standing job behind `freeze/1` and `unfreeze/1`, run by

test/vutuv/moderation_image_takedown_test.exs

@@ -56,6 +56,8 @@ defmodule Vutuv.ModerationImageTakedownTest do
 
   defp reload(user), do: Repo.get!(User, user.id)
 
+  defp reload_row(%ImageRow{id: id}), do: Repo.get(ImageRow, id)
+
   defp notice(attrs \\ %{}) do
     Map.merge(
       %{
@@ -351,6 +353,65 @@ defmodule Vutuv.ModerationImageTakedownTest do
     end
   end
 
+  # `Images.takedown_ready?/1` decides whether a report may name a picture, and
+  # the freeze is what accepting one eventually runs — so a kind the gate admits
+  # and the freeze cannot act on is an error page in front of the admin who
+  # upholds the case. Both tests split `Images.kinds()` by the gate rather than
+  # naming today's three, so a kind that joins the table has to answer for
+  # itself (issue #2057).
+  describe "the gate and the takedown answer one question (issue #2057)" do
+    setup %{owner: owner} do
+      {:ok, owner} = Accounts.update_user(owner, %{cover_photo: jpeg_upload("wide.jpg")})
+
+      {ready, refused} =
+        Enum.split_with(Images.kinds(), &Images.takedown_ready?(%ImageRow{kind: &1}))
+
+      {:ok, owner: owner, ready: ready, refused: refused}
+    end
+
+    test "every kind it admits really freezes, unfreezes and purges",
+         %{owner: owner, ready: ready} do
+      # Real pictures, because "the takedown can act on it" is a claim about
+      # files moving, not about a clause existing.
+      real = Map.new(~w(avatar cover), &{&1, Images.profile_image(owner.id, &1)})
+
+      for kind <- ready do
+        row = Map.get(real, kind)
+
+        assert row,
+               "#{kind} is takedown-ready but nothing here takes one offline. Either " <>
+                 "it has no strategy in Vutuv.Images' @takedown, and then the gate " <>
+                 "must not admit it, or it has one and this test needs a real " <>
+                 "picture of that kind."
+
+        assert :ok = Images.freeze(row)
+        assert reload_row(row).frozen_at
+
+        assert :ok = Images.unfreeze(reload_row(row))
+        refute reload_row(row).frozen_at
+
+        assert :ok = Images.purge(reload_row(row))
+        refute reload_row(row)
+      end
+    end
+
+    test "every kind it refuses is refused by all three, loudly",
+         %{owner: owner, refused: refused} do
+      for kind <- refused do
+        # No row is needed: the guard refuses before anything is read.
+        row = %ImageRow{id: Vutuv.UUIDv7.generate(), kind: kind, user_id: owner.id}
+
+        for {action, fun} <- [
+              {"freeze", &Images.freeze/1},
+              {"unfreeze", &Images.unfreeze/1},
+              {"purge", &Images.purge/1}
+            ] do
+          assert_raise ArgumentError, ~r/cannot #{action} an image of kind/, fn -> fun.(row) end
+        end
+      end
+    end
+  end
+
   describe "the picture-row bookkeeping" do
     # A frozen picture's member columns are empty on purpose, which is exactly
     # what the backfill calls an orphan row — and the row is the only record of