fix(bitdex): review round 3 — a comment my own fix broke, and a surviving mutant

Round-2 re-review of all five lanes at 47d883171b. Findings and remedies.

🔴 My round-1 fix broke the comment it repaired. metric-helpers.ts lost the
closing paren and the end of its sentence when the deleted endpoint name was cut
out, fusing two independent sentences into one unparseable line. Flagged by three
lanes independently. This is the class where fix rounds are more dangerous than
the diff they fix, and it happened on comment text where nothing typechecks.

🔴 A mutation of `useIndex` survives every test in the repo: dropping the
`!!input.hubId ||` arm passes 247 tests across the controller suites. That leaves
hub routing unpinned — with the index flag off a hub would route to getAllImages,
which throws rather than leaking, so the safety net holds while the routing
decision does not. Added a test; verified it kills the mutant rather than
assuming: `Tests 1 failed | 7 passed (8)`, failing test named, mutant reverted.

The first attempt at that control silently did not apply — a quoting error meant
the mutation was never written and the suite passed. A control that did not run
is indistinguishable from one that passed, so it is recorded here.

My FEEDBACK_AREAS comment named a mechanism that does not exist. It claimed
getFeedbackAreaSchema validates a moderator's query against the enum; that route
reads a Flipt flag and never touches a Feedback row, and there is no read path
over that table anywhere in the repo. Replaced with the true reason — the column
stores the label and this list is the only place the valid ones are written down
— and named the test that actually enforces the keep.

Stale comments the lanes caught, all describing deleted code as though it were
live: the ImagesInfinite JSDoc for a prop this PR removed; the `source` stamp in
image.controller.ts and its test docstring, both justified by a feed notice that
no longer exists; hubFilterArms' "both backends build their own clause syntax";
and the same claim in hub-feed-filter.test.ts.

image_post_triggers.sql is live programmability, not a record, and three of its
comments justified surviving design decisions by a BitDex sync trigger that was
dropped from prod tonight. Rewritten around the reasons that survive — the ~92M
unbackfilled rows and Meili's incremental sync window — with the rejected
alternative explicitly marked as not re-derived rather than left standing on a
dead premise.

Deferred, named rather than done: the two hub blocks in image.service.ts are now
byte-identical 15-line duplicates that were three copies with three clause
syntaxes. Collapsing them is the one reuse change worth making, but it is a
refactor on the hot feed path inside a removal PR.

Verified: typecheck 0 errors in 158s. Covering suites 95 passed (4 files), exit 0.

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

packages/civitai-db-schema/prisma/programmability/image_post_triggers.sql

@@ -17,24 +17,26 @@
 -- Why the BEFORE trigger fires on ALL updates, not just {scannedAt, postId}:
 -- there is deliberately NO backfill of the ~92M historical rows still holding a
 -- stale default-now() sortAt (Zuri, 2026-07-16: 88.1% mismatch ⇒ ~92M-row
--- rewrite, 200-400GB WAL — cancelled). The sortAt column is NOT NULL, so a
--- COALESCE(NEW.sortAt, …) belt in the downstream bitdex sync trigger cannot fall
--- back — it would read and emit the stale value. Recomputing on every write means
--- an unrelated UPDATE (e.g. an nsfwLevel-only edit) repairs the row's sortAt
--- before the sync trigger's emission expression reads it. A column-listed trigger
--- would leave those rows stale until they happened to receive a scannedAt/postId
--- write.
+-- rewrite, 200-400GB WAL — cancelled). Recomputing on every write means an
+-- unrelated UPDATE (e.g. an nsfwLevel-only edit) repairs the row's sortAt in
+-- passing. A column-listed trigger would leave those ~92M rows stale until they
+-- happened to receive a scannedAt/postId write.
+-- (This paragraph originally justified the belt against a BitDex sync trigger's
+-- COALESCE fallback. BitDex was decommissioned 2026-09-01 and that trigger is
+-- gone; the unbackfilled rows are the surviving reason.)
 --
 -- Fight / recursion: the Post fan-out's UPDATE (sortAt, updatedAt) now DOES fire
 -- the BEFORE trigger. No fight — at fan-out time the Post row already holds the
 -- new publishedAt (AFTER trigger), so set_image_sort_at recomputes the IDENTICAL
 -- GREATEST value the fan-out's SET clause used, and leaves updatedAt untouched. No
 -- recursion — a BEFORE trigger only mutates NEW in place; it issues no new UPDATE.
