mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
04b7cfa9d3
* wip(bitdex): excise the BitDex blocks from image.service.ts + delete dead files 🔴 INCOMPLETE — DOES NOT TYPECHECK. Committed so the analysis and the block excision survive the session, not because it is ready. Done: 18 files deleted (both jobs + tests, src/server/bitdex/, the two internal endpoints, bitdex-feed-serve.metrics + tests, six bitdex-* service tests), and 1,346 lines excised from image.service.ts — imports, the native filter helpers, postFilterBitdexDocs, isPublicallyPublished, isScheduledForFuture, BitdexCursoredPageUnavailable, fetchBitdexPrimary, mapBitdexDoc and getImagesFromBitdexPreFilter. Each cut by brace balance from its own declaration rather than by hand-typed line numbers. Verified before cutting: 11 of the 15 filter helpers and both post-filter predicates had no callers outside the removed blocks. Not done: ~52 in-place references in image.service.ts, the bitdexMode dispatch in getImagesFromSearch, image.controller.ts, image-search.service.ts, the client call sites, the two job registrations, the Flipt enum entries and the telemetry counters. claudedocs/bitdex-app-removal-handoff-2026-09-01.md records what the removed machinery did — the bdx: cursor codec, primary/shadow dispatch, the superset post-filter and its two predicates, and bitdexCallsObserved — because phase 2 depends on that understanding and it existed nowhere else. Baseline for whoever continues: typecheck was clean (0 errors, 410s) on this worktree at origin/main before any of these edits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KYrq2ighNp2rABTAvpzhj9 * chore(bitdex): finish the app-side removal Completes the WIP commit. BitDex is decommissioned — the engine is gone from the cluster, both cron jobs are off, and the 8 ops write triggers are dropped from prod — so the app code that routed to it is dead and comes out. image.service.ts: the primary/shadow dispatch in getImagesFromSearch, the bdx: cursor codec, fetchBitdexPrimary and its pagination loop, postFilterBitdexDocs and its two predicates, mapBitdexDoc, getImagesFromBitdexPreFilter, the native filter helpers and the BitdexCursoredPageUnavailable class. image.controller.ts / image-search.service.ts: both getFliptVariant evaluations, `useBitdex`, the `skipBitdex` carve-out, the bitdexMode pass-throughs and the now-unreachable getAllImagesIndex arm in the REST path. Routing is unchanged for every live request: the flag resolved to `off` for every segment, so `useBitdex` was already false everywhere. One semantic change, deliberate: `model3dId` resolved to `undefined` on a BitDex doc (fall back to getByPostId) and `null` on a Meili doc (confirmed-absent). With only Meili serving, the "not indexed" branch cannot occur and it collapses to null. Also: both job registrations, the three FLIPT_FEATURE_FLAGS entries, BITDEX_URL, the reemit_*/bitdex_audit_* counters, the feed-serve metrics registration, the bitdex-test skill, the dev-server console filter, and the admin sortAt reconcile endpoint. KEPT ON PURPOSE: `'bitdex-image-feed'` in FEEDBACK_AREAS. It is a stored data label on existing feedback rows, and the zod enum is what a moderator queries them through — removing it makes historical feedback unqueryable. The prompt that wrote it is gone, so no new rows carry it. Verified: typecheck 0 errors in 314s against a 0-error baseline on origin/main in the same worktree. Lint introduces nothing — the 4 errors in touched files reproduce identically at origin/main with only line numbers shifted. Covering suites 81 passed (5 files), exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KYrq2ighNp2rABTAvpzhj9 * fix(bitdex): review round 1 — remove the doc from the public repo, delete dead plumbing Findings from the five-lane review. Ranked as the lanes ranked them. 🔴 SECURITY. Removes claudedocs/bitdex-app-removal-handoff-2026-09-01.md, which I added in the previous commit. This repo is public and permanent, and that file named an unrotated production credential and where to find it, an internal secret's auth posture and namespace, host filesystem paths, node identifiers, a public DNS record, and two private repository names. Four categories from CLAUDE.md's "do not commit these" list. Flagged independently by the intent and safety lanes. Moved to _local/docs/plans/. Removal is not remediation — the branch was pushed, so the credential it named must be treated as disclosed and rotated on that basis; raised separately. Dead plumbing the removal left behind, found by intent, perf and safety independently: - ImagesInfinite kept the FeedbackPrompt import, the showFeedbackPrompt prop and the feedSnapshot destructure after its only mount was deleted, and five call sites still passed a prop that did nothing. - getFeedSources / resolveFeedSource / buildFeedSnapshot and FEED_SOURCE_NONE had no consumer left once that prompt went; buildFeedSnapshot still ran on every new data identity inside useQueryImages, the hook behind every image feed. Removed with their test rather than left passing against renamed labels. - Dead imports in image.service.ts (getFliptVariant, buildFliptContext, withDetachedSpan), image.controller.ts and image-search.service.ts (FLIPT_FEATURE_FLAGS, getFliptVariant, buildFliptContext, getAllImagesIndex), plus the unused PostFilterStats type and useRef in image.utils.ts. - The resolvedHubSources memo was inert: the two builders are mutually exclusive per request and each calls it once, so input.resolvedHub was written and never read. Memo and its ImageSearchInput field removed, helper kept. Comments that had become false rather than merely stale: - image.service.ts said Meili docs carry model3dId but "raw-SQL rows do NOT (the field is left undefined there -> the chip falls back to the postId lookup)". The raw-SQL path does carry it, selected and visibility-resolved to number | null. My earlier edit substituted "raw-SQL rows" into a sentence that was only ever true of BitDex docs. - Added the note the safety lane asked for: the model3dId null arm is correct only while one backend serves that path. - user-hub.service.ts claimed three filter builders apply the hub cap; there are two, and my earlier edit had left the sentence garbled. - metric-helpers.ts pointed at an internal stats endpoint deleted in this PR. - packages/civitai-flipt/README.md still used bitdex-image-search as its worked example; the identical example in its own env.ts was already fixed. Verified: typecheck 0 errors in 276s. Covering suites 75 passed (5 files), exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KYrq2ighNp2rABTAvpzhj9 * fix(bitdex): review round 2 — restore the routing-flag test, drop orphaned helper and stale comments Second round of five-lane findings. 🔴 The one that mattered: I removed coverage of `features.imageIndexFeed`. Flipping the fixture to `true` and deleting the flag-off routing test left `imageIndexFeed` appearing in exactly one test file as a single hardcoded literal, shared by every test — so `useIndex = !!hubId || (features.imageIndexFeed && !requiresDbPath)` could have its flag conjunct deleted outright with nothing in the suite going red. Found by the tests lane, which proved it structurally rather than by running. Added the replacement test with its own ctx. Verified it can fail rather than asserting it: mutating the controller to `!!input.hubId || !requiresDbPath` gives `Tests 1 failed | 6 passed (7)` and AssertionError: expected "vi.fn()" to be called 1 times, but got 0 times on the named test. Mutant reverted, 27 passed (2 files), exit 0. Also from the lanes: - `hubCreatorScope` had zero callers — its only one was inside the deleted region. Removed rather than left as a documented helper nothing constructs. - `getImagesFromFeedSearch`'s hideChallenges security note claimed image-search.service.ts spreads `data` into "all three branches" and that "two branches would filter and this one wouldn't". Two branches now, one filters. The arithmetic in a security argument is worth keeping correct. - `hub-feed-filter.test.ts` said the empty-intersection branch is "three separate `if (!capped)` lines"; two. Second copy of the sentence I fixed last round. - Dropped the flipt mock from image.controller.feed-source.test.ts — the assertion that gave it meaning went with the BitDex routing, leaving a mock nothing observes. Not changed, with reasons: the tests lane ruled my declared coverage gap a false alarm — `withMeta:false` and both tag assertions already exist against getImagesFromFeedSearch at index.test.ts:197-246, so replacements would have been duplicates. `hubFilterArms` kept as a seam though it now has one consumer; its parity comment was corrected last round. Verified: typecheck 0 errors in 163s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KYrq2ighNp2rABTAvpzhj9 * docs(feedback): say in the code why bitdex-image-feed survives its producer The slug has no writer since the BitDex decommission, so it reads as leftover and the obvious cleanup is to delete it. Deleting it breaks a read path: this enum is what getFeedbackAreaSchema validates a moderator's area query against, so removing the slug makes every historical row filed under it unqueryable through that route. The reason existed only in a PR body and a mail thread, which is where intent goes to be refactored away. Putting it at the constant so the next person meets it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KYrq2ighNp2rABTAvpzhj9 * fix(bitdex): review round 3 — a comment my own fix broke, and a surviving mutant Round-2 re-review of all five lanes at47d883171b. 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 * test(bitdex): drop the inert flipt and index mocks from the v1 images suite Last of the round-2 findings. This file still mocked `~/server/flipt/client` and seeded `getFliptVariant` twice with comments claiming the value selected a backend — it does not: routing there is `useLegacyMethod` alone. It also mocked `getAllImagesIndex`, which the handler graph can no longer reach, and nothing asserted on. The flipt mock was mine: I narrowed it to `FLIPT_FEATURE_FLAGS: {}` when removing the BitDex enum member. A hand-listed mock standing in an empty object for a real enum is the shape CLAUDE.md warns about — anything the graph later reads off it yields undefined, and `getFliptBoolean`/`isFlipt` were not on the mock at all. It passed only because `getAllImages` is mocked, one import edge from a confusing failure. Removed rather than widened: the module still loads transitively through `importOriginal` of image.service.ts, so no new env edge. Test count unchanged at 33 passed, which is the point — removing a mock nothing observed should move nothing. Verified: typecheck 0 errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KYrq2ighNp2rABTAvpzhj9 * fix(bitdex): re-point the flipt eval-context ledger at the moved lines CI `Unit tests (4)` was red on the head. The eval-context guard pins sites by file:line, and this PR deletes 1,346 lines from image.service.ts, so every ledgered line below the cut drifted. Both directions of the guard fired at once — four sites unledgered, four ledger rows stale — which is the guard working. Re-pointed five references, four ledger rows and one assertion: 4136 -> 3101 feed-fetch-filter-in-post 4277 -> 3142 feed-image-existence (also the argc assertion at :222) 5015 -> 3865 feed-image-existence 6149 -> 4670 feed-image-existence Each new line verified to be the same getFliptBoolean call as the old one on origin/main. No ledger row added, dropped, or re-reasoned. 🔴 I first reported this failure as pre-existing, on a control that did not run: `git stash` on an already-clean tree stashed nothing, so the "origin/main" run was my own branch again and returned the same 3 failures. The real control — detaching the worktree to origin/main — gives `Tests 12 passed (12)` there against `3 failed | 9 passed (12)` here. The failure is mine. A stash-based control is worthless unless something was actually stashed. Verified: 12 passed (12), exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KYrq2ighNp2rABTAvpzhj9 * test(image): witness the userHubs refusal on the feed handler Nothing tested it. Deleting both lines of the guard at image.controller.ts:283-284 left 115 tests green across all six hub-adjacent suites — `user-hub.router.gate` covers the hub procedures, and `image.getInfinite` is not one of them. `hubId` is a plain URL parameter, so without the refusal `/images?hubId=N` keeps serving a hub feed after `userHubs` is turned back off. That gate had no witness. Pre-existing, but this PR made it worse in a way worth naming: the hub-pinning test added in9d38e02e39sets `userHubs: true`, so it exercises the guard's pass-through and leaves the refusal unwitnessed — better-pinned neighbourhood, same exposed gate. Asserts the throw AND that neither fetch mock was called. The second half is load-bearing: a change that refused after dispatching would satisfy a throw-only assertion. Verified capable of failing, naming the test rather than merely going red: deleting both guard lines gives `Tests 1 failed | 8 passed (9)` with the failure on `refuses a hub outright when userHubs is off, without dispatching` and nothing else. Mutant reverted; typecheck 0 errors. Test-only — no runtime line changed. Found by the test lane in review round 3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KYrq2ighNp2rABTAvpzhj9 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
101 lines
5.8 KiB
TypeScript
101 lines
5.8 KiB
TypeScript
import { expect, test } from '@playwright/test';
|
|
import { storageStatePath } from './preview-fixtures';
|
|
import { trpcQuery } from './preview-trpc';
|
|
|
|
/**
|
|
* Feed-content smoke: the core BROWSE path actually renders real content.
|
|
*
|
|
* preview-smoke.spec.ts only proves /models and /images LOAD (HTTP <400, not
|
|
* bounced to /login or /preview-restricted) — it never asserts the feed returns
|
|
* any items. An empty-feed or broken-list regression (a busted model.getAll
|
|
* handler, a query that 500s, a filter that nukes every row) passes every preview
|
|
* spec today while shipping a blank site. This closes that gap for the DB-backed
|
|
* models feed by asserting its backing query returns a non-empty, structurally
|
|
* valid page of content.
|
|
*
|
|
* Why tRPC instead of scraping cards out of the DOM:
|
|
* - The /models feed IS this query — model.getAll backs it
|
|
* (src/components/Model/model.utils.ts:145 `trpc.model.getAll.useInfiniteQuery`).
|
|
* Asserting the query result is asserting the same data the card grid renders,
|
|
* minus the flaky virtualized-list / lazy-image DOM surface (cards mount on
|
|
* scroll/intersection, have no stable testid) — a DOM-card count would be the
|
|
* brittle proxy here, not the source of truth.
|
|
* - Self-contained against the preview's OWN dev-clone DB: no external service,
|
|
* no scroll timing, no networkidle (which never settles — see gotchas below).
|
|
*
|
|
* Runs as `tester` (a free member that PASSES the preview gate). model.getAll is a
|
|
* publicProcedure, reachable for a gate-passing user. page.request carries the
|
|
* test's storageState auth cookie, and preview-auth.middleware gates /api/trpc/*
|
|
* (only /api/auth, /login, /preview-restricted, /_next, /favicon are exempt —
|
|
* matcher '/:path*'), so the authed cookie is what lets the call through the gate —
|
|
* same authed-tRPC pattern as preview-engagement.spec.ts.
|
|
*
|
|
* Verified tRPC shape (civitai repo, paths relative to civitai/src):
|
|
* - model.getAll publicProcedure, input getAllModelsSchema (minus `page`)
|
|
* (server/routers/model.router.ts:141). All fields optional with
|
|
* server defaults, so `{ limit }` is a valid input.
|
|
* getModelsInfiniteHandler returns { items, nextCursor }
|
|
* (server/controllers/model.controller.ts:27 `return { items,
|
|
* nextCursor }`); each item carries a numeric `id`.
|
|
*
|
|
* Tolerant on COUNT, strict on STRUCTURE: the prod-clone dev DB has real content
|
|
* but we never assume specific models/titles. We assert ">= 1 item with a numeric
|
|
* id" — the structural intent is "the feed produced a real, non-empty page of
|
|
* identifiable content", not any particular row. An empty browse feed is itself
|
|
* the regression this spec exists to catch, so keep the non-empty assertion.
|
|
*
|
|
* ── Why the IMAGES feed is NOT asserted here (deliberately scoped out) ──────────
|
|
* The /images feed (image.getInfinite, broad/unfiltered) does NOT reliably read
|
|
* the DB in a preview: getInfiniteImagesHandler routes a broad query (no
|
|
* postId/modelId/collection/reaction filter → requiresDbPath=false) through
|
|
* getAllImagesIndex — the MEILISEARCH-backed index path — whenever
|
|
* `features.imageIndexFeed` is on,
|
|
* which is the production-like default (src/server/controllers/image.controller.ts
|
|
* :300-320). That path hits an image search index the preview doesn't populate and
|
|
* then hydrates from the dev clone, so a broad image.getInfinite returns flaky /
|
|
* empty results in preview (observed: 3 items once, 0 items on two consecutive
|
|
* smoke runs). This is the SAME external-index limitation site search has — see the
|
|
* search note pattern — so we don't ship an assertion that flakes on infra the
|
|
* preview doesn't own. The DB-backed models feed above is the solid, self-contained
|
|
* signal.
|
|
*
|
|
* FOLLOW-UP for reliable image-feed coverage (not done here): scope the query so
|
|
* `requiresDbPath` is true (e.g. `image.getInfinite { modelId }` — bare modelId
|
|
* forces getAllImages/DB per image.controller.ts:303), seeding the modelId at
|
|
* runtime from this models feed; or stand up a preview-local image index.
|
|
*
|
|
* Only runs under playwright.preview.config.ts (needs PREVIEW_URL + minted states).
|
|
*/
|
|
|
|
const ROLE = 'tester' as const;
|
|
|
|
// A small page is all we need to prove "the feed renders content"; keep it light
|
|
// on the single-replica preview pod (the suite runs serially, workers:1).
|
|
const FEED_LIMIT = 5;
|
|
|
|
test.describe('browse feed renders real content (tester)', () => {
|
|
test.use({ storageState: storageStatePath(ROLE) });
|
|
|
|
test('model.getAll (the /models feed) returns a non-empty page of models', async ({ page }) => {
|
|
// Warm the request context against the preview origin so page.request shares the
|
|
// auth cookie + a real navigated origin (preview-trpc stamps Origin/Referer for
|
|
// the CSRF gate, but navigating once is the safe baseline — mirrors the other
|
|
// tRPC-driven preview specs). domcontentloaded only: NEVER networkidle — the
|
|
// app's background traffic never idles, so a networkidle nav hangs to timeout.
|
|
await page.goto('/', { waitUntil: 'domcontentloaded' });
|
|
|
|
const data = await trpcQuery<{ items: Array<{ id: number }> }>(page.request, 'model.getAll', {
|
|
limit: FEED_LIMIT,
|
|
});
|
|
|
|
const items = data?.items ?? [];
|
|
// STRUCTURE: a real, non-empty page came back. An empty models feed is the
|
|
// exact regression this catches (broken handler / over-aggressive filter / 500).
|
|
expect(Array.isArray(items), 'model.getAll should return an items array').toBe(true);
|
|
expect(items.length, 'the models feed should render >= 1 model card').toBeGreaterThan(0);
|
|
// Each rendered card needs an identifiable model — assert the first item is a
|
|
// real row (numeric id), not a malformed/placeholder entry.
|
|
expect(typeof items[0]?.id, 'a feed model should carry a numeric id').toBe('number');
|
|
});
|
|
});
|