--- The fan-out still WRITES sortAt (rather than only bumping updatedAt and letting
--- the BEFORE trigger compute it) because the bitdex sync trigger detects publish
--- changes via OLD≠NEW on the stored column; a purely-computed emission expression
--- would evaluate the Post subselect identically for OLD and NEW at fire time and
--- MISS the publish (rejected alternative).
+-- The fan-out still WRITES sortAt rather than only bumping updatedAt. ⚠️ Its
+-- original reason was a BitDex sync trigger that detected publish changes via
+-- OLD≠NEW on the stored column; BitDex was decommissioned 2026-09-01 and that
+-- trigger no longer exists. The write is KEPT because the stored column is what
+-- Meili's incremental sync reads (see the fan-out author note below), but the
+-- rejected alternative — letting the BEFORE trigger compute it — has not been
+-- re-derived against the surviving consumers. Do that before changing this.
 
 -- Retire the 2024 predecessors (migration 20240719172747). Back then two authors
 -- wrote sortAt: update_image_sort_at() (Post side, later neutered by this file to
@@ -78,8 +80,8 @@ EXECUTE FUNCTION set_image_sort_at();
 -- 2. Fan-out author. A Post's publishedAt moving (publish, schedule, reschedule,
 --    unpublish — including the model/version publish/unpublish flows, which all
 --    rewrite Post.publishedAt) restamps every image on that post. It WRITES sortAt
---    (not a bare updatedAt touch) so the bitdex sync trigger sees OLD≠NEW on the
---    column and emits the publish change (see header). The BEFORE trigger re-fires
+--    (not a bare updatedAt touch); see the header note on why that write is kept
+--    now its original consumer is gone. The BEFORE trigger re-fires
 --    on this UPDATE and recomputes the same value — harmless.
 --    UNCONDITIONAL (no IS DISTINCT FROM guard): every image on the post gets its
 --    "updatedAt" bumped even when sortAt is unchanged. Meili's incremental image

src/components/Image/Infinite/ImagesInfinite.tsx

@@ -42,11 +42,6 @@ type ImagesInfiniteProps = {
   showAds?: boolean;
   showEmptyCta?: boolean;
   disableStoreFilters?: boolean;
-  /**
-   * Opt-in, because this component also backs image PICKERS (the collection
-   * add-content modal, challenge submission). Those are index-served too, but a
-   * "tell us if the feed looks off" box does not belong above a picker.
-   */
 } & Pick<ImagesContextState, 'collectionId' | 'judgeInfo' | 'judgingCategories'>;
 
 export default function ImagesInfinite(props: ImagesInfiniteProps) {

src/server/controllers/__tests__/image.controller.feed-source.test.ts

@@ -3,11 +3,11 @@ import type { Context } from '~/server/createContext';
 import type * as ImageService from '~/server/services/image.service';
 
 /**
- * The feed notice is gated on the `source` this handler emits, so the handler is
- * where the guard actually lives. Both branches must name themselves: an unnamed
- * DB page is indistinguishable from an index page that returned nothing, and the
- * client then cannot tell "routing changed mid-session" from "you reached the end
- * of the feed" — the first must take the notice down, the second must not.
+ * Which backend served the page is decided in this handler, so this is where the
+ * routing guard lives. Both branches must name themselves in `source`: an unnamed
+ * DB page is indistinguishable on the wire from an index page that returned nothing.
+ * No client consumes `source` today — these tests are what keep the two branches
+ * distinguishable.
  */
 
 const { getAllImagesMock, getAllImagesIndexMock } = vi.hoisted(() => ({
@@ -78,6 +78,31 @@ describe('getInfiniteImagesHandler names the backend that served the page', () =
     expect(result.source).toBe('db');
   });
 
+  // 🔴 The `!!input.hubId ||` arm of `useIndex`. Without this, deleting that arm
+  // passes every test in the repo — measured: 247 passed. A hub would then route to
+  // getAllImages with the index flag off, which throws rather than leaking, but the
+  // routing decision itself would be unpinned. `userHubs` is needed or the handler's
+  // own hub guard refuses before routing.
+  it('pins a hub to the index even with the index feed flag off', async () => {
+    const flagOffCtx = {
+      ...ctx,
+      features: {
+        ...(ctx as unknown as { features: object }).features,
+        imageIndexFeed: false,
+        userHubs: true,
+      },
+    } as typeof ctx;
+
+    const result = await getInfiniteImagesHandler({
+      input: { ...(indexBoundInput as object), hubId: 3 } as never,
+      ctx: flagOffCtx,
+    });
+
+    expect(getAllImagesIndexMock).toHaveBeenCalledTimes(1);
+    expect(getAllImagesMock).not.toHaveBeenCalled();
+    expect(result.source).toBe('meili');
+  });
+
   it('passes the index path’s own source through untouched', async () => {
     const result = await getInfiniteImagesHandler({ input: indexBoundInput, ctx });
 

src/server/controllers/image.controller.ts

@@ -345,9 +345,8 @@ export const getInfiniteImagesHandler = async ({
       });
       // Name this branch too, like the index path's `source`. Stamped here rather
       // than inside getAllImages because the index result type is derived from its
-      // return. Without it, a DB page and an index page that returned nothing are
-      // indistinguishable on the wire, and a client cannot tell a routing change
-      // mid-session from the end of the feed.
+      // return. No client reads it today — it is the wire contract, and the routing
+      // tests assert on it to tell a DB page from an index page that returned nothing.
       return { ...result, source: 'db' as const };
     }
   } catch (error) {

src/server/services/__tests__/hub-feed-filter.test.ts

@@ -238,8 +238,8 @@ describe('builders that cannot serve a hub refuse it', () => {
 });
 
 /**
- * The hub's own content cap (subtask 868kwp5f2). Each builder applies it and each
- * emits its own clause syntax, so a cap missing from one is a hub serving past its
+ * The hub's own content cap (subtask 868kwp5f2). Both builders apply it separately,
+ * so a cap missing from one is a hub serving past its
  * own setting on whichever backend that request happened to take — with nothing
  * red anywhere. Asserted on the EMITTED level list rather than on the call, because
  * the call is identical with and without the cap.

src/server/services/image.service.ts

@@ -3364,8 +3364,9 @@ async function resolvedHubSources(input: ImageSearchInput) {
 
 type HubFilterArm = { field: MetricsImageFilterableAttribute; ids: number[] };
 
-// The single enumeration of the arms a hub ORs together. Both backends build their
-// own clause syntax from this, so an arm added here cannot reach one of them only.
+// The single enumeration of the arms a hub ORs together. One builder consumes it
+// today (`buildHubFilter`); the split survives so a second clause syntax cannot be
+// given a different set of arms.
 // Returns null for "no arm", which callers must treat as "serve nothing"; treating
 // it as "no filter" hands the caller the global feed as their hub.
 function hubFilterArms(

src/server/utils/metric-helpers.ts

@@ -63,7 +63,8 @@ export const updateEntityMetric = async ({
   // used to live here was removed after the v2 + watcher cutover went permanent
   // (v5.0.1871). It wrote the in-app `entitymetric:Image:*` Redis cache, which
   // nothing reads anymore — image metric reads go through the watcher-fed
-  // `metrics:*` cache via MetricService (`getImageMetricsObject`,
+  // `metrics:*` cache via MetricService (`getImageMetricsObject`).
+  //
   // Comics no longer use this path either: every comic counter
   // (incl. reads) is now Postgres-owned via `ComicProjectMetric`, and the old
   // `entitymetric:Comic:*` Redis cache + its populator were removed.

src/shared/constants/feedback.constants.ts

@@ -2,10 +2,12 @@
 // migration, and its Flipt flag is derived from the slug.
 //
 // 🔴 `bitdex-image-feed` has no producer — BitDex was decommissioned 2026-09-01 and
-// its prompt went with it. KEEP IT ANYWAY. This enum is what `getFeedbackAreaSchema`
-// validates a moderator's query against, so deleting the slug makes every historical
-// feedback row filed under it unreadable through that route. It cannot grow: nothing
-// writes it any more.
+// its prompt went with it. KEEP IT ANYWAY. `Feedback.area` is a stored string column
+// and this list is the only place its valid labels are written down, so dropping the
+// slug orphans every historical row filed under it. Nothing in the app mounts a
+// prompt that writes it and its area flag is off, so it cannot grow.
+// Enforced: `feedback.schema.test.ts` compares a hand-typed list against this one,
+// so removing the slug fails there with an array diff rather than silently.
 export const FEEDBACK_AREAS = ['bitdex-image-feed', 'apps-marketplace'] as const;
 
 export type FeedbackArea = (typeof FEEDBACK_AREAS)[number];