26290 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
briant	8afaa0cfef	fix(ingestion): log the real error and elapsed time when a scan submit gets no response ... A submit that never gets a response is the one case where the error's identity is the whole diagnosis, and it was the one thing the log did not carry: JSON.stringify(new Error()) is `{}` because Error has no enumerable own properties, so every no-response failure recorded `error: {}`. Pass it through safeError, which the repo already uses for exactly this, and add the wall time across all attempts — the attempt count alone cannot tell three 15s aborts from an instant rejection. The test pins the serialization rather than the call: reverting to the raw error fails with `expected '{}' not to be '{}'`. The logging mock in the covering suite was hand-listed and silently dropped safeError; spread the original instead, since that module is pure apart from logToAxiom. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-18 15:37:34 -06:00
Zachary Lowden	dce428a492	feat(app-blocks): author-fee accrual ledger (slice 2a, dark) (#4944) ... * feat(app-blocks): author-fee accrual ledger + daily settlement rail (slice 2a, dark) The rail that pays an app author the per-generation fee slice 1 (#4922) learned to compute. Adds the ledger, the daily settlement job and the clawback path. NOTHING WRITES TO THE LEDGER YET. The viewer-charge path is deliberately not in this PR — see "What is NOT here" below. So this moves no money: the table stays empty, the settlement job no-ops on an empty scan, and the whole path is behind app-blocks-author-fee-enabled, which is false. TWO HOPS, MIRRORING THE MODEL LICENSING FEE. The licensing rail charges the viewer at generation time and writes a per-resource fee row, then deliver-creator-compensation mints to the creator daily. Same two hops and the same externalTransactionId dedup discipline here, in a civitai-owned table — because orchestration.resourceCompensations is keyed on a modelVersionId and written by the orchestrator, and an app fee has no model version. That fork is recorded rather than left for a reader to rediscover. NO FRACTIONAL ACCRUAL, AND THAT REVERSES THE DESIGN IT INHERITED. The licensing fee is fractional because it is priced per-image at 0.01 buzz and the viewer pays the ceiling of the sum, so the creator's share genuinely has sub-buzz resolution. This fee does not: max(flat, pct x base) is floored to whole Buzz before the viewer is shown or charged it (D7 requires the viewer see the exact number before the run, and Buzz cannot express a fraction). The author is credited exactly what the viewer was debited — the platform is a conduit and takes no cut. The daily batch therefore exists for ledger volume, not rounding. Consequence, stated in three places because slice 3 must surface it: an author who sets a 0 flat leg and a low percentage earns nothing on cheap generations, forever. Same shape as the $0.00 spend bounty this arc replaced; the difference is it is now the author's explicit choice and the platform default avoids it. DECISIONS IMPLEMENTED D6 blue Buzz in, blue Buzz out — settlement groups by buzz type and never coerces. A collapsed bucket would convert non-withdrawable Buzz into withdrawable earnings and no total would change. D10 the percent leg prices off base only (no code change; recorded). Self-dealing is excluded at accrual, and counted rather than dropped. The app owner is snapshotted at WRITE time so an ownership transfer cannot retroactively move earnings already accrued. CLAWBACK. The orchestrator refunds undelivered work after submit, so the fee has to follow or an author earns on a generation the viewer got refunded. Not a new policy — CalculateLicenseFees already weights every fee by delivered fraction. Before settlement the accrual is voided in place; after it, a negative carry-forward row nets against the next run. A bucket whose net goes non-positive is HELD, not forgiven at zero, so the debt stays visible. Mint happens BEFORE the status flip, deliberately: a crash between them settles late rather than paying twice, and the deterministic dedup key is what makes the retry safe. MIGRATION IS MANUAL-APPLY, AND SHIPS BEFORE THE CODE (rule 8, the #4903 precedent). Committed for history only. VERIFICATION pnpm typecheck OK - 0 type errors in 165s settlement suite 17 passed mutation sweep 6/6 killed, EACH by its own named test: self-dealing guard, D6 bucket key, dedup-key composition, non-positive hold, clawback sign, zero-fee guard positive control applied +1000 to the minted amount -> 2 tests red, proving the harness executes the code under test What is NOT here, and why: the viewer-charge path. The fee must be priced from the whatIf base and added to BOTH reservations before submit — otherwise it escapes the viewer's per-app consent budget entirely, which is the one real safety hole this design found. That touches four submit paths on the hot billing path and deserves its own focused audit rather than being folded in behind a new table. It is the next PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(app-blocks): FK targets app_blocks, not AppBlock — and the entry_type CHECK is unreachable Two corrections the dev-first apply caught, both in the migration only. 1. The app_block_id foreign key referenced "AppBlock", which does not exist. The Prisma model is AppBlock but it carries @@map("app_blocks"), so the physical table is app_blocks — confirmed against the existing block_spend_attribution_app_block_id_fkey, which points at app_blocks. Applied as written this would have failed outright on every database. Hand-writing the migration rather than generating it is what lost the mapped name. 2. The entry_type CHECK is SUBSUMED by the amount-sign CHECK and cannot fire. The sign check reads (entry_type='accrual' AND fee>=0) OR (entry_type='clawback' AND fee<=0), so any third value makes both disjuncts false and is rejected there first. Measured, not reasoned: an insert with entry_type='bogus' on dev came back rejected by ..._amount_sign_check, never by ..._entry_type_check. The constraint stays as an explicit statement of the allowed set, but it is now labelled as not-a-reachable-guard so nobody reads it as coverage. If the sign check is ever loosened this becomes live and needs its own negative control. APPLIED AND VERIFIED — dev first, then prod, per rule 8. dev cnpg-cluster-dev-1 / cnpg-database-dev prod cnpg-cluster-nvme0-5 / cnpg-database (primary re-derived live), with SET lock_timeout='5s' — the four FKs take a lock on User, a hot table, so failing fast beats queueing behind it Both databases: 18 columns, 4 foreign keys, 5 checks, 4 indexes, 0 rows, with a positive control (a bogus column name returns 0, so the query discriminates). block_spend_attribution unchanged at 602 rows. DDL replayed to both prod standbys (-4 and -7 report 18 columns). Constraint behaviour was exercised on dev, not merely confirmed present: a valid row inserts (positive control), and 7 negative controls are all rejected — 6 of them by their own named constraint. The seventh is finding 2 above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(app-blocks): round 0 — two comments asserted seams that do not exist, and the new-table rationale was false Round 0 (requirements & deletion) found five things wrong with what this PR ASSERTS rather than with what it does. Four are fixed here; the fifth is a deletion decision for the operator and is left open. 1. THE NEW-TABLE RATIONALE WAS FALSE, in both the migration and the Prisma docblock. It claimed block_spend_attribution is "IMMUTABLE by design". It is not — that model carries status, voidedReason, confirmedAt, voidedAt, paidOutAt and payoutId, the same accrued/settled/voided lifecycle plus a payout key. Those columns are merely dead today because the rail that wrote them was removed, which is a different claim. A reader who checked the schema would have found the lifecycle columns and concluded the reasoning was wrong. The requirement survives on a better reason, verified rather than substituted: recordSpendAttribution runs inside `void (async () => { … })()` so that a failed attribution write can never break a generation — droppable telemetry. An accrual is a money obligation and must be awaited. The seam, not the row shape, is what separates them. 2. A comment named `chargeBlockAuthorFee` "in the router" as the caller that performs the debit. That function has never existed anywhere in this repository; the name appeared only in that sentence. 3. A comment claimed "the charge path reads this same predicate before taking the money" about the self-dealing exclusion. There is no charge path and no shared predicate — slice 1 has no self-dealing check at all. Corrected, and turned into an explicit obligation on slice 2b: call this before the debit or extract it, because the exclusion is NOT enforced upstream today. 4. BlockAuthorFeeAccrualStatus and BlockAuthorFeeEntryType were exported and then never referenced, including inside their own file — every status was written as a bare literal, so a typo'd 'setled' would have compiled and matched nothing. Now bound to named constants used at all ten write and compare sites. 5. The settlement job's getJobDate/setLastRun cursor gated nothing. settleBlockAuthorFees scans `status: 'accrued'` with no date filter, so the cursor was read only to interpolate into a log line and then written back — two DB round-trips and a persisted KeyValue row no branch consulted. The real idempotency mechanism is the deterministic externalTransactionId. Removed; a cursor that reads like a run-once guard while guarding nothing is worse than none. Also: the D<n> labels had no referent inside this repository — they index an internal decision memo that is not here, so a bare "D6" was authority a reader could not resolve. D1, D6 and D7 are now stated in full at the top of the service, and the note records that D8 and D10 implement nothing in this file. And a caveat that was missing entirely: a newly added cron is NOT picked up by a deploy. Jobs are discovered through /api/internal/get-jobs and the external scheduler needs an explicit refresh, so this job will not be dispatched until someone performs that out-of-band step. Merged does not mean running. VERIFICATION after the fixes pnpm typecheck OK - 0 type errors settlement suite 17 passed mutation sweep 6/6 still killed, each by its own named test — re-run after the literal-to-constant refactor, since that touched all ten status/entry comparison sites LEFT OPEN, deliberately — operator decisions, not mine: * clawbackBlockAuthorFee has ZERO production callers, and its negative carry-forward arm is unreachable until something has settled (two PRs away). * The base rate: bulk-payout-block-attributions.ts is the same machine — registered daily job, idempotent mint, clawback carry-forward, net<=0 hold — built 2026-05-31 and still unwired. mintPayoutForOwner has no production caller to this day. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(app-blocks): retire the clawback from slice 2a — it had zero callers and an unreachable arm Round 0's deletion candidate, taken. The clawback reverses a charge this PR deliberately does not make, so it cannot be needed yet: * clawbackBlockAuthorFee had ZERO production callers — only its own tests. * Its negative carry-forward arm is reachable only once a row has status 'settled'. Nothing has settled and nothing can until the charge path lands and the job runs for a day — at least two PRs away. The base rate is what made this decisive rather than tidy. bulk-payout-block-attributions.ts is the same machine — registered daily job, idempotent mint, clawback carry-forward, net<=0 hold — built 2026-05-31 and still unwired; mintPayoutForOwner has no production caller to this day. The way that rail rotted is that it shipped ahead of its consumer. Slice 2b brings the clawback back together with the refund path that drives it. REMOVED clawbackBlockAuthorFee, ClawbackReason, ClawbackBlockAuthorFeeResult the entry_type column, its composite unique, and BlockAuthorFeeEntryType the amount-SIGN check (there are no negative rows now) the entry_type check — already measured unreachable, shadowed by the sign check the 'clawed_back' status the non-positive bucket hold, bucketsSkippedNonPositive, and its test 5 clawback tests REPLACED, so the invariant lives where it can actually be violated CHECK (fee_buzz > 0). The settlement job's old "a bucket can never sum to <= 0" branch was unreachable the moment negative rows went away, and an unreachable guard reads as coverage while providing none. The rule is now enforced at the write (accrueBlockAuthorFee refuses fee <= 0) and in the schema, not in a dead branch. Unique key is now workflow_id alone. VERIFICATION pnpm typecheck OK - 0 type errors (one real error caught first: the job still logged the removed bucketsSkippedNonPositive field) settlement suite 11 passed (was 17; 6 removed with the clawback) mutation sweep 4/4 killed, each by its own named test — self-dealing, D6 bucket key, dedup-key composition, zero-fee guard positive control +1000 on the minted amount -> 2 tests red, so the harness genuinely executes the code SCHEMA RE-APPLIED — dev then prod, table dropped and recreated (it was empty and nothing referenced it). The drop was GUARDED: a DO block re-counts rows and inbound constraints at the moment of the drop and raises rather than destroying anything, so a row that landed in between aborts the transaction. Guard printed "0 rows, 0 inbound references" on both. dev cnpg-cluster-dev-1 17 cols, 4 checks, 0 rows prod cnpg-cluster-nvme0-5 17 cols, 4 checks, 0 rows (lock_timeout 5s) standbys -4 and -7 converged to the same after WAL replay; -7 lagged ~30s and was polled to convergence rather than assumed block_spend_attribution unchanged Constraints exercised on dev, not merely counted: a valid row inserts, and 6 negative controls are each rejected BY THEIR OWN named constraint — no shadowing now that the redundant entry_type check is gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(app-blocks): round 1 — the settlement rail could lose money one way and pay twice the other Round 1 returned "needs rework" with four blockers. All four fixed. CI was RED at the previous head and I had not checked it after pushing; that is what surfaced three of these. 1. 🔴 THE MINT RESULT WAS DISCARDED, AND ROWS FLIPPED REGARDLESS. createBuzzTransactionMany does NOT throw on a per-transaction failure — its own comment says an insufficientFunds or otherwise-rejected result "is dropped from BOTH arrays: the money did NOT move and it is otherwise invisible". It also filters out transactions failing `fromAccountId !== toAccountId && amount > 0` before calling the service. The old code awaited it, ignored the return, and flipped every row to settled: the author was never paid while the ledger asserted they were, and status='accrued' was the only handle that could have found it afterwards. Now reconciled by COUNT (successes come back as opaque ids, so the count is the only thing that reconciles); a conflict counts as settled because the money already moved under that key. If the count does not account for every bucket, NOTHING is flipped — we cannot tell which dropped, so the rows stay accrued and retry. The precedent is challenge-funding.ts, which carries the same reconciliation and the same comment. 2. 🔴 THE DEDUP KEY NAMED A DAY BUT THE SCAN WAS UNBOUNDED, so it was wrong in BOTH directions, and the module comment confidently asserted the opposite. (a) SILENT LOSS — a second run on the same day swept up rows accrued since the first, minted them under the SAME key, got a benign-looking conflict, and flipped them settled. Money gone. Compounded by (1), which made it invisible. (b) DOUBLE PAY — if the flip failed after a good mint, rows stayed accrued until the next run 24h later, under a DIFFERENT date key, and were minted again. The comment claimed "the next run re-derives the same key"; on a daily cron it never does. Fixed by bounding the scan at the day boundary: a run for day D settles only rows accrued strictly before the start of D. Every row now belongs to exactly one settlement day, derivable from the row, so (a) cannot sweep fresh rows and (b) re-derives the same key and conflicts instead of paying twice. 🔴 This is also why deleting getJobDate/setLastRun last round was not the whole story. It gated nothing AS WRITTEN — that part was right — but it was standing in for a run-once guard, and removing it without replacing the mechanism left (a) exposed. The boundary is the replacement, and it is stronger: it bounds the ROWS rather than the invocations, so it holds under the concurrent runs createJob's lock expiry can produce. 3. 🔴 CI RED — three checks, all mine: * block-spend-attribution-status-default — and this one was emitting a FALSE claim about a table this PR does not touch. The guard filtered migration files with a file-level `sql.includes(TABLE)`, which reads PROSE: this migration's comment header explains why it is a SEPARATE table from block_spend_attribution, which passed the filter; the `"status" TEXT … DEFAULT` regex then matched THIS table's own column and won on last-wins sort order. It reported `provisions DEFAULT 'accrued' … while the schema says @default("tracked")` about a column nobody had changed. That is worse than a false red: the guard exists because a default the payout read does not select makes rows invisible to payout with no error (#4036), so anyone "fixing" it by following its message ships that defect. Fixed in the GUARD — comments stripped, file split into statements, only statements naming the table scanned — not by rewording this migration, which would leave the next table to trip it. * app-access.call-site-ledger — the guard fails on GROWTH because it forces a collaborator decision. Registered: the author fee accrues to the app OWNER only, deliberately not widened to ACCEPTED collaborators. That is the ledger's existing D4 applied to a new earnings surface, not a new decision — earnings are appOwnerUserId-keyed precisely so an ex-owner keeps what they accrued before transferring an app away, and widening the WRITE would make that inexpressible. Resolving the owner at settlement instead would retroactively re-route earnings on every transfer. * ESLint + Prettier — both new files were unformatted. Formatted. Plus no-direct-shared-module-mock: the test now uses the canonical db/logging mocks via the repo's own codemod. 4. 🟡 buzz_type had no CHECK — the one money-critical column without one, while status and governing_leg both had theirs. BuzzAccountType also contains BANK types (creatorProgramBank, cashPending, cashSettled, club), and settlement cast the column to it unchecked, so a junk or bank value would surface only as a silently-dropped transaction. Now CHECK IN ('blue','green','yellow','red'), verified on dev: 'creatorProgramBank' rejected by that constraint by name, 'blue' accepted as a positive control. Also fixed: the settlement scan read the REPLICA while the flip wrote the primary (replica lag would re-bucket an already-settled row); buzzMinted was incremented even when updateMany matched 0 rows, so the job logged an affirmative "minted" for money it had not moved; and the settlement key was built from two independent spellings, only one of which was pinned. TESTS — the absences round 1 named were structural, not oversight: the fake resolved createBuzzTransactionMany to `undefined`, which can express neither a conflict nor a drop. It now returns `{ transactions, conflicts }` like the real one, and five tests were added: mint-did-not-reconcile flips nothing, a conflict counts as settled, the day boundary is midnight of the settled day, a concurrent flip claims no buzz, and one key serves both the mint and the row stamp. VERIFICATION pnpm typecheck OK - 0 type errors the four red suites 50 passed (4 files) prettier --check clean (positive control: the earlier --write listed all five paths by name, so they do resolve) mutation sweep 6 real mutants, 6 KILLED, each by its own named test: discarded mint result, missing day boundary, replica read, unconditional buzz count, duplicated key spelling positive control +1000 on the minted amount -> 2 tests red 1 SURVIVOR, and it is an EQUIVALENT MUTANT, not a gap: dateStr off `date` vs off `boundary` is the same string for every input (toISOString is always UTC and boundary is midnight of that same UTC day; checked across day edges and a year boundary). Recorded in-code so nobody writes a test asserting a difference that cannot exist. SCHEMA re-applied dev then prod for the buzz_type CHECK, same guarded drop (re-counts rows and inbound references at the moment of the drop, raises rather than destroying). All three prod instances: 17 cols, 5 checks, 0 rows. Standby -4 lagged ~30s and was polled to convergence rather than assumed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(app-blocks): round 2 — the settlement key named the RUN day, so a deferred retry paid twice Round 2 found that round 1's flagship fix did not do what it said, and that its fail-closed branch made things worse. Both confirmed against the code before acting; both were mine. 🔴 F1 — THE KEY WAS DERIVED FROM THE INVOCATION, NOT FROM THE ROW. `dateStr` came from `args.date` — the day the job ran — while the scan admitted every accrued row below the boundary, i.e. all of history, bucketed by (owner, buzzType) with no accrual day. So a row whose flip failed on 09-18 was re-scanned on 09-19 and minted under `…-2026-09-19-…`: a NEW key, no conflict, owner paid twice. That is exactly the failure mode the module comment claimed the day boundary had fixed — asserted three times, false every time. The sentence "an unflipped row still belongs to its original day, so its key is unchanged" was the worst of them: the day never came from a row. 🔴 F2 — THE FAIL-CLOSED BRANCH FED F1. One dropped bucket meant NOTHING in the batch was flipped, including buckets whose money had already moved — and those then sat `accrued` until the next day, where F1 re-minted them. Round 1 introduced this while fixing an under-payment, and its comment claimed the trade was "money owed, which is recoverable". It was money PAID TWICE, which is not. 🟡 F3 — a same-day second run still lost money via `take` truncation: run 1 mints a partial bucket and flips, run 2 picks up the remainder, builds the same key, gets a conflict, and flips those rows without ever paying for them. A conflict says that KEY minted; it does not say those ROWS were in it. THE REDESIGN — one complete accrual day at a time, keyed on the row. * The loop settles the OLDEST unsettled accrual day, scanning exactly [dayStart, dayEnd) rather than everything below a boundary. * The bucket carries `accrualDay`, and `keyForBucket` builds the key from it. Every component now comes from the rows; nothing comes from the clock. A retry tomorrow, next week, or after a month with the flag off re-derives the SAME key and conflicts. F1 closed. * A day that exceeds `limit` is SKIPPED WHOLE and reported (`daysTruncated`), never cut. `take: limit + 1` makes the overflow detectable instead of silent. A partial bucket is what F3 lost money to, so the rule is that a bucket is always settled whole. F3 closed. * ONE BUCKET PER MINT CALL, so a drop is attributable. createBuzzTransactionMany reports only counts and opaque ids, which is why the batched version could not say which bucket failed and answered by flipping nothing. Per-bucket makes it answerable: this bucket moved, or it did not — and its peers are unaffected. F2 closed. 🟢 F6 — the "EQUIVALENT MUTANT" comment was FALSE as written, and I had checked it with fixtures that could not see the counter-example. Date.UTC maps years 0-99 to 1900+y, so year 0026 gives `0026-…` from one spelling and `1926-…` from the other. Unreachable in production, but the comment forbade writing the test that would have found it. Gone with the rewrite. 🟡 F4 — the job's prose contradicted the service: it still said the scan had "no date filter" (the recorded rationale for deleting the cursor) and "since the last run". Both corrected, and the cursor's real justification stated: idempotency lives in the ROWS, not the invocation, which is what holds when createJob's lock expires and two runs overlap. 🟢 F5 — the status-default guard stripped only `--` comments while Prisma-generated migrations in this repo open with `/* Warnings */` headers, so the prose was wider than the code. Both comment forms are stripped now. VERIFICATION pnpm typecheck OK - 0 type errors affected suites 52 passed (4 files) prettier clean mutation sweep 7 mutants, 7 KILLED, each by its own named test: run-day key, truncated oversized day, flip-on-drop, un-day-scoped scan, D6 bucket collapse, unconditional buzz count positive control +1000 on the minted amount -> 2 tests red No SURVIVORS this round. The previous round reported one and explained it away as equivalent; that explanation was wrong (F6), which is a reason to distrust a survivor-with-a-story rather than to be reassured by one. Schema unchanged — no migration in this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(app-blocks): round 3 — the day loop could never advance past a day it could not finish Round 3's verdict was "merge after fixing 🔴-1". The money path held this time — eleven of thirteen claims landed clean and no double-payment or silent-loss path survived — but the rewrite introduced a LIVENESS defect, and my own mutation sweep could not have caught it. 🔴 A DAY THE LOOP COULD NOT FINISH BLOCKED EVERY LATER DAY. The loop re-derived "the oldest unsettled day" on every iteration with nothing to advance past one that did not complete. Two arms: * an OVERSIZED day hit `break` and stayed the oldest forever, so every later day was blocked permanently — and since the job passes no `limit`, the only recovery was a code change and a deploy; * a persistently REJECTED bucket left its rows `accrued`, so the next iteration re-selected the same day and re-minted it, burning all 30 iterations on one stuck owner while every other author stopped being paid. Neither risks money — the row-derived key still makes a retry conflict — but both halt settlement for everyone else behind a single log line. The previous round's comment even stated the rule for the second arm ("the day is still the oldest, so the next iteration would select it again and spin") sixty lines above the code that did exactly that. FIXED with a monotonic per-run cursor: the day selection asks for the oldest day AT OR AFTER `cursorFrom`, and the cursor advances to `dayEnd` BEFORE any early exit, so every path leaves its day behind. A stuck day now costs one iteration and is retried on the next run instead of wedging the queue. The oversized and emptied-day arms `continue` rather than `break`, which is only safe because the cursor makes a spin impossible. 🟡 THE MINT IS NOW WRAPPED. The buzz client THROWS on any non-2xx and its retry allowlist covers only connection-level errors, so a 5xx throws on the first response. Unwrapped, bucket 1 of N aborted buckets 2..N, the day loop and the completion log — and splitting one batched call into N per-bucket calls multiplied that exposure by N, so this was a hazard the previous round's own fix created. A throw is now handled exactly like a drop: rows stay `accrued`, the same key is re-derived next run, peers still settle. 🟢 Two claims corrected rather than reworded: the module header still described a `Math.floor` the previous commit deleted (the only remaining `Math.floor` in the file was inside the sentence about it), and "a conflict is the same payment" was stated unconditionally when it holds only while no row can JOIN an already-minted (day, owner, currency) group. That precondition is now written down, along with what breaks it — a backfill writing a historical `accrued_at` would turn it into a silent underpayment. 🔴 MY OWN MUTATION SWEEP WAS THE REAL FINDING, and it is the one worth carrying forward. Every settlement test used a single-day fixture, so the loop body ran exactly once in every test. The multi-day walk, the cursor, `maxDays` and the continue-vs-break semantics were structurally unreachable — a mutant over any of them would have killed nothing. Last round's "7 mutants, 7 killed" was therefore a true statement about seven mutants that did not include the control flow that round had just rewritten. A sweep is only as wide as the mutants you imagined. Fixed by a `days()` fixture that drives N days, plus 7 tests: the multi-day walk, an oversized day not blocking later days, a dropped bucket not blocking later days, a thrown mint treated as a drop, the cursor advancing strictly, `maxDays`, and a day that empties under the loop. ⚠️ AND THAT FIXTURE IMMEDIATELY EXPOSED A TEST-ISOLATION LEAK: `vi.clearAllMocks()` clears call history but NOT `mockResolvedValueOnce` queues, so a day queue left by one test was consumed by the next and the suite was order-dependent. The mocks fed with `...Once` are now explicitly reset. VERIFICATION pnpm typecheck OK - 0 type errors affected suites 59 passed (4 files) prettier clean mutation sweep 11 mutants, 11 KILLED, each by its own named test — five of them loop control flow (no cursor advance, cursor absent from the query, oversized-day break, empty-day break, maxDays ignored) and six money path (run-day key, truncate oversized day, uncaught throw, D6 collapse, ...) positive control +1000 on the minted amount -> 2 tests red Two mutants needed a second pass and both are recorded rather than quietly re-run: `empty_day_breaks_run` SURVIVED the first sweep — a genuine gap, closed by the new empty-day test — and `oversized_breaks_run` was SKIPPED because its pattern matched 10 sites, which is a sweep that reports nothing while looking like it ran. Schema unchanged; no migration in this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(app-blocks): round 4 — the wrap stopped one statement short, and a false claim about retries Round 4's verdict was SAFE TO MERGE with no blockers: the three liveness defects round 3 set out to fix are genuinely fixed, verified by watching five mutants go red rather than by reading the claims. What it found was four 🟡s of one shape — the code is right and the comment claims more than the code does — plus two gaps in my own mutation sweep. 🟡 THE RETRY CLAIM WAS FALSE, IN THE REASSURING DIRECTION. The comment said the buzz client's "retry allowlist covers only connection-level errors — a 5xx or a 504 throws on the first response". Traced and wrong: `createTransactions` calls `post` with no options, so `shouldRetry` is undefined, and `withRetries` reads `shouldRetry ? predicate(...) : true` under a comment saying "Absent predicate keeps the historical behaviour: retry everything". `isSafeToRetry` is exported and never applied on this path. So a 5xx IS retried at the client default (3), and a run with N payable buckets can issue up to 4N POSTs during an outage. That paragraph was the file's only statement about mint cost, so it is what someone would have sized a timeout or an alert against. 🟡 THE DIAGNOSTIC COULD NOT TELL PERMANENT FROM TRANSIENT. `buzzService` is built with a `mapError`, so every non-2xx arrives as a TRPCError carrying the fixed string "An unexpected error ocurred, please try again later". A permanent 400 and a transient 503 produced BYTE-IDENTICAL log lines, repeated daily forever, so the permanent one was indistinguishable from noise — and the repo already ships `getBuzzApiStatus` to read the real status back through the wrapper. Now logged. 🟡 THE WRAP STOPPED ONE STATEMENT SHORT. The mint was wrapped; `updateMany` one line below it was not — and the sentence justifying the wrap ("bucket 1 of N throwing aborts buckets 2..N, the day loop and the completion log") stayed true, verbatim, of the unwrapped call. `id: { in: rowIds }` can carry up to `limit` ids, so a statement timeout there is not exotic. Money was safe either way; what it cost was every remaining bucket and every later day in the run. Wrapped, with its own log line — a flip that throws after a successful mint is the one case where money moved and the rows do not say so. 🟡 THE CURSOR BOUNDS BLOCKING WITHIN A RUN, NOT ACROSS RUNS — and round 3's comment claimed the stronger property ("nothing is abandoned permanently"). Both stuck-day arms are permanent: an oversized day is unsettleable until someone raises `limit`, and a persistently-rejected owner's rows stay accrued forever. Each is re-selected on every later run, so 30 accumulated stuck days would consume the whole budget and settlement would stop for everyone — the same end state the cursor was added to prevent, reached 30 days later. `maxDays` now counts only PRODUCTIVE days, with `maxIterations` (default maxDays * 4) as the absolute bound. 🟢 Removed an unkillable clause rather than leaving it to read as a guard: `threw === null &&` in the `moved` expression. `mint` is assigned only inside the `try`, so it is null on every throw path and the expression was already false there — measured, deleting it left the suite green. 🟢 Completed the test reset list: the comment claimed EVERY `...Once` mock was reset, while `create` — fed `mockRejectedValueOnce` twice — was not. 🔴 TWO GAPS IN MY OWN SWEEP, both closed, both worth recording: * `maxIterations`'s DEFAULT was unpinned — the only test exercising the cap passed one explicitly, so a mutant replacing the default SURVIVED. * `threwStatus` was unpinned — a mutant nulling it SURVIVED. And the first fix was initially WORSE than the gap: feeding the same stuck day forever "killed" the mutant only by HANGING the suite — 43s, `tests 0ms`, no named failure, indistinguishable from a CI timeout. It was also a mock artifact: in production the cursor is monotonic and bounded by `boundary`, so an unbounded default could never hang. The day supply is finite now and the mutant fails an assertion in 5ms. ⚠️ AND A REPORTING CORRECTION I OWE: round 3's sweep was reported as "11 killed, each by its own named test". Two of the five loop mutants share one killer, so the independence claim was overstated. This round's sweep prints the FULL killer list per mutant rather than the first. VERIFICATION pnpm typecheck OK - 0 type errors affected suites 64 passed (4 files) prettier clean mutation sweep 9 mutants, 9 KILLED. Killer counts reported rather than assumed: flip-unwrapped 1, stuck-days-toward-budget 1, maxIterations-default 1, threwStatus 1, mint-throw 1, D6 collapse 1, no-cursor-advance 1, run-day key 5. positive control +1000 on the minted amount -> 2 tests red Two mutants needed a precise anchor rather than a shell pattern (18 and 10 incidental matches respectively) — a sweep that SKIPS reports nothing while looking like it ran, which is the same failure shape as a silent zero. Schema unchanged; no migration in this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(app-blocks): round 5 — the productive-day fix covered one of the two arms it named, and a test I reported as closing a gap did not Round 5 returned three 🟡s. Two of them refute claims made in the previous round's commit message, and I verified both against the code before acting. 🟡 F1 — THE PRODUCTIVE-DAY COUNT DELIVERED HALF OF WHAT ITS COMMENT PROMISED. `productiveDays += 1` fired as soon as a day had rows, BEFORE a single mint was attempted. The comment enumerated two permanent stuck arms and said the count now excluded both. Only the oversized arm was excluded — it `continue`s above the increment. The persistently-rejected-owner arm, named verbatim in the same comment, reached the increment and was counted productive even when every bucket dropped and zero rows flipped. So one permanently-rejected owner still accrues one stuck-but-"productive" day per day, and thirty of them exhaust `maxDays` on stuck days alone — settlement stops for everyone, which is the failure the count was added to prevent. The test named for that behaviour only ever exercised the oversized arm (`limit: 1`), so it passed while its own arm was uncovered. Now counted AFTER the bucket loop, conditional on something landing, and pinned by a test that drops every bucket on two days and asserts the third still settles. 🟡 F2 — THE MOTIVATING EXAMPLE WAS THE ONE CASE WHERE THE CLAIM IS FALSE. The new `threwStatus` comment said "a permanent 400 and a transient 503 produce BYTE-IDENTICAL log lines". `mapError` names 400, 404 and 409 explicitly ("Your request is invalid", "Not found", "There is a conflict with the transaction"), so those three were already distinguishable. The fix is still worth having — 401, 403, 408, 429, 500, 502 and 503 all fall to `default` and genuinely are identical, so a permanent auth failure versus a transient outage was the real indistinguishable pair. The example was wrong, not the reason; corrected rather than reworded. 🟡 F3 — A TEST I REPORTED AS CLOSING A MUTATION GAP DID NOT CLOSE IT. The `threwStatus` test used a plain Error carrying a `status` property and asserted `toHaveProperty('threwStatus')` — a single-argument EXISTENCE check. `getBuzzApiStatus` returns undefined for that shape, so the field logged `null`, and the mutant `threwStatus: null` SURVIVED while the test passed. Re-measured here before fixing: mutant applied, 30/30 still green. The previous commit and PR comment both recorded that gap as closed. It now uses a real `BuzzApiError` inside a `TRPCError` cause — the shape production actually produces — and asserts the VALUE is 503. 🟡 F4 — WRAPPING THE FLIP MADE ITS FAILURE SILENT. Before the wrap an `updateMany` throw failed the job and was loud; after it, the run returns success with only an Axiom line. A systemic flip failure would report `rowsSettled: 0` on a job that says it succeeded, every night, while money left on day one of each bucket. Added `flipFailures` to the result and the job log — the one counter that means money moved without a settled row — and put the amount on the log line, which carried `settlementKey` but not the sum. 🟢 F5 — the flip-failure message asserted a state the code cannot observe. A connection drop after the UPDATE commits raises with the rows already flipped, so "money moved, rows still accrued" would be the opposite of the truth on a line an operator acts on. Now "mint landed, flip did not confirm". 🟢 F6 — the `buzzMinted` comment claimed `count > 0` distinguishes "a payment this run did not make". It does not: it cannot tell a fresh mint from a conflict on a key an earlier run already paid, and the wrapped flip makes that path ordinary. The cross-run total stays correct, so this is reporting, not money — stated rather than reworded, because the exact claim is what a reader would rely on. VERIFICATION pnpm typecheck OK - 0 type errors affected suites 65 passed (4 files) prettier clean mutation sweep 3 mutants re-run against the new guards, 3 KILLED, each by its own named test: threwStatus nulled (this one SURVIVED before the fix — re-measured, not assumed), productiveDays counted unconditionally, flipFailures not counted. ⚠️ SCOPE OF THAT SWEEP, STATED BECAUSE THE LAST FOUR ROUNDS OVERSTATED THEIRS: it covers the three guards this round added or repaired. The nine mutants from round 4 were not re-run. Schema unchanged; no migration in this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(app-blocks): split slice 2 — this PR is the ACCRUAL LEDGER only, settlement goes to 2b The settlement rail has no consumer. `accrueBlockAuthorFee` has zero callers on main, so the table it writes accrues nothing, so the daily job it feeds settles nothing — a cron, a Buzz-minting path and 25 tests whose first real exercise would be the day a charge path lands. That is a lot of unexercised money-moving code to carry through review on the strength of a table that is empty by construction. It moves to `zach/app-blocks-author-fee-slice2b` and comes back with the viewer-charge path that gives it rows. What leaves this PR: - `settleBlockAuthorFees`, `SettlementBucket`, `SettleBlockAuthorFeesResult`, `utcDayStart`, `keyForBucket` - `src/server/jobs/settle-block-author-fees.ts` and its registration in the run-jobs webhook (reverted to the main version verbatim) - the 25-test settlement suite What stays — the ledger, which is the part that has to exist first: - the migration, unchanged and NOT re-timestamped; it is already hand-applied to dev and all three prod instances - `accrueBlockAuthorFee` and the six tests covering it - the `bafa_` id helper - the call-site-ledger registration and the migration-comment filtering fix RENAMED `author-fee-settlement.service.ts` to `author-fee-accrual.service.ts`. A file named for settlement that settles nothing is the same defect this PR's own audit ladder found four times in rounds 4 and 5 — a name claiming more than the code does. The call-site ledger keys on the file path and fails on both GROWTH and SHRINK, so its entry moved with the file. `STATUS_SETTLED` is now exported despite nothing in this slice reading or writing it. It names one of the two states of a CHECK-constrained column this slice's migration ships, and 2b is the writer; exporting it keeps one spelling of the literal across both slices rather than letting 2b re-declare it, where a typo would match no row and fail silently. Its comment says so, so its presence is not read as evidence that anything settles. Kept as a commit on top rather than a force-push: the six audit rounds behind the settlement code are the record of how it got correct, and 2b inherits that history. The PR diff is computed against the merge base, so what reviewers see is the ledger-only slice either way. Rail is still DARK: 0 callers of `accrueBlockAuthorFee`, flag `app-blocks-author-fee-enabled` is `enabled: false`. Tests: 4323 -> 4298 in the blocks suite, exactly the 25 settlement tests, moved not deleted. The four-suite run goes 65 -> 40. Typecheck 0 errors; the 43-file lint-rules suite is green at 599. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 16:09:08 -05:00
Justin Maier	8aa9e5cef2	revert(remix): drop the verify hint from the remix menu (#4951) ... Justin reviewed it on a dev server and does not want it there. The menu is back to exactly its pre-#4939 state — same three options, same labels, no per-option annotation. What stays from that PR is remixClaimState in utils/remix-claim.ts, which has no user-visible surface and is what keeps the 0.75 rule to one derivation for the free-path work. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 13:58:52 -06:00
Justin Maier	c4c402ef40	fix(search): page the models delta scan by keyset, not OFFSET (#4938) ... * fix(search): page the models delta scan by keyset, not OFFSET prepareBatches walked the set of models updated since the last run with an unordered OFFSET/LIMIT loop. The set is re-evaluated on every page and its membership moves while the scan runs: an edit that unpublishes a model, or flips it to Unsearchable, removes a row from under the cursor and shifts every later page down by one, so a model eligible for the whole scan is silently never indexed. At ~1,659 published edits per day a multi-page scan meets that routinely, and this loop is also what an index repair leans on. Ordering the OFFSET query would not have fixed it -- order was never the problem, membership was. Page by a forward-only id cursor instead, which is unreachable by a membership change because ids are immutable. prepareBatches is hoisted to an exported prepareModelsBatches, mirroring prepareUsersBatches, so the paging can be driven by a test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(search): make the keyset paging fake refuse what it cannot read Review found the fake answered queries it had not understood, so three mutations of the production query passed: - ORDER BY id DESC passed all four cases. The fake sorted ascending in both arms whatever the SQL asked, and /ORDER BY id/ matches DESC. Against a real database that mutation walks the cursor backwards from the top of the table and the scan never terminates. - A literal LIMIT 2000 fell through to a members.size default, handing back the whole set on page one -- at which point the headline case passes under an OFFSET implementation too. - id >= instead of id > fell through to an uncursored read and reddened with "the scan is not advancing", which is not what that defect does. The fake now honours ORDER BY direction and throws on a query whose LIMIT or cursor it cannot find. Case 1 also asserts the mid-scan edit actually landed -- keyset is meant to be unmoved by it, so nothing else in that case could tell a dead mutation from a working one -- and pins batchSize against the production constant so page-size drift names itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(search): pin the page query's eligibility predicates The paging fake models membership as an opaque set of ids, so it cannot see which rows the WHERE clause selects. Two review lanes independently demonstrated the consequence: deleting any one of the three predicates left the whole suite green. Dropping the updatedAt bound turns the delta scan into a full scan of every published model every 15 minutes; dropping the availability bound puts Unsearchable models into the public index. Both are the shape of an ordinary WHERE-clause tidy-up, and this is the only test that reads this query. Pinned textually rather than by teaching the fake to carry per-row timestamps: one assertion covers all three predicates where a behavioural fixture would only cover updatedAt, and a smaller fix round is the safer one. The watermark comment now names the symbols it depends on instead of restating base.search-index's semantics, which would rot silently if that file changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(search): pin the page query by whole clause, values, and the empty page Round three of review found three mutations of the query that left the suite green. Substring assertions were the common cause. - Widening. Appending `OR availability = 'Unsearchable'` leaves every asserted fragment present while AND binds tighter than OR, so every Unsearchable model is returned on every page. The sibling test next door already carried a written record of an adversarial round that beat this same assertion shape, so its `norm`/`renderTag`/`whereClausesOf` helpers move to `sql-shape.test-utils` and both files now pin a whole normalised clause with toBe rather than substrings. - Value corruption. `renderTag` renders a bind param as `?`, so the clause is blind to values and `Availability.Unsearchable` -> `Private` was green. The page query's bind values are pinned separately. - The empty-page break was unreachable: every fixture ended on a short page, so deleting `if (!ids.length) break` left the suite green while production reads `ids[ids.length - 1].id` off an empty array and kills the index job on any run where the eligible set is an exact multiple of the page size, or empty. A fixture of exactly READ_BATCH_SIZE members reaches it; the deletion now fails with that same TypeError. No production change in this commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(search): pin that the processor runs the function under test Round four of review demonstrated the cheapest possible revert of this PR: leave the exported prepareModelsBatches alone and re-inline the old OFFSET body at the wiring site. Production goes back to row-losing paging and all 17 tests stay green, because every case imports the exported function directly and nothing in the repo read modelsSearchIndex.prepareBatches. The test file's own header claimed such a revert would redden it. That was false. createSearchIndexUpdateProcessor now returns prepareBatches, alongside the updateSyncChunkSize it already exposed for the same reason, and the test asserts the processor runs the function it drives. Measured cost, recorded in the test: a behaviour-preserving wrapper at the wiring site also fails this. That is inherent to an identity assertion, and the fix is to keep the wiring a direct reference. Also drops `toBe` from the sql-shape docstring, which named an assertion neither of its two callers uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(search): pin the bounds query and the id range it returns prepareModelsBatches returns four things; this file pinned two. Nothing read startId/endId, and the fake short-circuited the MIN/MAX statement on text.includes('MIN(id)') and handed back a hardcoded row, so the whole bounds query was invisible. Review demonstrated two mutants that left every case green: - swapping the aliases to MAX(id) as "startId", MIN(id) as "endId" makes endId - startId negative in base.search-index's range fan-out, so newly created models silently stop being indexed on every run; - deleting the bounds query guts the full rebuild, which then indexes zero models while the case named "issues no page query at all on a full rebuild" stays green, because it only asserts no page query fired. The fake now answers the aggregate the statement asked for, rather than returning fixed numbers under fixed names, so an alias swap produces a different id. The bounds statement's WHERE clause and binds are pinned the way the page query's already were. The "createdAt" bound there against "updatedAt" on the page query is pre-existing and deliberate, and is now pinned so a one-word change between the two cannot pass unnoticed. The file header claimed the fake refuses a query it cannot read. The bounds query was the one place it defaulted instead, which is why this survived six rounds; the header now says what the fake actually does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(search): pin the rebuild path's bounds, and both statements' table The rebuild branch was the one place startId/endId are the entire output, and the only case on it asserted that updateIds was empty and no page query fired. Both of those are also true of a rebuild that does nothing: returning { startId: 0, endId: 0, updateIds: [] } early for a missing watermark passed every case, and makes base.search-index's range fan-out zero tasks, so a whole index rebuild creates no batches and indexes nothing. Neither statement's table was pinned either. whereClausesOf captures from WHERE onward, so FROM "Model" -> FROM "ModelVersion" in either query left the file green while the scan paged a different entity. Also corrects the header's frequency claim. It said a multi-page scan meets concurrent edits as a matter of routine, citing ~1,659 edits a day; at a 15-minute cadence that is ~17 rows against a 2,000-row page, which argues the single-page case. The PR description was corrected for this and the source was not, which left the retracted claim in the file the next reader actually opens. The corrected text also states the mechanism that does not need an edit at all: no ORDER BY over a parallel seq scan lets synchronize_seqscans cut successive pages out of different orderings. That paragraph first shipped broken, because a cron string in a block comment closes it. The comment now says so rather than leaving the next person to rediscover it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 13:57:08 -06:00
briant	c6da1a2dc4	fix(ingestion): stop the retry cron double-submitting a scan already in flight ... The Image INSERT trigger queues a new image immediately, but ingestImage stamps scanRequestedAt only once its upload-path submit returns. A cron run landing inside that window read the NULL as "never submitted" and submitted a second workflow for the same image. Measured on prod 2026-09-18: of 41,865 images scanned in 12h, 166 carried two workflow ids; 161 of those were created within 30s of a cron tick, against a 9.8% uniform baseline, spread over 112 users. A Pending image with no scanRequestedAt is now deferred for SUBMIT_IN_FLIGHT_GRACE minutes. It stays in the JobQueue while deferred — without that it prunes as stale and an image whose submit died silently would never be re-driven, which is the trigger's whole purpose. The deferral count is reported alongside waitingForRetry. Two existing fixtures modelled "new image, never submitted" as createdAt: now, which the grace defers; aged them past it so each test still exercises its own subject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-18 13:36:01 -06:00
Justin Maier	d7038c5aa8	fix(home-blocks): hydrate the viewer's own reactions on cached image blocks (#4935) ... * fix(home-blocks): hydrate the viewer own reactions on cached image blocks Home block payloads are stored in one Redis entry with no user segment AND served through edgeCacheIt with canCache left true, so every viewer is handed the same image objects with `reactions: []`. reaction.toggle acts on the database row rather than on what is drawn, so a viewer whose reaction shows un-highlighted clicks it and deletes the reaction they already had. Keeps the shared payload anonymous and hydrates on the client instead: a new reaction.getMyImageReactions procedure, and a useHydratedImageReactions hook that the three home blocks rendering ImageCard call on the list they hand to ImagesProvider, which is the same window the image detail dialog browses. The query and its grouping already existed twice inside image.service.ts; both now call one exported getUserReactionsForImages, which the new procedure is the third caller of. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(home-blocks): make the hydrated list the only one a block can render Review found the guard proved the blocks MENTION the hook, not that the hydrated array reaches the cards: keeping the call and rendering a second, un-hydrated binding restored the bug in full with the suite green. Fixed structurally rather than by a stronger string match. The hook result is now passed straight into useDedupedCappedItems, so the capped list is the only array in scope, and the guard asserts that shape. Hydrating before the cap also stops the query key churning as earlier blocks publish their dedupe claims. Also from review: - reactionQueryChunks is pure and exported, so the signed-out gate has a test. Losing it is one UNAUTHORIZED request per home block for the majority of front-page traffic. - The chunk size and the input schema's max are two copies of one number; a test pins them equal. Divergence fails zod, React Query swallows it, and the grid silently stays un-hydrated. - `reactions` is optional on the hook's item type, so the collection blocks pass their union in without a cast, and the merge reads it with `?? []`. - chunkIds moves to array-helpers; sticker.util, StickerPlacementBatchProvider and RemixGalleryBatchProvider had three copies of one body between them. - Dropped the `toContain('useHydratedImageReactions')` assertion: deleting the call leaves the import, so it barely fails. The structural assertion subsumes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(home-blocks): correct the measured cost, and stop re-asking per mount Round 2 of review. The cost model I wrote into the hook's doc comment was wrong in the direction that understates it: a FeaturedCollections block renders one section per pick and its renderCount is 5, so its picks do not share a call, and one of the three Feed blocks carries models and asks nothing. Nine requests on today's prod home page, not six. The per-request figure was measured at the post-cap shape this branch replaced; over a block's whole pre-cap pool it is 0.4-1.2 ms, about 4.6 ms of replica time for the page. Both numbers corrected rather than dropped, since a follow-up ticket quotes them. Two behaviour changes, both from the same review: - The ids are sorted before chunking, so a repeat visit can reuse the answer. Every block shuffles its pool on mount, which made the query key fresh every time and `staleTime` nearly inert. The sort lives here and NOT in `chunkIds`, whose other callers page and need insertion order. - Hydration waits on hidden preferences. `useApplyHiddenPreferences` does not block, so between the payload landing and the preference maps resolving it hands back the unfiltered pool; asking about that first spent a whole extra round of requests per cold load. The guard now accepts either nesting order. The property that kills the un-hydrated binding is the inlining, not which hook is outermost, and hydrating after the cap is a defensible shape this guard has no business forbidding - it was this branch's own design one commit ago. Its comment also stops claiming the bug is impossible to write: `filtered` is still a binding. What the guard removes is the INVITED mutation. The router-rung assertion is an allow-list of authed procedures rather than one spelling, so it reds on a loosening and not on a tightening. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(home-blocks): take the entity, not a boolean the caller derives Review found a green mutant against the subject: flipping `{ enabled: !loadingPreferences }` to `{ enabled: loadingPreferences }` switched hydration off on every render that draws a card, because the blocks return a skeleton while that flag is true. Every check in this repo stayed green - the guard reads the composition rather than the argument's value, and the pure tests prove the gate behaves correctly with the boolean it is handed, never that the boolean handed to it is right. So the boolean is gone rather than guarded. The hook takes the entity type, required and typed, and derives `entity === 'image'` in one place the existing pure tests already reach. A mistyped 'images' is now a compile error; there is no default to flip; and `type === 'image'` is no longer restated at three call sites. The `!loadingPreferences` half is deleted outright rather than moved. It was a no-op: `filterPreferences` returns `items: []` while preferences load, so the hook was already being handed an empty pool, never the unfiltered one. The comment claiming otherwise was the stated rationale for the gate, duplicated in all three blocks. The rung check reads every occurrence rather than `match`'s first. A doc comment above the declaration that quotes it - the natural thing a future editor writes - would otherwise satisfy a first-match check while the declaration underneath said something else. Three comment corrections, all of them claims that had stopped being true: - The blocks said inlining left "the only array in scope". It does not; `filtered` is still there. What inlining removes is the INVITED mistake, which is the one that happens. - The pre-cap move was justified partly on the detail dialog browsing the wider window. It does not: all three blocks hand `ImagesProvider` the capped list. The decision rests on the re-keying alone. - `chunkIds`' docstring argues against sorting and now has a caller that sorts first; it says why. The randomised shuffle control is reversed instead, so it is certainly red rather than almost surely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(home-blocks): cover the hook body, which nothing in the repo executed Two lanes independently found the same class: `reactionQueryChunks` and `mergeUserImageReactions` are well covered, and the composition between them was covered by nothing. Three one-token edits inside the hook body switch hydration off on the front page with typecheck, eslint, the guard and the whole unit suite green - the `byImageId` memo dep, the final memo's dep list, and the `query.data ?? {}` fallback. Nothing else can see those. `react-hooks/exhaustive-deps` arrives as a WARNING via next/core-web-vitals, `lint` is `eslint src/` with no `--max-warnings`, and the CI workflow says so out loud - "Errors only (no --max-warnings): the repo has 3,470 warnings". So dependency-array correctness has no automated enforcement in this repo, and one of the two dep lists sits under an `eslint-disable-next-line react-hooks/exhaustive-deps`, which makes "clean up the disabled rule" an ordinary edit for someone who has never read this ticket. The test renders the hook with a stubbed `useQueries`, asserts the un-hydrated state synchronously as a negative control, then makes the queries report data and asserts the hydrated state. It asserts a state that ARRIVES, so there is nothing on a timer to race. Two things this round got wrong first and are worth recording: - The stub originally returned its canned results whatever the hook asked for, so the non-image case passed data to a surface that had issued no query. It now returns one result per chunk, as the real `useQueries` does. The negative control is what caught it. - The first sort-direction assertion, `chunks[0][0]).toBe(1)`, could not fail: 1 sorts first lexicographically too. Measured, not assumed - the mutant stayed green on that line. It now asserts where the two orders actually disagree, at the second element. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(home-blocks): make the query stub faithful in content, not only in count Follow-up on the hook-body test, from the same lane that asked for it. The stub kept the descriptor COUNT honest and discarded what was in them, so `{ imageIds: chunk }` changed to `{ imageIds: chunks[0] }` stayed green — and in production that makes every chunk after the first ask about the first one's ids, so the back half of a large grid never hydrates and its cards go back to deleting on click. The stub now keeps the descriptors and a test asserts the hook asked each chunk about its own ids. Control: that mutation reds with `expected [ 100, 100 ] to deeply equal [ 100, 50 ]`. That path is unreachable on today's config — `FEED_FETCH_CEILING` and the collection limit are both 100, which is `REACTION_FETCH_CHUNK`, so a block's pool is always exactly one chunk. The test is there for the day one of those numbers goes up, which is a one-token edit nothing else connects to this hook. The comment says so rather than implying live coverage. Three smaller things in the same family: - The stub mapped one result per descriptor instead of slicing. A `queryResults` shorter than the descriptor list silently handed the hook a shape `useQueries` cannot produce. - `queryResults` is reset in `beforeEach`. Inheriting a previous test's value would inherit it as HYDRATED data, which is the direction that produces a false green. - `images` being hoisted out of the probe is load-bearing for the dep-list control — a fresh identity each render makes the memo recompute regardless — and nothing said so. Now it does. The `entity: 'model'` test keeps its assertion and loses its claim: the stub is what withholds the answer there, so it cannot tell a hook that filters from one that never asked. What it does prove is that the gate survives the whole body, which the pure tests next door cannot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(home-blocks): assert what came back, not only what was asked Round 6. The multi-chunk test proved the hook asked each chunk about its own ids and never looked at the hook's output, so the response half of the same shape was open: collapsing `Object.assign({}, ...queries.map(q => q.data ?? {}))` to `queries[0]?.data ?? {}` was green, and in production that drops every chunk after the first — images 101+ render un-hydrated and the first click deletes. It now asserts both ends, and the assertion on the LAST image is the only place in the suite where a chunk other than the first has to land. Same round, same root cause one argument to the right: the stub captured the descriptor's input and discarded its options, so deleting `{ staleTime: 60_000 }` was green. That is the option the round-3 sort exists to make worth having — a repeating query key buys nothing if nothing holds the answer — and the pure sort tests structurally cannot see it, because it lives in the hook. The stub now captures both arguments and the test pins it. Controls, applied and reverted: - `Object.assign(...)` -> `queries[0]?.data ?? {}`: red, `expected [] to deeply equal [ { userId: 9266475, … } ]` - `{ staleTime: 60_000 }` deleted: red - `{ imageIds: chunk }` -> `{ imageIds: chunks[0] }`: red, `expected [ 100, 100 ] to deeply equal [ 100, 50 ]` Multi-chunk remains unreachable on today's config — both ceilings are 100, which is the chunk size — and the test says so. The point of closing both halves rather than one is that the asymmetry would be invisible to whoever raises that number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(home-blocks): cover the render production actually performs first Round 7 found the third dependency array, and unlike the multi-chunk pair this one is reachable today on every page load. `useApplyHiddenPreferences` returns `items: []` unconditionally while hidden preferences load, so every home block hands the hook an EMPTY array on its first render and the real pool on a later one. Drop the id join from the `chunks` memo's deps and it freezes at `chunkIds([], 100)`: nothing is ever asked, the merge is a permanent no-op, every card renders un-hydrated, and the first click deletes. Typecheck, eslint, the guard and all three existing hook-body cases stayed green — they all start with a NON-EMPTY pool, so a frozen `chunks` is frozen at the right value in every one of them. The new case mounts empty and populates on rerender, which is the sequence production performs. It is deliberately a separate case rather than an amendment to an existing one, and the file now says why: the fixture decides which dep list a case can see. A stable `images` binding can see the `byImageId` and final memos and cannot see `chunks`; a growing `images` reaches `chunks` and cannot see the final memo, because a fresh identity makes that one recompute regardless of its deps. The two shapes are mutually blind, so folding them together would close one and silently unarm the other. Controls, applied and reverted, all three dep lists at once to prove the new case disarmed neither of the existing ones: - `chunks` deps -> `[userId, entity]`: red on the new case - `[images, byImageId, userId]` -> `[images, userId]`: still red - `byImageId` deps -> `[queries.length]`: still red Also renamed the stub's captured parameter from `imageIds` to `input`, since the first descriptor argument is itself an object with an `imageIds` key and `d.imageIds.imageIds` read like a typo. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(home-blocks): pin the pool CHANGING, not merely growing Round 8. Keying the `chunks` memo on `images.length` instead of the id join was green against all four cases, because every one of them either holds the pool still or only grows it. In production `useApplyHiddenPreferences` hands back the PREVIOUS items during a refetch and then the new ones, and a home block's payload size comes from its config rather than its content — so a same-sized, different-membership swap is the ordinary refetch, not an exotic one. Under that mutant the hook keeps asking about the previous ids, the new images render un-hydrated, and the first click deletes. Closed by extending the existing empty-then-populated case with a same-length swap rather than adding a fifth fixture, so it disarms nothing. The fixture rule in the file's doc comment is restated to one that generalises: `chunks` has three deps and each needs something to VARY across a rerender to be visible at all, while the other two memos need something to HOLD STILL. "Stable versus growing" read as an exhaustive pair and is not one — chunk count is its own axis, and `userId` and `entity` are axes no case varies today, since `useCurrentUser` is a constant mock. Dropping either of those from the `chunks` deps is green against every case in this file. Recorded in the comment rather than quietly left out. Controls, applied and reverted, all five at once so a new case cannot silently unarm an older one: - `chunks` deps -> `[images.length, userId, entity]`: red on the extended case - `chunks` deps -> `[userId, entity]`: still red - `[images, byImageId, userId]` -> `[images, userId]`: still red - `byImageId` deps -> `[queries.length]`: still red, two cases - response merge -> `queries[0]?.data ?? {}`: still red Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(home-blocks): assert the non-image case asks for nothing, as its name says Round 9 found no green mutant. Its one free item: the non-image case asserted only the hook's OUTPUT, never that no query was issued — which the pure `it.each` next door already covers. Its name has promised the stronger thing for four rounds, and the `asked` capture that makes it possible arrived two rounds after the case was written and the case was never revisited. With the assertion it is the only place pinning that no query is issued outside `chunks`. Control: removing `entity !== 'image'` from the gate reds it with `expected [ Array(1) ] to deeply equal []`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 13:27:06 -06:00
Justin Maier	0340f692bf	docs(tests): correct the seam guard's prose, and the false absolutes each fix introduced (#4940) ... * docs(tests): correct the seam guard's header and cut what argues rather than informs The header claimed "you cannot call the function without importing the module it lives in". A consumer reached through a re-exporting barrel matches neither half of the detector; what saves the ledger is that the BARREL matches the `from` clause and joins it, one file away from the consumer that gates. Stated, with the limit, because a reader trusting the absolute would stop looking. Cuts the dated "85/85 green" count, the change-log narration of what the ledger used to be, the summary of the assertions below it, and a clause arguing the guard is correct. The mutants are the proof now; the header does not need to make the case. Comment-only: the diff contains no non-comment lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): restore the truncated clause on the corpus-loop control The comment above `expectedCorpus` ended mid-sentence, dropping the half that explains why scoping the corpus loop is green: no detector fixture can observe that loop at all, because `verdictFor` seeds `SOURCE` directly and runs past it; and every corpus member already carries the token such a filter would scope by, so the filter excludes nothing. Green there means inert, not caught, and the re-derivation below is what would actually disagree. Comment-only: the diff contains no non-comment lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): correct the detector limit and move it onto the code it constrains The header said a consumer reached through a re-exporting barrel matches neither half of the detector. It matches the symbol half: a name-preserving re-export still writes `classifyGatedImageForViewer(`, so the consumer joins the ledger at its own path. Only a barrel hop that also renames escapes both. That mattered more than wording, because the barrel case is the symbol half's ONLY unique contribution - an alias, a namespace import and a re-export from the logic module all carry the `from` clause - so the header handed a future tidier an argument for deleting it. The corrected statement lives on `isCallSite` rather than in the header, where it sits beside the expression it describes instead of drifting from it. The same docblock justified the symbol half as catching a namespace import or a re-export, both of which the import half already catches. The header keeps the rule and drops what restated it: the detection mechanism (stated twice more, on `LOGIC_MODULE_IMPORT` and `isCallSite`), the paragraph arguing a per-file suite could not catch this, and a summary of two sibling suites' assertions - which also over-claimed, since the grid withholds an unrated image's url from its author too when the image is flagged or scan-refused. The corpus-loop comment now states the fact rather than the mutation-testing note: every corpus member's path contains the token, so narrowing the loop by it excludes nothing. Comment-only: the diff contains no non-comment lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): state the detector's escape shapes as a property, not a list The previous round's docblock said only a renaming barrel hop escapes both halves. It does not. A renaming dynamic import escapes both with no barrel anywhere: `await import('…logic')` carries no `from` clause for the import half, and a renamed destructure puts a `:` where the symbol half needs `(`. The file still enters the corpus, is scanned, and comes back not-a-call-site. A helper handed the function as a value escapes the same way. That distinction is the safety-relevant part and it is now stated: a renaming barrel hop reddens this suite at the barrel, while a renaming `import()` or a helper reddens nothing at all. Written as a property of what escapes rather than an enumeration of shapes, so finding a fourth shape does not make it false again. Restores the clause saying each file type-checks and each file's own suite passes. It was cut as self-justification, but it is the only statement of why the two per-consumer suites cannot substitute for this one, it lives nowhere else, and being about the nature of cross-file defects rather than about any code, it cannot drift. Comment-only: the diff contains no non-comment lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): state both halves as spellings, and stop claiming the per-file suites are blind Two false statements, both introduced by the previous two rounds of this branch. The restored clause said each file type-checks and each file's own suite passes, so this is the class of defect no per-file suite can see. Not true of the tree as it stands: `block-post.service.test.ts` has an `it.each` whose first two rows are `{ ingestion: 'Pending' }` and `{ nsfwLevel: 0 }`, both reaching the gate at `block-post.service.ts:592`, so rewriting that gate as `=== 'hidden'` fails them. It was true when the seam was created and stopped being true when those cases were written. The narrower statement is the one that does not rot: neither file is wrong on its own, so nothing fails until someone writes a per-consumer case for the new state - which is exactly what a third consumer would not have. The escape condition said a file escapes both halves by naming neither the module nor the symbol. A renaming `import()` names the module in full and escapes anyway, because the import half keys on a `from` clause rather than on the module's name - so the condition excluded a case the same paragraph listed two clauses later. Both halves are now stated as what they are, spellings, with the `from` forms left to the regex's own docblock instead of restated fifty lines away. Every false absolute on this file has been a claim about that regex written far from it. Comment-only: the diff contains no non-comment lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): cut the detector's consequence claims, and make its referent exact The docblock claimed that when an escape shape lands in an already-ledgered file, the `status !== 'visible'` containment still holds the line. False for half the ledger: `targets` is `EXPECTED_CALL_SITES` minus `MAY_BRANCH_ON_HIDDEN`, pinned by name to `block-post.service.ts` alone, so the grid projection is ledgered with no content assertion against it at all. The claim read as a backstop that does not exist for the one consumer allowed to branch on `=== 'hidden'`. Its companion - that a barrel hop still reddens at the barrel - was unconditional in the same way: a barrel re-exporting via an extension the regex does not list matches neither half itself, so that hop reddens nowhere either. Both are deleted rather than qualified. This is the fifth false statement in this docblock in four rounds, every one of them a consequence claim about a text matcher; a deletion is the only edit here that cannot produce a sixth. What remains is the part that has survived every round: each half pins a spelling, and a file writing neither is not a call site. That sentence defers to `LOGIC_MODULE_IMPORT`'s own docblock for the forms, which makes it load-bearing, and it under-described them - it named the rooted, relative and extensionless spellings while the regex also accepts `.ts`, `.tsx`, `.js` and `.jsx`. A reader following the pointer to check a `.js` specifier was told by implication it was not covered. Now stated exactly, with its closed end. Comment-only: the diff contains no non-comment lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 11:41:10 -06:00
Justin Maier	e50d122cb2	feat(remix): say at click time which remix modes we can verify (#4939) ... * feat(remix): say at click time which remix modes we can verify A prompt-reuse remix feeds no image to the job, so the server resolves no sourceImageIds and the remix gallery correctly refuses the free submission. The submit modal already explains that (RemixGallerySubmitModal renders freeUnavailableReason outside the free/paid block, deliberately). What is missing is earlier: the three remix options look alike at the moment of choosing, so the difference is only discoverable after generating. Mark the two that feed the image itself. On the options that HAVE the property rather than the one that lacks it — reusing a prompt is a legitimate remix and the menu should not read as warning someone off it. It says we can verify, not that the submission will be free: whether free is on offer is five more rungs in freeSubmissionOffer, and a menu that promised it would be overruled at the modal. remixClaimState is split out of remixClaimHolds so the claim's outcome — holds, carrier, why not, and the score where the prompt is the carrier — has one derivation. The predicate is now a wrapper over it. A surface that computed its own similarity would be a second copy of the 0.75 threshold, and the copy that drifts is the one telling someone their remix still counts while the submit is about to drop it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(remix): assert the reason, not just the boolean Review round. The comment on remixClaimState said the drift notice renders it. There is no drift notice — it moved to the following PR — so the comment asserted a consumer that does not exist. Corrected to state the instruction that is actually load-bearing: this is the only derivation, reuse it rather than recomputing the threshold. Every existing test asserted through the holds boolean, so carrier, reason and score could take any value and stay green. That is the half the next PR branches on. The pair that matters is drifted against uncarried: both are holds:false and nothing else separates them, so reporting drifted for a cleared prompt box would tell someone they had changed their mind at the moment they cleared it to retype. Asserted field by field rather than with toMatchObject, which truncates to "expected { holds: false, ...(3) } to match object { holds: false, ...(3) }" and never names the value that was wrong. Verified by reverting: swapping the reason on the cleared-prompt branch now fails with "expected 'drifted' to be 'uncarried'". Three comment blocks trimmed to the fact they carry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(remix): cover the three branches a mutation passed through Second review round found the first one incomplete. Three branches had no assertion at the state level, so mutating them printed nothing: the no-remix reason, the branch where the remix seeded no prompt at all — which is a different branch from the form's prompt being cleared, and carries nothing rather than the prompt — and the media branch's reason. The drifted score assertion was vacuous by fixture rather than by shape. The two prompts share no token after cleaning, so every term in the similarity is zero and the score is exactly 0; toBeLessThan(0.75) is then true of any bounded wrong answer, including a constant. Replaced with an ordering over three fixtures whose scores were measured rather than assumed: 0.7601 for two tags changed, 0.2969 for four of eleven left, 0 for disjoint. A constant score now fails with "expected 0.1 to be greater than 0.1". Comments cut, not trimmed. "A server-side check is coming" was the same unfalsifiable forward claim as the one this round already corrected, for a PR that does not exist. The block comment over the tests restated what the test names say and what remix-claim.ts states three lines from the branch it describes, and the note defending the assertion style was the fix round arguing with a reviewer inside the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(remix): drop the comment defending the assertion, keep the calibration The note above the ordering assertions described the previous version of the test rather than this one, and claimed the null fallback guarded a case that cannot occur at that call site — all three scores come from the branch that always returns a number. The test name already says what the assertions say. The fixture docs keep what a reader cannot recover by eye, that the overlap is calibrated rather than incidental, and lose the measured decimals. Those are pinned by no assertion, so a retuned similarity would leave them wrong with everything still green. The numbers are in the PR body, which is dated by nature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(remix): assert the expired branch carries nothing Third review round, third branch whose mutation printed nothing: the expired claim asserted its reason, its holds and its score, and not its carrier. Verified silent by running it — 15 passed with carrier changed to 'prompt' — and now fails with "expected 'prompt' to be null". Two comments corrected rather than trimmed. The fixture docs said eleven tags where the seed has nine; eleven is its token count, which is what the similarity works on, but the sentence says tags. The state doc claimed expired and uncarried are not caused by the person, which is false for the branch where they cleared the prompt box themselves — the inline comment two lines below says exactly that. The predicate's own doc restated its signature and went. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(remix): assert the whole state on every branch Three review rounds each found one more field unasserted on one more branch, a different field each time. Assertions written per branch pin what their author was thinking about, so a fourth round would have sampled the same hole rather than closed it. toEqual over the whole object on every branch, table-driven. A field-level mutation cannot survive it, because there is no field that no assertion mentions. Verified against all five survivors the rounds found, plus a sixth chosen on a branch none of them touched: all six red, the sixth printing carrier prompt against media. score is expect.any(Number) where the prompt carries the claim. Its value is pinned by the ordering beside it, which rules out a constant and a wrong ranking and does not rule out a monotone-but-wrong score. That is the ceiling of an ordering property rather than a gap here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 11:38:30 -06:00
briant	5ff986be11	chore(moderator): release moderator-v0.0.69 moderator-v0.0.69	2026-09-18 11:03:07 -06:00
briant	6495f6f61a	feat(moderation): minor-flag lookup + search, server-side ToS'd filter, self-harm unpublish reason ... - Minor Hash Matches: a search box (model id, user id or username) filters all three tabs, and a model-id search shows that model's minor-flag state with Revert / Keep flagged regardless of the 30-day auto-flag window. An aged-out same-uploader auto-flag previously had no revert path, so the sweep kept re-applying it. (ClickUp 868m6mzbv, 868m6mw8a) - Bulk Image Manager: "Only ToS'd" / "Hide removed" now filter in the query (rows and count) instead of over the loaded page, where an account's few removed images sat thousands of rows past the window and never appeared. Getters take a BatchWindow options object. (ClickUp 868m67w6t) - Unpublish reasons: add 'self-harm' (ToS 9.6(g)) to the model and article lists. (ClickUp 868m576m8) - Docs: parity checklist, minor-hash detection doc, extraction-plan line count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 10:50:09 -06:00
Justin Maier	3cc390be62	fix(search-index): report the ids a transform drops instead of writing nothing silently (#4933) ... * fix(search-index): report the ids a transform drops instead of writing nothing silently A row that pullData returns and transformData then drops is written by nobody and reported by nobody: pushData is handed a batch that does not contain it, the task completes, and updateSync reports success. Two published models with no versions stayed stale in models_v9 through a 280k-id bulk repair and a targeted re-enqueue because of it (868m6jk7w). Count the ids a targeted batch asked for against the documents its transform produced, carry the difference to the queue, and return it as droppedIds / droppedIdSample alongside failedIds. update(), updateSync() and processQueues() each log a line naming the count and a sample when it is nonzero. An unreadable transform output reports nothing rather than everything - a false alarm on every batch would be worse than no alarm - and a processor that handles an id without writing a document opts in with getHandledIds, which collections does for the ids it prunes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(search-index): account for pull-side drops, surface the count to the caller Review round over the first commit. Five changes, each from a finding: - A targeted pull that comes back falsy at step 0 now reports its ids. users, comics, tools and metrics-images return null from pullData on an empty batch, and the base treats that as done before a transform task exists - so the worst case, every requested id producing nothing, was the one case reporting zero. Step 0 only: a falsy return at a later step can mean the step sequence ran out rather than no rows, and that is not provably a drop from here. - The mod reindex endpoint returns the numbers instead of a bare ok. It was the only production reader of the result and the surface the incident ran through. - The drop line is console.log, not console.error. The Update queue legitimately carries ids the index filters out - model-scan-result queues an Update for every scanned Draft model - so an error-level line would be nonzero by design on every cron run, which is the alarm-nobody-believes this is meant to avoid. - handledWithoutDocument is reported rather than subtracted into silence. A getHandledIds hook is the one way an id stops being counted as dropped, so that number is the only thing that would show a processor pruning wrongly. - updateSync dedupes its ids, and the accounting can no longer fail a batch: a throw from a processor-supplied hook would have retried and then reported the batch as failed, an observation destroying the write it exists to watch. Tests: the object-of-arrays case now uses divergent id sets, the collections hook is asserted as the processor's own rather than a copy of it, and the log line, the sample cap, the retried push and the dedupe each get a case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(search-index): dedupe before chunking, and pin the contract the endpoint changed Round 2 of review. The one that mattered: the endpoint change broke an existing test that no control could see. src/__tests__/pages/api/mod/update-index.test.ts asserted the success body was exactly { status: 'ok' } - a pinned decision this PR reverses on purpose - and all eight controls ran against the two search-index files, so the window never covered the third hunk. The case is renamed for the new contract and asserts the fields rather than tolerating them. - updateSync deduped BEFORE chunking, per action. Deduping per chunk left a repeated id counted once per chunk it landed in, so the guarantee held only for callers whose duplicates happened to be adjacent. Per action because the same id may legitimately arrive once as Update and once as Delete. - The 500 body carries handledWithoutDocument too, so a caller does not branch on the status code to find out how many ids produced nothing. - The drop log builds its parts, so a prune-only run reads "3 handled without a document" instead of "0 ids produced no document (sample: ); 3 handled...". - The step-0 comment named images and metrics-images as the exposure. images is retired - it reports nothing because it does nothing. The four processors that actually return null at step 0 are users, comics, tools, metrics-images. - droppedIdCount's "true total" doc carries its one exception: a batch whose accounting threw contributes 0 and says so at console.error. Tests: a cross-chunk dedupe case, a hook over an unreadable shape, a prune-only log case, an empty-log-line guard, and the accounting-throw case now pins the pushed payload, the fail-open count and the console.error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(search-index): state the action taxonomy once, and test the delete path Round 3 of review. - The dedupe key and both updateSync filters read one `actionOf` helper. Stated separately, a third action added to the enum would get its own dedupe bucket while matching neither filter, and would vanish from the run silently. The filter half of that predates this PR; the dedupe made it a second site. - Two cases the round-3 diff had no coverage for: dedupe is per ACTION, so an Update and a Delete for one id do not collapse into one - a lost deletion is worse than the miscount the dedupe fixes - and the two Deletes still collapse into a single cleanup call. That is also the first coverage the delete branch of updateSync has had. - The joined log line, asserted on the one case where both halves are nonzero. A log emitting the handled part only when nothing dropped passed every case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(search-index): pin actionOf where it disagrees with identity One derivation behind the dedupe key and both filters is the right shape, but it moves all three together: a mutation inside actionOf leaves a test that reaches the taxonomy only through updateSync passing. The per-action case now carries a bare id - the one input where `?? Update` is not identity - so the assertion prints [ 9 ] instead of [ 9, 10 ] when the normalisation goes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(search-index): name the number for what it counts, not for loss droppedIds implies loss. The number includes rows the index legitimately does not want - model-scan-result queues an Update for every scanned Draft model and the models pull filters to Published - so it is a population count, and an operator reading "droppedIds: 47" in a log at 2am concludes 47 repairs failed. A paragraph in the PR body does not reach that reader; the noun does. droppedIds -> idsWithoutDocument, droppedIdSample -> idsWithoutDocumentSample, droppedIdCount -> idsWithoutDocumentCount, and the comments move with them. It pairs with the existing handledWithoutDocument, so the two numbers read as ids with no document and the subset a processor claims to have handled anyway. The pre-existing droppedIds names in feed-image-existence-check.metrics.ts, auction.service.ts and image.service.ts are other subsystems' and untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(search-index): stop the actionOf comment claiming a protection it does not give The docblock said a third enum action would, without the helper, get its own dedupe bucket while matching neither filter and vanish from the run. Stated together it still would: a third member returns itself from actionOf, matches neither filter, and vanishes exactly as before. The helper collapses the DEFAULTING rule, not taxonomy coverage, and the only person who ever reads that comment is someone adding a third action - it was telling them they were covered. Also retires the old noun from the spec surface: the file, its describe and the fixture constant. Keeping "drop" for the event and idsWithoutDocument for the state is the choice; a filename nobody can grep for the behaviour is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(search-index): one spelling of the accounting triple, and two truthful comments - The endpoint spelled the three accounting fields into both response bodies verbatim; a fourth field on the result would have landed in one only. - actionOf's docblock said "one statement" of the no-action-means-Update rule. Repo-wide that is false: SearchIndexUpdate.queueUpdate states it the other way (strict equality, no defaulting), so an item queued with no action there reaches neither bucket. The comment now scopes its claim to updateSync and names the other statement, because the reader who trusts it is the one calling queueUpdate([{ id }]) and getting silence. - handledWithoutDocument's doc names what collections calls the same set, disqualifiedIds, so an operator chasing a nonzero number in a response body knows what to grep for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * style(search-index): format the renamed spec CI treats a git-mv'd file as an added file and gates added files on prettier. The rename edits landed after the last prettier run, so the new path was never formatted at its new name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 10:41:55 -06:00
Zachary Lowden	32121182a3	refactor(blocks): remove the dead platform-funded spend bounty rail (#4932) ... * refactor(blocks): remove the dead platform-funded spend bounty rail The percentage author bounty on block-initiated generation spend never paid anything and structurally could not: the share was floor(gross_cents * pct / 100) computed per row, and a block generation grosses well under 20 cents, so at 5% every row floored to zero. It has been superseded by the additive, author-set, viewer-paid per-generation author fee (#4922). Removed: - the SPEND leg of backpayTrackedAttributions (the whole function had no production caller, and was double-dark behind a Flipt flag that does not exist plus SIGNED_OFF_RATE_CARD_VERSION === null) - computeSpendShare + SpendShareResult in rate-card.ts, whose only non-test call site was that spend leg - app-bounty-cap.service.ts entirely (reserveAppBountyAccrual / refundAppBountyAccrual), plus the dynamic import + await it added to every spend-attribution write, where its own comment said it was a guaranteed no-op because the reserved amount is hardcoded 0 - REDIS_SYS_KEYS.BLOCKS.BOUNTY_CAP, which only that module used Deliberately kept: - the MEMBERSHIP leg. backpayTrackedAttributions still reads block_subscription_attribution and still calls computeSubscriptionShare; flow C is not superseded and its rate is an open leadership decision. The Sybil per-app cap keeps its coverage, re-expressed over subscription rows. - RateCard.spendSharePct and every published card's value. Cards are immutable snapshots and rows stamp a version; the field is documented as retired rather than removed. - the spend attribution write path's shape. block_spend_attribution rows are still written with spend_share_pct = 0, app_owner_share_cents = 0 and rate_card_version = 'unrated'; no column is dropped and no migration is added. What this costs: 52 status='tracked' spend rows from 9 distinct non-operator users become permanently unpayable. That is acceptable only because the removed formula pays $0.00 on every one of them at any rate below 100% — the loss is nominal, not real. Also drops the half of block-spend-attribution-status-default.test.ts that pinned the schema default against the status the SPEND backpay read selected. That read no longer exists, so the guard failed closed rather than passing vacuously; the schema-vs-migrations half is unchanged. Comments naming the removed bounty across blocks.router.ts, buzz-helpers.ts, app-spend-cap.service.ts and RATE_CARD_V4 are corrected in the same pass. * docs(blocks): re-point the dev-token spend gate and correct the rail's stale claims Round-0 audit follow-ups on the spend-bounty removal. Comment-only: `git diff` over this commit contains zero non-comment changed lines (checked mechanically, with a positive control — the same filter over the parent commit reports 883). 1. RE-POINT A SECURITY GATE RATHER THAN DROP IT (dev-token.ts, both sites). The APPID MISATTRIBUTION block ended with "GATE: before #2605 turns on a non-zero spendSharePct, re-confirm no pending-path mint can ever land a real OauthClient.id in appId". After the parent commit nothing reads a rate card's spendSharePct, so that trigger can never occur and the gate was unclosable. The hazard moved rather than went away, onto two surfaces that are CLOSER than the dormant ledger they replaced, and the comment now names both: a live reader today (app-analytics.service.ts aggregates block_spend_attribution for the app-owner dashboard), and the author fee (recordSpendAttribution drives observeBlockAuthorFee, #4922 slice 1). Verified, not assumed: the call path (buzz-attribution.service.ts:19 import, called at :686, the only non-test caller) and the flag state (app-blocks-author-fee-enabled — plain global boolean, base enabled: false, no segment, no rollouts, read from the flag store's authoritative revision). CORRECTION TO THE AUDIT'S FRAMING: slice 1 is observe-only and carries NO recipient — it is keyed on baseGenerationBuzz / priceIsCap / generationType alone — so appId is not load-bearing for it and a forged row cannot misdirect a fee today, flag on or off. The flag's state is therefore deliberately NOT the trigger. The re-pointed trigger is the SETTLEMENT slice landing a recipient derived from this app resolution, and the gate names what re-confirms it: the existing S1 case in src/tests/api/v1/blocks/dev-token.test.ts. That assertion is the pin — the gate points at a running test, not at prose. 2. RETIREMENT NOTE AT THE ACTIVE RATE CARD'S DECLARATION SITE (rate-card.ts). Only V4's doc block and the type-level doc were annotated; RATE_CARD_V5 — the ACTIVE card — still read "spendSharePct: 5, // Carried from V4 verbatim". An engineer authoring a V6 edits that line, not the type. The immutability defence does not bind: recordSpendAttribution hardcodes rateCardVersion = UNRATED_RATE_CARD_VERSION, so spend rows stamp 'unrated'. Recorded honestly as "retained without a KNOWN historical referent" rather than as an absolute, because of one caveat the audit did not have: the pre-track-only path in #2627 DID stamp share.rateCardVersion, and #2635 retrofitted it the SAME DAY (2026-06-18); that retrofit's migration records contemporaneously that prod held 0 spend rows. Established from code history, not re-confirmed against the database. The field is NOT removed. The type-level doc's "rows stamp a card version" justification is corrected too. 3. THE KEPT MODULE NAMED A DISBURSER THAT DOES NOT EXIST (backpay.service.ts). It claimed "A SEPARATE payout rail (PR #2605) later disburses confirmed rows". #2605's full diff aggregates and flips blockBuzzAttribution ONLY — the PURCHASE table — with zero references to block_subscription_attribution or block_spend_attribution. The earlier pass checked #2605 against the DELETED code, correctly, and never against the KEPT code's own claim. No replacement disburser is invented: confirmed is a terminal state in practice, and whoever builds one owns re-pointing that paragraph. Three sibling claims of the same class, found by enumerating every #2605 reference in the tree, corrected the same way: buzz-helpers.ts, blocks.router.ts and the S1 test's own comment. isPayoutEligibleBuzz is still load-bearing, just not where those comments said — it narrows the realized debit to its PAID portion BEFORE that becomes grossValueCents, so free Buzz never enters the money basis at all, which is a stronger placement than a payout-time gate. 4. STALE BOUNTY COMMENTS the parent commit claimed were "corrected in the same pass": blocks.router.ts (the author-bounty spend basis note, plus three more "bounty" phrasings), schema.full.prisma, dev-token.ts, block-workflows.service.ts, and two in buzz-attribution.service.ts (UNRATED_RATE_CARD_VERSION's "share-pending" doc, and "awaiting the payout-time backpay" on the spend write). The audit was half wrong about the Prisma one: it reported both halves false and the named guard deleted. block-spend-attribution-status-default.test.ts STILL EXISTS AND PASSES — what was dropped is its second assertion. The schema-vs-migrations half is real, still asserted, and is now the whole of what that comment claims. BOUNDARY RATIONALE VERIFIED by enumerating every non-test reference per table: flow A has a live independent reader (app-analytics.service.ts:408), so removing the bounty orphans nothing; flow C has a live writer (stripe.service.ts:1091, the paid-invoice path) whose only reader is the backpay. TESTS. Covering set 681/681 passed over 9 files (the parent's 6 plus dev-token, author-fee and no-divergent-author-fee-base). Prettier clean. Full unit suite on this host: 42,109 passed / 29 failed / 33 skipped over 1,876 files. 🔴 That is 26 more failures than the parent commit recorded, and NONE are from this change — established by control, not by argument: all 9 edits were reverted and the three affected files re-run at pristine HEAD, which reproduced 26 failed / 3 files identically. They are a host artifact (Node v26.8.1 — localStorage is undefined, so zustand's persist middleware throws "Cannot read properties of undefined (reading 'setItem')"; plus a @prisma/client loadLibrary failure), and they fail instantly (8-16ms), which rules out load. The remaining 3 are the parent's known flakes — 2 in eventloop-watchdog.capture and 1 in moderation.instrumentation, not 3 in eventloop-watchdog as recorded. NOT DONE, deliberately: the gate's premise "no production code reads a rate card's spendSharePct" is true and mechanically checkable, but a no-*.test.ts guard forces edits to package.json's test:lint-rules plus literal count-and-name edits in CLAUDE.md and .claude/agents/civitai-test-review.md (pinned by no-lint-rules-script-drift) — a five-file change about the deletion rather than about the gate's instruction. Filed in the PR body with a closing condition. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(blocks): upgrade the spendSharePct retirement note to a measured claim Round-1 audit follow-ups on the spend-bounty removal. Round 1 returned no red findings and a safe-to-merge verdict; every item below is yellow or green, and every one is the same class this PR exists to fix — a stale claim about the removed rail. NOT comment-only, by exactly one line. `git diff -U0` over this change contains ONE non-comment changed line: the deleted `expect(mockDbRead).not .toHaveProperty('blockSpendAttribution')` in backpay.service.test.ts (item 5). Checked mechanically with a positive control — the same filter over the parent commit's parent reports 883, independently reproducing the figure that commit recorded for itself. 1. THE RETIREMENT NOTE IS NOW MEASURED, NOT INFERRED (rate-card.ts, both the RATE_CARD_V5 note and the type-level doc). It said the field is retained "without a KNOWN historical referent", established from code history plus a contemporaneous migration note and explicitly NOT re-confirmed against the database. It has now been re-confirmed: a GROUP BY rate_card_version over the whole of block_spend_attribution in production on 2026-09-18 returned ONE row — 'unrated', count 601. The grouping is self-discriminating, so a second stamped version would have come back as a second row; none did. Stated at the scope actually measured: that is a fact about that table at that moment, not a guarantee about the future. The note says so, and says it holds only while recordSpendAttribution stays the sole writer hardcoding the sentinel. 2. THE SCHEMA'S MODEL DOC STILL DECLARED THE TABLE TO BE THE REMOVED RAIL (schema.full.prisma). The model-level doc for BlockSpendAttribution opened "W3 flow A — App Blocks buzz SPEND attribution (author bounty)" and carried an ACCOUNTING paragraph asserting the author share is "a PLATFORM-FUNDED BOUNTY paid on top of the spend, sized as a percentage of the spend's USD value" — present tense, for a rail this PR deletes. This PR had already edited that file (the `status` comment ~100 lines below), so a reader met a freshly corrected comment under a stale header and concluded the header was current. Rewritten to lead with TRACK-ONLY, name what the rows actually carry, name the live consumer (app-analytics.service.ts, reporting — not payout), and keep the platform-funded accounting only as explicitly historical context for why the CHECK constraints are asymmetric with the purchase table. Two field docs fixed with it: rate_card_version (always the sentinel) and spend_share_pct, whose doc read "the spend rev-share percentage stamped at write time" for a column the write path hardcodes to 0. 3. THE FUNCTION'S CONTRACT LINE CONTRADICTED ITS OWN DOC BLOCK (buzz-attribution.service.ts). recordSpendAttribution's JSDoc opened "Record an author bounty for a block-initiated generation" while line 484 of the same block, added by this PR, said the percentage author bounty is GONE. The PR rewrote the body and left the first sentence — the line most readers take as the contract. Its section banner at :269 said the same thing. 4. THE RE-POINTED GATE COULD BE SATISFIED BY ACCIDENT (author-fee.ts). Round 1 verified the dev-token gate's reasoning and its mechanical closing condition. The gap was DELIVERY: its trigger is "the settlement PR's author must have read this block", and nothing on the slice-2 surface pointed back at it — `git grep 'dev-token' -- src/server/services/blocks/` returns zero hits in author-fee.ts, and none of its eight `slice 2` markers mention appId or the app resolution. Slice 2 will be authored here plus a settlement service; its author has no reason to open a dev-token mint handler. Added a back-pointer beside the SLICE 1 IS DARK banner, where settlement is first named. Observability only — the underlying hazard is already closed in code (the pending path mints pending-pubreq_<ULID>). 5. THE VACUOUS ASSERTION (backpay.service.test.ts). `expect(mockDbRead).not .toHaveProperty('blockSpendAttribution')` asserted a property of the test's own hoisted mock literal, declared 200 lines above. It could only fail if someone edited that literal, so it said nothing about the service while reading as coverage of it. DELETED rather than restated — the real guard is the implicit one round 1 mutation-tested and found genuine (the mock exposes only the subscription delegate, so a reinstated spend read throws on an undefined delegate, and reaching a resolved summary at all is the proof). The comment now names that mechanism explicitly, and records why the assertion went rather than leaving a silent deletion. 6. GREEN: the "accrue the author bounty off the REALIZED debit" note in blocks.router.ts, twelve lines above a hunk this PR rewrote. The realized-vs-estimate reasoning survives the removal on its own terms — the row is the durable money BASIS — so it is re-derived rather than deleted. RE-SWEEP. The earlier enumeration used grep '#2605' and grep 'bounty' over src/, which can see neither a .prisma model doc nor anything under packages/. Re-run over every tracked file for spendSharePct|spend_share_pct|block_spend_attribution |BlockSpendAttribution|platform-funded|#2605. Beyond the items above it found three stale present-tense comments in blocks.router.workflow.test.ts ("the PAID portion must accrue the author bounty", "the author bounty must accrue off what the user NET paid", "no author bounty accrues") — corrected to name the recorded money basis, which is what those cases actually pin. Deliberately NOT changed: the migration .sql files, which are the historical record of what each migration did when authored; the generated packages/civitai-db-schema/src/{models.ts, kysely/types.ts}, which mirror types and carry no prose claim about the rail; and prisma/schema.prisma, which is gitignored and generated. TESTS, real counts, all green, no regression against the round-1 baselines: backpay.service 16, rate-card 24, spend-attribution.service 40 (80/80 over 3 files); dev-token 108 + no-divergent-author-fee-base 6 (114/114); the router workflow suite + block-spend-attribution-status-default (427/427); and the whole civitai-db-schema package, which owns the prisma-schema parser and the drift gate, 423/423 over 17 files. Prettier clean on all six TS files (schema.full .prisma has no prettier parser configured — pre-existing, the parent commit edited it the same way). A comment change has NO killing mutant, and none is claimed. The one behavioural delta is a deleted assertion that was already proven to assert nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(db-schema): regenerate kysely types for the BlockSpendAttribution field docs CI caught a real gap: lint.yml step "Generated DB schema files match their generator" (pnpm run db:check-generated) failed on the previous commit, which also skipped "Package unit tests" and therefore tripped the "Assert every package suite actually ran" ledger. One root cause, two red steps. FIELD-level /// docs in schema.full.prisma propagate into the generated packages/civitai-db-schema/src/kysely/types.ts; MODEL-level /// docs do not. The earlier commits in this PR edited only the model doc, so the gate never fired and their precedent read as "doc edits here need no regeneration". That inference was wrong the moment this PR added docs to rate_card_version and spend_share_pct. Regenerated with `pnpm run db:generate` inside the flake dev shell (prisma engines are unavailable to a bare npx on NixOS). Only kysely/types.ts moved; models.ts and enums.ts are unchanged. The diff is comment-only — the two propagated doc blocks, no type or field change. Verified: civitai-db-schema package suite 423/423 over 17 files; prettier clean; a fresh db:generate now leaves the tree clean, which is exactly what db:check-generated asserts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(blocks): genericise the prod measurement and fix five stale bounty claims Round-2 audit follow-up. Comments and test names only; zero executable delta (verified by masking comments + it() descriptions and re-diffing HEAD vs index, with a positive control at 8fe669313f showing 37 and 4 masked-differing lines). 1. The published row count is removed from every copy of the production measurement (rate-card.ts twice, schema.full.prisma model- and field-level, and the kysely types the field doc regenerates into). This repo is public and a raw row count is a product-usage figure. Everything that makes the claim auditable is kept: that it was MEASURED not inferred, the date, the GROUP BY rate_card_version query shape, why the grouping is self-discriminating, and the re-measure condition. The number is recoverable by re-running the query. 2. The schema's WHAT THE ROWS CARRY NOW block asserted status = 'tracked' unconditionally, which is false for a reachable class: recordSpendAttribution writes 'voided' on self-spend and internal-owner apps. That distinction is deliberately live, and app-analytics.service.ts applies no status filter, so a reader trusting the old sentence could treat the column as inert. The block now names the void case and points at the service's own doc rather than restating it - a restatement is what went stale. 3. Five stale claims the previous sweep missed because its alternation had no bounty term, plus one more the corrected sweep found at line 6355: two test NAMES (what CI prints), two describe-level notes and two inline comments in blocks.router.workflow.test.ts, re-pointed from "the bounty accrues" to the recorded money basis / payout eligibility. 4. blocks.router.ts:6036 said the bounty accrues off what the user NET paid - the same claim the test file already had corrected 45 lines from a block this PR rewrote. Same handler, same commit, now both halves done. 5. The note this PR introduced at blocks.router.workflow.test.ts:2876 said the FREE portion "must never be" recorded as the money basis. Its own sibling test at the BLUE-ONLY case refutes that: with no paid debit the handler records buzzType 'blue' and the realized cost. What blue never is, is payout-eligible. Corrected to say that. Regenerated the kysely types; db:check-generated exits 0, and its negative control (a doctored index copy) exits 1. Suites re-run, all matching the round-2 baselines: blocks services 80/80, router-workflow + status-default 427/427, dev-token 108/108, db-schema 423/423. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 11:37:26 -05:00
Justin Maier	22ec53d8dc	test(blocks): make the seam guard's controls constrain what the ledger consumes (#4934) ... * test(blocks): make the seam detector control constrain the predicate the ledger uses The control asserted LOGIC_MODULE_IMPORT directly, so the union in isCallSite could be narrowed to an intersection with every case still green — both real consumers satisfy both halves. Route the synthetic sources through isCallSite via a seeded key deleted in a finally, and add the symbol-only arm so the union is pinned from both sides. Second control for the comment strip, which runs at load and so was observable by nothing: at least one file must name the symbol raw and not after stripping. It pins that the strip is applied, not that it changes a verdict — no file's prose spells a call or an import today, so dropping it is fragility rather than a bypass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(blocks): close the walk-scope and call-position holes in the seam guard Review found two more mutants that stayed green. Narrowing the walk to src/server keeps every assertion green — `> 500` reads like a scope check and is not one, src/server alone clears it five times over — while a consumer added under components/ or pages/ becomes invisible to the ledger. And the only symbol-half arm was namespace-qualified, so dropping the `(` degraded the detector from "calls it" to "mentions it" with nothing red. Adds the trees the walk must reach, a bare-call arm, a negative that varies only the symbol half, and a throw if a real file ever occupies the synthetic rel — the one path where the seam could evict a consumer instead of reporting one. Drops the parallel RAW map for an on-demand read, matching the two guards that already do this, so nothing has to keep two key sets in step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(blocks): check the walk's coverage, not its reach Review found the scope assertions this round added were the same category of proxy they replaced: `some(rel => rel.startsWith(tree))` is satisfied by one surviving file, so dropping `x` from the extension filter (1,720 files, all of components/ and pages/) or skipping a single directory (340 under pages/api, where the block REST routes live) stayed green. Enumerate the expected side independently of the walk instead, and name the files any narrowing loses. Also pins which comment kinds the strip removes — the count was satisfiable by a block-comment survivor, so a stripper that stopped handling `//` was green — and adds an explicit-extension import arm. `components/ActionIconInput.tsx` is a directory, so the enumeration filters on dirents rather than on the extension alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(blocks): stop the guard's two sides sharing a rule, and pin the pre-filter Three green mutants from review, one of them introduced by the previous round. The containment check shared `isTestPath` with the walk it grades, so widening that predicate moved both sides together and hid whole trees — the same loss the round before it closed, relocated into the shared helper. The previous round also REPLACED the per-tree loop rather than adding beside it, and that loop caught this. Spell the test-path rule out again on the expected side, assert set equality rather than containment, floor the expected side, and keep the loop. The raw pre-filter decides what the detector is ever asked about, and `verdictFor` writes past it, so nothing constrained it: dropping its module half made a re-exporting barrel invisible with everything green. Named and pinned. The comment-kind fixtures started at column 0, so a line-start-anchored stripper — the regression that module's own header records — passed them. Indented, plus a trailing-comment arm. `walk` uses dirents rather than statSync per entry: 258 ms vs 1,699 ms over the same 6,506 files, measured, which more than pays for the second enumeration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(blocks): pin every spelling the pre-filter must admit, and the rules it shares The pre-filter control had one module-half fixture, so narrowing the predicate to that exact literal stayed green while a relative-path re-exporting barrel — the likely spelling for one sitting beside the module — never entered the corpus. Four spellings now, matching the discipline the detector control has. Three more, each closing something nothing observed: `isTestPath` was spelled twice and asserted zero times, so this branch's own `(^|/)tests/` anchor fix changed both copies together and the equality was green across it; `toRel` feeds both sides of that equality, so a normalisation that mangled every rel identically was invisible; and every stripper arm was a `not.toContain`, which a stripper returning '' satisfies. Folds the orphaned docblock this branch left above `couldBeCallSite` into it, and takes the review's comment trims. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(blocks): ask the detector about a rel outside the blocks directory `verdictFor` hardcoded one synthetic rel under server/services/blocks/, and every real corpus file lives under server/ too — so scoping `isCallSite` by path prefix stayed green while a consumer added under components/ or pages/ would never join the ledger. The rel is an input to the predicate; now it is varied. Also takes the review's comment fixes: the pre-filter docblock no longer restates the header's overstated absolute, a miscount and a misbound reference are corrected, and the four barrel fixtures no longer claim a discrimination a substring predicate cannot make. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(blocks): pin that 'contests/' is not a test path Both copies of the test-path rule spell the same anchor, so mutating both to a bare `tests/` substring — the natural tidy-up — is green: the equality sees matching sides and the fixtures carry no path that discriminates. Two real route files under pages/moderator/contests/ would leave the corpus. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(blocks): derive the corpus independently, and vary the rel on an axis that discriminates Both rel fixtures added last round contained the token `blocks` — as does every real corpus member — so a substring scope (`/blocks/i`) on either the detector or the corpus loop stayed green. The fixtures now use rels carrying no such token, and neither names a real file, so a future consumer at that path cannot turn the occupancy guard into a fixture-hygiene error in an unrelated test. The corpus loop was reachable by no control at all: `verdictFor` writes past it by construction. Re-derive the expected corpus from the same walk and compare, so any filter the loop grows shows up as a set difference. Adds a fixture for the occupancy throw, which was itself unexercised — and is what catches a `verdictFor` that takes the rel and ignores it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(blocks): keep the pages/ rel axis, which the last round traded away Swapping the `pages/api/v1/blocks/images.ts` fixture for a `server/services/` one fixed the `blocks`-token axis and dropped the tree axis with it: scoping the detector to exclude `pages/` was red before that commit and green after. Both axes are independent and both are needed. Third fixture restored on the new convention — synthetic filename, no token, no real file. Also drops a path round-trip in the corpus derivation and points its comment at the two upstream cases that make sharing both inputs safe, since deleting either would make it vacuous. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 10:09:24 -06:00
Briant Diehl	83eee12058	Merge pull request #4869 from civitai/yue2-generator ... Add YuE2 music generation	2026-09-18 10:06:09 -06:00
Justin Maier	d4dc1058e6	fix(notifications): bust the unread-count cache for the rows cleanup deletes (#4937) ... * fix(notifications): bust the unread-count cache for rows cleanup deletes cleanupNotifications batch-deleted UserNotification rows without touching the per-user unread-count cache, so a badge kept reporting notifications whose rows were already gone — "Updates 2" over a list saying "All caught up" — until a mark-read or the one-week TTL cleared it. The delete now RETURNs "userId", viewed and busts the count cache for the users whose deleted row was unread. Read rows are skipped: the cached hash only holds unread counts, so deleting a read row cannot make it wrong (~35% of a batch on prod data). bustUser rather than decrementUser so the next count re-derives from the DB instead of drifting. The lag window is flagged before each bust, as markReadImpl does, so a count landing before the replica catches up cannot cache the not-yet-deleted rows for another week. Measured on the prod replica: a 10k-row batch carries ~8.4k distinct users, ~5.5k of them with an unread row, so the bust side costs ~0.55 Redis DELs per deleted row. Busts are deduped within a batch only — not across the sweep, because a user who reads their bell mid-sweep re-populates the cache from rows a later batch then deletes, and a sweep-wide dedupe would skip that second bust and re-create this bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(notifications): say what the lag flag actually closes, and count the busts that landed Review findings on the previous commit. The comment claimed the lag flag stops a count from caching the pre-delete number. It does not: a count that has already picked the replica pool is unaffected, and its setUser still lands after the bust. The flag narrows the window to counts that start after it — the same exposure markReadImpl carries — so the comment now says that instead of claiming closure. The sweep log reported users attempted rather than keys dropped, so a redis outage that dropped none would still log a full sweep. It now counts the busts that resolved. Tests: the redis-failure case rejected only bustUser, so the swallow on the lag flag — the call that fails FIRST in an outage, and would otherwise abort the whole sweep — was untested. It now rejects both and asserts the later user is still busted. Added a batch wider than CLEANUP_BUST_CONCURRENCY so the worker pool's cursor takes its second iteration in a test rather than never, and an assertion on the logged bust count. beforeEach resets the mocks instead of clearing them: a mockRejectedValueOnce survives mockClear and would poison the next test's first bust. Also dropped the undated prod measurements from the comments (they belong in the PR body, where they cannot rot silently) and stopped defending resp.rows against an undefined node-pg never returns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(notifications): say what the sweep's bust count actually counts, and pin the concurrency cap Second review round on the same change. The log field claimed to count keys dropped. It counts DELs redis acknowledged: bustUser discards the reply, redis acks a DEL for an absent key, and most swept users have no cached count at all — so the number is acks, and it is now named bustsAcked and says so. It still does the job it was added for, which is to stop a redis outage reporting a fully-busted sweep. The lag paragraph asserted the primary-read narrowing unconditionally and then mentioned four lines later that the flag no-ops when REPLICATION_LAG_DELAY is unset. Those cannot both describe production: with the tracker disabled the narrowing is zero and the bust is the only thing working. Said so, and pointed at L5 in docs/plans/notifications-review-action-items.md, which owns deciding that value. Tests: added one that counts busts in flight and asserts the peak is exactly CLEANUP_BUST_CONCURRENCY. Nothing else in the file could see the cap — replace the pool with an unbounded Promise.all and every other assertion still passed, while the cap is what stands between one batch and thousands of simultaneous redis round-trips. Sorted the ids in the redis-rejection assertion, which depended on worker continuation order. Corrected the mock comment, which described a protection the bare vi.fn() stubs do not provide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(notifications): close nine mutants the cleanup suite let through Round 3 asked two lanes for mutants that go GREEN rather than for prose. They produced nine distinct ones and all nine passed the suite as it stood. None of them is a hypothetical: the SQL predicate, the RETURNING list, the dedupe order and the cross-batch accumulation were all unasserted, so the tests verified the bust machinery thoroughly and the delete itself not at all. Closed, each verified by applying the mutant and watching a named assertion fail: - The sweep could delete the NEWEST rows (`"createdAt" >`), or none at all (`LIMIT 0`), and every test passed — the fake pool answers any query with the rows the test programmed. Now pins the predicate and the limit. - `RETURNING "userId", viewed IS FALSE AS viewed` CONTAINS the old toContain string, and inverts the filter so cleanup busts the read users and nobody else — the original bug, restored, green. The guard is anchored now. - Deduping before the unread filter drops any user whose first returned row is read. DELETE ... RETURNING yields rows in physical order, so that was a coin flip per affected user. The fixture now puts the read row first. - `bustsAcked` accumulates across batches; one busting batch could not tell `=` from `+=`. The log assertion now spans two batches. - Flagging `userIds[0]` for the whole batch left every other user without the primary-read narrowing; the ordering test has one user, so it could not see "each user". The wide test now pins the flag's argument per user. - The concurrency cap is per sweep and covers both round-trips. Hoisting the lag flags out of the pool, or dropping the await on the bust pass so batches overlap, both left the DEL half at 25 and passed. The peak test now counts both calls across two batches. - A bust pass skipped after the first full batch passed everything, because no fixture came near CLEANUP_BATCH_SIZE. It is exported for test visibility and there is a full-batch test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(notifications): pin the whole DELETE, not three fragments of it Round 4 produced six more green mutants, three of them created by round 3's own closures — which is the argument for the change this commit makes. The fragment guards were each substring-blind in a different direction. `LIMIT 10000` passed `LIMIT 100000`. `"createdAt" < $1` passed `"createdAt" < $1 AND viewed`, a sweep that deletes only READ rows and busts nobody while reporting a healthy `deleted`. Neither guard saw the table name, so the subquery could be aimed at "Notification" and delete UserNotification rows by id collision. And the limit guard interpolated CLEANUP_BATCH_SIZE, so both sides moved together and the constant could be set to 1 — a sweep that cannot finish inside the client's timeout — with every assertion still agreeing. So the statement is now asserted whole, by equality, with every literal spelled out. That is a golden string rather than a behavioural check, and the comment says so: the fake pool executes no SQL, so this is the only thing between the suite and a cleanup aimed at the wrong rows. Two behavioural closures beside it: - The full-batch fixture now carries CLEANUP_BATCH_SIZE DISTINCT users and asserts the bust count. All-one-user dedupes to a single bust, which let a truncated bust list — `userIds.splice(1000)`, the shape of a plausible per-batch cap — pass while stranding ~88% of a real batch. - A batch wider than the pool with EVERY bust rejecting. The narrow fixtures gave each user its own worker, so no user was ever reached after a failure; a worker that gave up on error retired and the users past the 25th were never attempted. Also pinned: the lag flag repeats across batches for the same user. Its TTL is REPLICATION_LAG_DELAY seconds and a sweep runs for minutes, so a sweep-wide "already flagged" cache would leave every later batch of that user's rows reading the replica. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(notifications): pin two decisions a later reader would correctly undo Both are things a reviewer raised as tempting to "fix", and neither is recoverable from the code. The peak test hardcodes 25 while its title names CLEANUP_BUST_CONCURRENCY, which reads as an inconsistency — but asserting toBe(CLEANUP_BUST_CONCURRENCY) would agree with the source at every width, including no cap at all. Same shape as the LIMIT guard that let a tenfold batch size through: a guard must not read its expected value from the thing under test. And the two fullness tests look redundant. They are not: one covers a gate that stops on a FULL batch, the other a gate that stops on a short one, and deleting either opens its half. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(notifications): let the fake database fail, and tell the two pools apart Round 5 went after surface rather than spelling, and found the seams the SQL assertion cannot reach: the fake pool could not fail, and it could not be told apart from the read pool. - `notifDbWrite()` -> `notifDbRead()` was a one-word green mutation, because the mock returned the SAME object for both. In an environment with NOTIFICATION_DB_REPLICA_URL set, that is every batch failing with "cannot execute DELETE in a read-only transaction" — and it is a silent no-op anywhere without a replica, so it works wherever you would test it and dies where it matters. The mock now gives the read pool its own object, which throws. - The fake could not reject, so the whole query error path was unobserved: swallowing a transient postgres error would end a sweep early, log a healthy `deleted`, and hand the admin endpoint a 200 while rows accumulated nightly. There is now a failure queue and a test that the error comes out. - Only `captured[0]` was ever asserted, so batches 2..N were unconstrained in both statement and cutoff. Now asserted across every call of a 60-batch sweep, which also puts a floor under a "runaway" pass cap — the loop's real guarantee is that it exits on an empty batch, and a cap above 60 is still invisible. - The logger can now reject, pinning the `.catch` on a call made without await: an unhandled rejection there takes the process down AFTER the deletes have happened, so the caller sees a transport failure and retries the whole sweep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 09:41:19 -06:00
Luis E. Rojas Cabrera	e2916bde43	Merge pull request #4911 from civitai/fix/include-license-fees-in-peak-earning ... fix(creator-program): include license fees in peak earning calculation	2026-09-18 11:37:16 -04:00
Luis E. Rojas Cabrera	7f3288dcde	Merge pull request #4914 from civitai/feat/add-copy-link-and-highlight-support ... feat(model-comments): add copy-link and highlight support	2026-09-18 11:36:57 -04:00
rmatif	8b1bbb0aa4	Explain YuE2 score planning controls	2026-09-18 17:31:34 +02:00
Luis E. Rojas Cabrera	0f86986a95	Merge pull request #4921 from civitai/fix/training-studio-tester-feedback ... Training Studio: first fix pass over consolidated tester feedback	2026-09-18 11:25:56 -04:00
Luis Rojas	059a0f0ee3	fix(training-studio): close the review's labeling-race and balance gaps ... Five of Copilot's six findings were real: - A missing/non-numeric balance from getBuzzAccount now returns null from the embed's getBuzzBalances instead of coercing to 0 — blue:0 asserted the whole price was non-Blue with certainty, defeating the fail-safe "up to" confirmation the unknown-balance path exists for. - sourceLabel keeps the ARRIVAL text untrimmed (zip .txt and reused captions), as the switch dialog promises. - Un-captioned reuse items become labelable only once hydration fills blobUrl — the drain runs again after hydration, restoring the pre-lazy behavior (failed hydration still degrades to the manual editor). - A drain result resolving between abort and delivery is dropped, so a mode switch can't receive an old-mode label into a freshly reset tile. - An aborted drain's finally no longer clears the replacement drain's progress counter — slot and labelRun release only under the identity guard. The sixth (getBuzzAccount "returns an array") is wrong — the router's getUserBuzzAccounts reduces rows into a {clientType: balance} record, which is exactly how useQueryBuzz consumes it (initialData[type]). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DV3Ku4Eu9qTtzd61zt19dZ	2026-09-18 11:19:24 -04:00
Justin Maier	9ec2a284ca	docs(track): correct the hasRemixOfId semantics block (#4936) ... * docs(track): correct the hasRemixOfId semantics block The paragraph told an analyst three things that are no longer true, and the field it describes is one a roll-up is taken over. `hasRemixOfId` was documented as ungated on prompt similarity. That held between #3871 and d381b4967e, which reinstated the >=0.75 gate on the branch where the prompt is the carrier. There are three definitions across history, not the two the block named, so the boundary it warns about is in the wrong place. The 'video' bullet claimed the field is absent because the video form has no similarity hook "yet", and cited VideoGenerationForm.tsx line numbers. That file contains no trackAction call and no hasRemixOfId, and nothing has emitted formVersion:'video' since cd6e161960 removed the legacy forms. Video renders through the same footer as everything else and emits the field like any other row. Left as written, an empty 'video' bucket reads as a drop in video generation rather than the end of a label — a wrong number with a plausible story attached. 'form-graph' is one of the two footers emitting the field today and the block predated it. Release numbers are written as the earliest release that could carry each change, with the text saying the real boundary is the deploy: a tag bounds the artefact, not the rollout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(track): name the form-graph cohort, drop the methodology note Review round on the block above. The roll-up guidance was incomplete in the way that produces a wrong number rather than a confused reader. 'new' and 'form-graph' are different populations, not just different forms: the form-graph lane is gated by formGraphGenerator, so a rate computed across the two buckets compares a cohort against everybody. The static ['mod'] beside the flag is only the Flipt-down fallback and Flipt overrides the role check in both directions, so the audience is stated as unknowable from here rather than guessed at. The release-versus-deploy note argued for its own methodology over two sentences. The warning a reader needs is that slicing at the tag date is early; the reasoning behind it belongs in the PR, not the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 09:10:38 -06:00
rmatif	043468bafa	Match YuE2 to Simple and Custom music modes	2026-09-18 14:08:03 +02:00
Zachary Lowden	f58826ba36	fix(blocks): record a real generationType for App Blocks pass-through step submits (step:<$type>) (#4928) ... * fix(blocks): record a real generationType for pass-through step submits Every App Blocks PASS-THROUGH submit (`kind:'step'` with a bare orchestrator `$type` instead of a registry id) wrote `block_spend_attribution.generation_type = NULL`. The pass-through submit handler never passed the field at all, and NULL is a legitimate value in that column, so nothing surfaced it: an unbackfillable hole in a fee-bearing column on the one arm whose type set is open by design. An untyped spend event can never be typed retrospectively, which is why the column exists ahead of the per-generation-type author fee. The value recorded is `step:<$type>` — NAMESPACED, not the bare `$type`. WHY THE NAMESPACE. `textToImage` and `customComfy` are THEMSELVES real keys of the live orchestrator `WorkflowStepTemplate.discriminator.mapping` (measured 2026-09-17: 50 keys; `imageGen`, `convertImage` and `chatCompletion` are in there too). A bare `$type` would stamp `{kind:'step', $type:'textToImage'}` as `generationType: 'textToImage'` — indistinguishable from a genuine `kind:'textToImage'` submit and priced as one by a fee that keys on the coarse segment. Under `step:` the coarse key of every pass-through row is `step`, so a caller cannot reach another fee group. The same reasoning covers a `$type` equal to a registry id: `step:convert-image` is not `convert-image`. THE SUBTYPE IS CALLER-INFLUENCED, AND BOUNDED BY SHAPE ONLY. This arm validates `$type` against nothing but the platform-internal denylist, and the attribution write is best-effort off an already-billed submit, so the token is the app's. It is bounded to non-empty, colon-free, length-capped ASCII by one anchored expression (three properties, one rule — a separate non-empty check would be unkillable); anything else DEGRADES to the bare `step` rather than throwing. All 50 live `$type` keys match that class, so no legitimate submit loses depth, while the wire's own `z.string().min(1).max(64)` admits `a:b` and `foo bar` — the degrade path is reachable from the wire, and a test proves it against the real schema. There is no vendored `$type` catalog in this repo, so a membership bound is not available; that is stated in the header rather than papered over. DESIGN NOTES, all recorded in code: - `step` joins `BLOCK_WORKFLOW_KIND_GENERATION_TYPES`, which is what puts it inside the existing registry-collision guard: a step registered as `step` would make a bare `step` ambiguous, and that is now a red test. - `blockGenerationSubtypesFor` becomes `blockGenerationSubtypeRule` and returns an OPEN sentinel for this one key, so the open axis cannot read as a closed empty set. `isBlockGenerationType` gains exactly one branch on it. - `BLOCK_GENERATION_TYPES` (the ledger) carries the bare `step` and NO `step:` value: the arm is infinite, so the bidirectional ledger test now states the exemption and pins it (no member starts with `step:`, and the resolver demonstrably emits accepted values the ledger does not contain) rather than being weakened into vacuity. - `PASS_THROUGH_TYPE_MAX_CHARS` is exported so the wire cap and this module's import-light copy of it are pinned equal in both directions. Red/green matrix: 12 isolated mutants, each killed by named tests, each restored and the tree checksum-verified. Suites: generation-type 76/76, blocks.router.workflow 429/429 (426 at base), blocks service+schema 4572/4572 across 187 files, test:lint-rules 593/593 across 42 files (no ratchet touched). typecheck 0 errors with a positive control proving it can go red on the changed file. eslint: 21 problems / 3 errors / 18 warnings both before and after on the router files — all pre-existing, none introduced. No schema change: the column is already TEXT with no CHECK and has no readers. * fix(blocks): close the review findings on the pass-through generationType Five parallel review lanes ran over the previous commit. This is the fix round — two real defects, four coverage holes the first mutation sweep could not see, and four stale claims the widening made false. DEFECTS 1. `composeBlockGenerationType('step', undefined)` minted `step:undefined` into the fee-bearing column. The guard was `subtype !== null`, so an off-type caller's `undefined` interpolated as the STRING "undefined" and the shape test accepted it. Harmless while every subtype axis was a closed set (`textToImage:undefined` is in none), which is precisely why the open axis made it reachable. Verified by execution, then fixed to `typeof subtype === 'string'` and pinned for both arms. 2. The router test's `expect(stamped).not.toBeNull()` — the one assertion written to name the defect — could not see it. The pre-change code OMITTED the field, so the value arrived as `undefined`, and `expect(undefined).not.toBeNull()` passes. Now `not.toBeUndefined()`. COVERAGE THE FIRST SWEEP MISSED (each mutant now dies with a named test) - No fixture contained a DIGIT, so narrowing the class to `[A-Za-z._-]` survived a fully green suite — and the next upstream `$type` carrying one (`model3DPreview`, `miniMaxMusic3`) would have silently lost its subtype. - Only the whitespace corner of the class's complement was sampled, so WIDENING it by any ordinary punctuation mark survived. Now enumerated in both directions over all 95 printable ASCII: exactly 65 admitted, 30 refused. - The arm discriminator `step === undefined` could be loosened to `!step`, `step == null` or `typeof step !== 'string'` with nothing red — each of which records `step:<$type>` for a body whose contract answer is `null`. Also pins arm PRECEDENCE (a registry body with a stray `$type` still resolves to its step id) and an OWN-key `step: undefined`, which a resolver "tidied" to `'step' in body` would send back to NULL with the router suite still green. - 🔴 THE SHARPEST ONE: narrowing the WRITE-SIDE re-check from the shape bound to the LEDGER (`BLOCK_GENERATION_TYPES.includes`) reinstates the original defect — every pass-through row back to NULL — and no test in the repo saw it, because the router tests assert what was PASSED and that line decides what is PERSISTED. The ledger deliberately contains no `step:` value, so it is the refactor the module's own prose invites. Two tests in `spend-attribution.service.test.ts` now cover it. CLAIMS THE WIDENING MADE FALSE (a comment is a claim) - `buzz-attribution.service.ts`: the log field's `// Bounded (registry-derived or null)` (flagged independently by two lanes), the param doc's "NEVER the orchestrator's internal `$type`", and the re-check's "the segment after the first colon in the closed set that key allows". - `schema.full.prisma`'s column comment — the doc the unbuilt fee's author actually reads — plus its generated mirror in `packages/civitai-db-schema/src/kysely` (`db:check-generated` passes; comment-only, no DDL, no migration). - This module's own justification for the literal copy of the 64-char cap said `workflow.schema` "pulls in zod and the whole block wire surface". Review MEASURED that false: every runtime module it imports is already in this module's graph, so importing it would add exactly one module. Corrected to the narrow true claim, including that this precedent is weaker than the one it invoked. - The test file's header claimed all pass-through assertions are red at base; three of them hold at base too (there, `step` was not a coarse key, so `step:` values were refused for a different reason). Relabelled as bounds, not regression coverage. Same for the `SP` fixture's rationale, which claimed a formatter hazard that does not exist — the real reason is edge-space legibility. ALSO: the character class is a FOURTH property beyond the three the bound was asked for, and unlike the cap it can never have a live seam test — so the header now states that a future upstream `$type` with an out-of-class character would lose depth with nothing reporting it, and a new test pins the class against the full 50-key snapshot of the live mapping (measured 2026-09-17) so the claim is at least checkable. `isBlockPassThroughSubtype` is module-private again (it had no consumer), the coarse key is pinned to the wire `kind` literal by parsing with the constant, the wire-seam test now resolves `parsed.data` rather than a hand-built twin, and the `TOTAL AND NON-THROWING` docblock is scoped to JSON-shaped input (an own accessor on a read property propagates its throw — unreachable through the wire, and true of the pre-existing reads too). VERIFICATION: 20 isolated mutants, all killed with named tests, on the shipped tree; M20 was first rejected for dying of a ReferenceError (32 of 33 red, none of them mine) and re-run as a compound mutant that carries its own import, after which exactly one test — the intended one — fails. typecheck 0 errors. eslint: the two files this round touches carry 2 pre-existing errors, identical before and after. prettier clean. test:lint-rules 593/593 over 42 files, no ratchet touched. * fix(blocks): close the DELTA-round findings (comments + two coverage holes) Round 2 of the audit ladder re-reviewed the fix round. Comment-and-test only — no behaviour changes. COVERAGE - 🔴 The non-ASCII half of the character class was sampled at ONE code point. The printable-ASCII enumeration sweeps 0x20–0x7E, so widening the class into a non-Latin RANGE (Cyrillic, CJK) or by DEL survived the whole suite green. New test with `\u`-escaped fixtures only — a zero-width space, a BOM, an NBSP and DEL are invisible in a diff and in most greps, and one of them arrived as a stray NUL the first time it was written, which made the file read as binary. Verified: the mutant that adds `Ѐ-ӿ` to the class is killed by this test ALONE — the ASCII sweep cannot see it, which is the point. - Deleted an assertion that reads as coverage and provides none: the per-character loop over `refused` is strictly implied by the set equality above it, since admitted ∪ refused partitions the 95 code points. Same shape as the reversed `toBe` deleted last round. - Deleted the router test's `not.toBeUndefined()` for the same reason (implied by the literal `toBe`), and moved the explanation of why the ORIGINAL `not.toBeNull()` was blind onto the line that actually carries the claim. CLAIMS - 🔴 `buzz-attribution.service.ts`'s log-field comment gave the WRONG REASON. Last round replaced "registry-derived" with "safe as a log field because the bound excludes newlines, control bytes, quotes and whitespace" — also false: the sink builds the line with a single `JSON.stringify` (`@civitai/axiom`'s client, read to confirm), so the field is an escaped JSON string value and could not break the line whatever the class admitted. Fixing a false claim with a different false claim is the failure mode this file's own discipline names; the comment now says what the bound really buys (bounded width, no unicode/control junk) and keeps the part that matters — not a metric label, not an object key. - 🔴 The header said an out-of-class upstream `$type` would lose depth with "NOTHING REPORTS IT". Overstated: `WHERE generation_type = 'step'` is EXACTLY that population — `composeBlockGenerationType(BLOCK_PASS_THROUGH_COARSE_TYPE, …)` is the only producer of a bare `step`, it is called from one place, and the wire guarantees `$type` is a non-empty string. No alert, one query. This matters because the PR asks the operator to ratify the character class, and it was framed against the class on a false premise. - The test file's "which of these hold at base" list was wrong a SECOND time: it named three tests when the answer is five (`a body with NEITHER a step id NOR a string $type` and `a REGISTRY body carrying a stray $type` also hold at base — the pre-change resolver never read `$type` and had only the registry arm). A central enumeration that has now been wrong twice is the wrong mechanism, so each such test is labelled `HOLDS AT BASE` at its own `it(`, the way this file's two `INVARIANT GUARD` tests already were. Grep the marker for the population. - "adding `+ ~ % @ …` (or anything else) to the class turns no other test red" — the parenthetical was false for six characters that ARE pinned elsewhere. - "in BOTH directions" survived in two docblocks describing the cap seam after the reversed assertion was deleted; both now say what actually holds (one equality plus the literal). - `spend-attribution.service.test.ts` still said the write re-checks "against the same registry-derived list" — the exact claim fixed one file over, and it contradicted a comment 16 lines above it that this PR wrote. DECLINED, with reasons - Using `BLOCK_PASS_THROUGH_COARSE_TYPE` for the resolver's `kind === 'step'` test. That comparison means "the wire kind is step", which BOTH arms share; spelling it with the pass-through COARSE-KEY constant would read as "this is the pass-through arm" and be wrong. The constant↔wire seam is pinned by parsing instead. - Pinning the SET of `recordSpendAttribution` call sites so a fifth writer cannot omit the field. Correct and valuable — and a new convention guard in `no-*.test.ts`, which this repo requires be wired into `test:lint-rules` plus the two exact-phrasing counts `no-lint-rules-script-drift` reads. That is its own PR; it is recorded in the PR body with a closing condition, and with the reviewer's sharper point that making the field REQUIRED would not close the class (the type still admits `null`). VERIFICATION: 21 isolated mutants now, all killed with named tests on the shipped tree. typecheck 0 errors. eslint 3 errors / 17 warnings over the six files, all three individually measured pre-existing at base. prettier clean. * docs(blocks): fix three label inaccuracies found by the round-3 delta audit Comments only — `git diff HEAD~1 HEAD` with comments stripped is code-identical. - 🔴 THE `HOLDS AT BASE` POPULATION IS SIX, NOT FIVE, and the unmarked one was the test the previous commit ADDED. `refuses NON-ASCII and DEL` passes on the pre-change module for the same reason the other five do: at base `step` was not a coarse key, so every `step:<anything>` was already refused. Being NEW is not the same claim as being RED AT BASE, and the previous commit replaced a twice-wrong central list with "grep HOLDS AT BASE for the population" — which made that grep wrong on its first outing. Marker added, with the reason. - The corrected parenthetical in the printable-ASCII test pointed at "the next test" for the space/tab/newline/slash fixtures, and the same commit INSERTED a test between them. Now named rather than positional, which is what a pointer in a file that keeps growing has to be. - The new DETECTOR paragraph (`WHERE generation_type = 'step'` is exactly the shape-refused population) was stated unconditionally. It is true today, and it holds only because every `recordSpendAttribution` writer passes this resolver's output — a hand-built `'step'` type-checks and the write-side re-check accepts it. That writer-set property is precisely what the DECLINED call-site ledger test would pin, so the dependency is now named where the claim is made. Verified: 3 suites green (544/544), typecheck 0 errors, prettier clean. The audit lane that raised these confirmed by execution that all nine fixtures of the new test pass at base and that six isolated non-ASCII/DEL class mutants are each killed by that test alone — so the marker is a labelling correction, not a coverage one. * docs(blocks): the collision test is an invariant guard PLUS one red-at-base line Comment/title only. The closing audit round came back CLEAN and recorded this as out of its scope — but it is a label my own first commit falsified, so it gets fixed rather than left because a reviewer scoped it out. `INVARIANT GUARD (not regression coverage): no registered step id collides with a kind key` acquired an assertion in this PR — `expect(BLOCK_WORKFLOW_KIND_GENERATION _TYPES).toContain('step')` — which is RED at base, where that tuple held only `textToImage` and `customComfy`. So the test's own comment ("it would have passed before this change too") and the file header's "Two tests are explicitly labelled INVARIANT GUARD … they would pass on either tree" were both wrong for it. The LOOP is still a true invariant guard; the added line is regression coverage. Title, body and header now separate the two, because the halves have different standing. No behaviour change; 82/82 in the covering suite; prettier clean. * test(blocks): pin the fee seam, now that the fee exists on main `main` gained `feat(blocks): compute the per-generation author fee, dark (slice 1 of 3)` (#4922) mid-review — the consumer this PR's namespace decision exists to protect. Rebased onto it (one textual conflict, in the pass-through `recordSpendAttribution` object literal: both sides ADD fields, so both were kept), and then checked the SEMANTIC interaction rather than assuming a clean rebase meant a clean merge. WHAT THE MERGED CODE DOES, read rather than guessed: `author-fee.ts`'s `resolveBlockAuthorFeeParams` bounds its key with `isBlockGenerationType`, tries the FULL value, falls back to `blockGenerationCoarseType`, and labels its counters `coarse_type` with the result. So this PR's central claim — a pass-through row must not decompose to a kind key — is now a claim about a real function, and it was asserted nowhere. A new CROSS-MODULE SEAM describe asserts it: - `step:textToImage` resolves under the `step` group and NOT the `textToImage` one, and the two get different params; - `step:chat-completion` does not inherit a FEE-FREE entry (the sharper direction — recorded bare, such a row would pay nothing); - under the PLATFORM config a pass-through row falls to the default and is labelled `step`, as does a bare degraded `step`; - an out-of-shape `step:` value gets `coarseType: null` from the fee too, i.e. no guessed group. 🔴 SCOPED HONESTLY, because the un-namespaced harm is LATENT TODAY: the fee is dark and the platform table has ONE entry (`chat-completion` → 0/0), so a bare and a namespaced value would both land on the default right now. The exposure goes live the moment any table carries a kind-key or fee-free entry — which slice 3 makes per-app and author-editable. The fixtures therefore use a config WITH such entries; a test over the single-entry platform table could not see the hazard at all. Also: two comments called the fee "(unbuilt)". It is built (dark). Both corrected, and `blockGenerationCoarseType`'s docblock now records that it HAS a production caller — which also retires the review finding that nothing routed a future fee reader through it. MERGED-TREE VERIFICATION (a clean rebase is not a clean merge): - blocks service + schema + router suites: 189 files, 5080/5080 - generation-type + author-fee + spend-attribution: 3 files, 186/186 - test:lint-rules: 43 files (the fee added one), 599/599, no ratchet touched - typecheck 0 errors — ⚠️ after `pnpm run db:generate`. Before regenerating it reported 10 errors in `user-hub.service.ts`, a file this PR does not touch: #4927 added a Prisma field and the local generated client predated the rebase. A stale instrument, not a defect, and worth knowing before someone bisects it. - db:check-generated passes.	2026-09-18 00:20:05 -05:00
Justin Maier	29e8ac13fb	refactor(creator-shop): return named pack detail fields instead of the meta column (#4929) ... * refactor(creator-shop): return named pack detail fields instead of the meta column `getPackDetail` feeds a public procedure, so it now names the four meta fields a pack page renders — cover, cover tiles, Blue Buzz opt-in and purchase count — rather than returning the shared meta blob. The pack editor's rejected-vs-archived split moves with it: the endpoint returns `lastReviewWasRejection`, derived server-side through the same `wasLastReviewARejection` helper the rule already had, so the client keeps identical behaviour without deriving it a second time. The source guard that pinned that derivation moves to the new location. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(creator-shop): share the pack display whitelist, scope the review verdict Review follow-ups on the pack detail narrowing: - `packDisplayMeta` moves from `creator-shop.service` into `creator-shop.data` and `getPackDetail` spreads it, so the pack page's meta whitelist has one definition instead of a fourth hand-written copy. The detail response gains `packMemberCount` and defaults `acceptsBlueBuzz`, matching the storefront. - `lastReviewWasRejection` is answered only for a moderator or the lister — the two viewers whose editor asks the question. Everyone else gets no answer rather than a false one. - The response fence in the new test covers the whole response and the members array, not just `meta`, and uses sentinel values that cannot collide with a price or an id. The editor guard pins the operator and matches an optional-chained read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(creator-shop): one viewer predicate for pack detail, and fence the shared whitelist Second round of review follow-ups: - The pack detail gate and the review-verdict answer were two spellings of one rule. `canReadPrivateState` is now named once, above the gate, and used by both, so a tightening cannot move one and leave the other. Both uses are covered by tests. - `packDisplayMeta` gains its own tests: a behavioural key listing, and a source-level one, because a conditionally spread field that no fixture sets is invisible to the first. The helper now feeds a public response, so a field added there for a storefront card has to be added here too. - The pack detail response fence extends to the members array by key, not only by value, and the verdict test uses a rejecting fixture so a hardcoded `false` cannot satisfy it. The editor guard matches whitespace, which its line length was four characters away from needing. - Dropped an unused member-join mock from the fixture rather than leaving it stating that the members come from a table this path never queries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(creator-shop): count the whitelist's fields, not just their spellings A key regex over the source can only see the spelling it guesses at, and `{ coverUrl }` shorthand carries no colon — so the fence counts the conditional spreads as well, which fires however a field is written, and the names say which three. Anchored through the `=` so a longer identifier declared above cannot capture the slice. Also: the helper's docblock said one of its four call sites was public; all four are. And two comments claimed more than their tests pin — sharing a predicate is coverage of behaviour, not a guarantee the sharing survives, and the accepting arms of the gate test are what stop its refusals passing for the wrong reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(creator-shop): use the canonical db client mock in the pack detail fences The hand-written `vi.mock('~/server/db/client')` tripped the shared-mock ratchet. The codemod refused it because one local fn aliased the reader's and the writer's `userCosmetic.findMany`, which is the aliasing that mock exists to stop: the ownership read here is on the WRITER deliberately, so a reader-only declaration would leave it empty and quote an undiscounted price. Declared against `dbMock` per client instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(creator-shop): make the writer-pool ownership read observable A comment claimed the reader/writer distinction was load-bearing here while no assertion could see it: the ownership fixture only ever answered an empty array, which is also the reader's default, so pointing the read at the replica changed nothing. One test now owns a member and asserts the discounted quote, and the mutation fails it with `expected 0 to be greater than 0`. Two blind spots noted where the next reader meets them: the canonical mock resets once per file, so call counts accumulate across it; and the whitelist fence cannot see a field added by delegating to a nested helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 23:02:40 -06:00
Justin Maier	b1a59932b5	test(blocks): restore the local non-vacuity check in the seam guard (#4931) ... * test(blocks): restore the local non-vacuity check beside the backslash filter Without it the filter is `[].filter()` on an empty walk. The walk control in the neighbouring `it` answers a different question, and a guarantee that lives in another test is not one this assertion can rely on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(tests): take the comment-review trims on the seam guard Cuts the label the assertion below already states and the clause defending the diff, and fixes the neighbouring count this branch made stale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 22:58:07 -06:00
ZacxDev	5de4ce490d	chore(moderator): release moderator-v0.0.68 moderator-v0.0.68	2026-09-17 23:47:34 -05:00
ZacxDev	a2eeefb8be	chore(moderator): release moderator-v0.0.67	2026-09-17 23:47:34 -05:00
Justin Maier	4a2a6e97b0	fix(tests): key the block-gated-images seam ledger in POSIX separators (#4930) ... * fix(tests): key the block-gated-images seam ledger in POSIX separators relative() answers in the host separator while every ledger literal in the guard is POSIX, so on Windows `rel === DEFINITION`, `toContain(DEFINITION)` and `SOURCE.get(<literal>)` all missed: the walk's own positive control failed and the two real assertions ran on the empty string. Normalise once where the rel is produced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(blocks): make the seam guard's normalisation assertion falsifiable Three review findings. `split(sep).join('/')` is the identity on Linux, so the assertion pinning it could only ever fail on Windows; `replace` folds on every host and the case now asserts a hardcoded separated input, which reddens on CI too. And the `=== 'hidden'` prohibition read `SOURCE.get(rel) ?? ''`, so a ledger literal that stopped matching a key let the negative half pass free on the empty string while only the positive half reddened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(blocks): pin the normalisation at its call site, not just the helper Extracting toPosix created a second removable site and the pinning assertion had moved onto the one nobody would delete: dropping the toPosix(...) call inside toRel left it green. Assert the composed toRel over a join()-built path, restore the check over the walk's real keys, and drop the duplicate of the walk control. Also name the consumers the `=== 'hidden'` prohibition iterates — allow-listing the last one left that test executing zero assertions and green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 22:42:38 -06:00
Zachary Lowden	f835d678fd	feat(blocks): compute the per-generation author fee, dark (slice 1 of 3) (#4922) ... * feat(blocks): compute the per-generation author fee, dark The App Blocks author fee is moving from a platform-funded percentage bounty to an ADDITIVE, AUTHOR-SET, VIEWER-PAID per-generation fee: the viewer pays it on top of the base generation cost, the platform takes no cut and funds nothing. fee = max(flatBuzz, pctOfBase x base_generation_buzz) This is slice 1 of 3 — the computation and its configuration. It is DARK: it moves no money, writes no column and reads no row. Settlement onto the licensing-fee rail is slice 2; the author-facing config UI and the viewer-facing disclosure are slice 3. What lands: - src/server/services/blocks/author-fee.ts — the platform defaults (1 flat / 5% of base), the ceilings (flat <= 100, pct <= 100%, and NO minimum on either), the per-generation-type table, the pure computation, and the one gated entry point. - app-blocks-author-fee-enabled — a dedicated global flag, fail-closed. The flag is read FIRST, before the base is inspected and before the telemetry module is imported, so with it off the computation is unreachable and emits nothing at all. The flag does not exist yet, so the as-merged behaviour is fully dark. - Three counters and seven log fields on the path spend attribution already uses, so the settlement slice can be sized from real traffic before anyone is charged. Nothing is persisted. Decisions worth flagging: - NO MIGRATION. The defaults apply to every app including the 24 that already exist, so absence of per-app configuration means the default applies and there is nothing to back-fill. Per-app storage only becomes necessary when an author can edit it, which is slice 3; a table nobody can write to yet is pure cost on a database whose migrations are applied by hand per environment. - ZERO BASE MINTS NOTHING, as a rule separate from the formula. A plain max(1, 5% x 0) is 1, so the flat leg would invent a fee for a generation that cost nothing. Zero-base generations are real (17 of 600 measured events). - THE BASE IS WorkflowCost.base, NOT .total AND NOT the attribution row's buzzAmount. `total` already carries the per-resource model licensing fees, the lineage fee and the viewer's tips; the attribution row's buzzAmount is the realized paid debit, i.e. the same gross. A percentage of either would take a cut of another creator's licensing fee and would compound as fee-charging resources stack. All three are plain positive Buzz integers, so nothing downstream can tell them apart — which is why the base is hoisted off the raw orchestrator response at each submit path and pinned by a seam test. - The block-facing snapshot's cost stays `{ total }` only. Surfacing `base` there would publish the cost breakdown to every third-party app for a number no app has asked for. - SELF-SPEND IS CHARGED, unlike attribution (which voids it) and unlike computeSpendShare (which zeroes the share). A bounty is the platform paying an author out of platform money, so self-spend is a wash; this fee is the viewer paying the author, and an author using their own app is a viewer. At settlement that becomes a transfer from an account to itself, which slice 2 must decide about explicitly rather than discover from a rejected transaction. Called out at the call site. - The fee STACKS on top of the lineage fee and the model licensing fee. Slice 1 computes this app's own entry only; it wires nothing into the orchestrator's fees[] array. - Deliberately NOT a rate card. A RateCard splits platform revenue with an author; this is new money flowing to the author with no platform share. * fix(blocks): seed the author-fee type table, and correct what slice 1 claims Round-0 audit findings applied. The largest is that the platform per-type table shipped EMPTY, so every production lookup fell to the default and the resolver's type/coarse arms were unreachable outside their own unit tests. - SEED `chat-completion` -> 0 flat / 0 pct. Justin's motivating example as a PLATFORM default rather than something each author rediscovers in slice 3: chat completion is the highest-frequency generation type an app runs, so a 1 Buzz flat floor per turn is a per-message toll, not a fee on a generation. Seeding also makes the resolver live, which changes what is worth logging. - REPLACE the "not a rate card" argument, which was FALSE as written. It said a rate card splits platform revenue while this is new money with no platform share; RATE_CARD_V4's own accounting note refutes that - its spend bounty is "a SEPARATE platform expense paid ON TOP ... NOT a slice carved out of the viewer's money", with no three-way conservation invariant. The conclusion stands for two other reasons, both verified: a RateCard is an immutable, platform-wide snapshot stamped on a row at write time while this fee is per-app, author-set and mutable; and every card field is a percent of USD CENTS, where the card's per-row cent flooring is exactly what made the bounty pay $0.00 (at spendSharePct 5, computeSpendShare returns 0 cents for every generation under 200 Buzz). - DROP the two dynamic imports. The comment claimed the flag is read "before the telemetry module is even imported"; that was false - buzz-attribution statically imports ~/server/prom/client and blocks.router statically imports app-blocks-flag, so both were already in the module cache and the .catch fallback was unreachable. Static imports now, claim deleted not reworded. - TRIM the Axiom fields 7 -> 4, one instrument per property. Kept authorFeeSkipped (the only instrument for the flag-disabled denominator), authorFeeBuzz and authorFeeBaseBuzz (the counters are labelled coarse_type only, so the per-app / per-isSelfSpend cut lives here) and authorFeeParamsSource (no counter carries it, and it is genuinely variable now the table is seeded). Dropped authorFeeObserved (derivable), authorFeeLeg (duplicates the observed counter's outcome label) and authorFeeParamsClamped - re-derived after the seed and still a compile-time constant false. A ledger test now fails when the set grows OR shrinks. - REMOVE the redundant .catch at the call site. observeBlockAuthorFee is total by contract, so it was unreachable - and had it been reachable it would file a throw into the flag-disabled population, one of the two denominators the slice-2 sizing read divides by. - MOVE the seam guard to src/server/services/__tests__/no-divergent-author-fee-base.test.ts. It is a source-text structural guard over a call-site population, which is the class the no-*.test.ts convention-guard directory exists for. Outside it, no-lint-rules-script-drift could not see it and test:lint-rules did not run it, so the one genuinely red-at-base guard here only ran in the full suite. test:lint-rules: 42 files / 593 tests -> 43 / 598. - FIX the guard's self-contradicting header, which said the base must be snapshot.cost?.base - the opposite of its own next paragraph and assertions. BlockWorkflowSnapshot.cost is deliberately { total } only, so the base comes off the raw submit response. - RECORD that the max combinator is operator-specified verbatim ("make it a 'largest of flat or percent'"). Round 0 flagged it unattributed only because it had not been told. Red-before/green-after, measured by restoring byType to [] and swapping in origin/main's router (both md5-verified back): 8 failed / 43 passed -> 51 passed. Mutation sweep 13 mutants, 12 killed each by the guard owning the property with its own message, plus a comment-only negative control that SURVIVED. No guard was unkillable; none deleted. * fix(blocks): apply round-1 audit findings to the author fee Round 1 returned no red findings; every item below is a yellow or green. Three of them were properties the code HAD and nothing asserted — each was found by a mutant that survived a fully green suite, so each fix is carried here by that same mutant now dying, not by a test merely being added. ONE REAL DEFECT — an inert constant with two spellings of one policy. BLOCK_AUTHOR_FEE_MAX_PCT_OF_BASE had exactly two references: its declaration and one test asserting toBe(1). No implementation read it. clampBlockAuthorFeeParams capped the percentage leg with BLOCK_AUTHOR_FEE_BASIS_POINTS_SCALE, which was doing double duty as scale factor AND ceiling. Setting the constant to 0.5 and deleting the one pinning line left every test green — i.e. the "pct <= 100%" policy had two independent spellings that agreed only by coincidence, and slice 3 reads this constant for author-input validation, at which point a UI bound and an enforced bound can silently disagree. Fixed by DERIVING: BLOCK_AUTHOR_FEE_MAX_PCT_BASIS_POINTS is Math.round(MAX_PCT_OF_BASE * SCALE), and the clamp enforces that. Guarded by a test asserting both halves — the RELATIONSHIP (an over-ceiling config must land at exactly the declared fraction of the base) and the LITERAL (so the pair cannot drift together). Proven with a mutant that un-derives the bound and deletes both literal pins, leaving only the relationship to catch it: it dies, naming the divergence. THREE UNASSERTED PROPERTIES, each proven by its mutant. - Self-spend IS charged an author fee. This DIVERGES from attribution two lines up, where isSelfSpend voids the row and zeroes the share — a bounty is the platform paying an author out of platform money, while the fee is the viewer paying the author, and an author using their own app is a viewer. The divergence was argued in ~15 lines of comment and pinned by nothing: routing self-spend to a null base survived green. Now asserted end-to-end on the log line (voided row, isSelfSpend true, fee 32 off a 640 base). - The fee is observed AFTER the successful write, never before. That ordering is the only thing stopping a P2002 re-poll from double-counting, and the sizing number is this slice's entire purpose. Hoisting the observation above the create survived green. Now asserted via the flag read — the one thing every path through observeBlockAuthorFee does — as "a duplicate emits no second observation". - The flag-off docblock overstated its test. The comment claimed "a gate moved below the computation would show up right here"; it does not, because computeBlockAuthorFee is pure and a hoisted copy emits no counter either. WIDENED rather than narrowed: a new test hands observeBlockAuthorFee an args object whose generationType and config are GETTERS, and asserts neither is read with the flag off — any invocation of the computation must read its inputs, purity or not. The same test carries its own positive control (flag ON reads both). The old comment is corrected to say exactly what the counter assertions do and do not pin. Harmless in slice 1; load-bearing in slice 2, where the gated code moves money. SMALLER ITEMS. - The comment justifying the removed .catch called the enclosing catch "loud", which understates it by three effects: a rejection there also skips the success Axiom line for a row that WAS written, skips the write counter, and runs refundAppBountyAccrual against a persisted row (inert only while appOwnerShareCents is identically 0). The unreachability argument is sound for today's caller, so the comment is corrected rather than the .catch reinstated — and it now records that a reinstated catch needs a THIRD skip reason, never flag-disabled and never base-unavailable, both being live denominators. - The test comment claiming "the dynamic imports inside observeBlockAuthorFee resolve to them" is corrected: both imports are static, and vi.mock hoisting is what makes the mocks apply. The same claim was already deleted from the source docblock. - The basis-point quantization now FLOORS instead of rounding. Math.round(0.049999 * 10_000) is 500 bp, i.e. a full 5%, against the module's own promise that "a stated 5% never charges more than 5%". Unreachable in slice 1 (only 0.05 and 0 exist), reachable the moment authors type numbers. A naive Math.floor is NOT the fix on its own: measured, it loses a basis point on 573 of the 10,001 exact basis-point inputs, because 0.0029 * 10_000 lands at 28.999999999999996 and an author typing 0.29% would be charged 0.28%. Normalising to 6 decimal places first makes all 10,001 exact (pinned by an exhaustive test) while still flooring 0.049999 to 499. - One spelling of the missing-base skip. The Prometheus outcome label said base_unavailable while the Axiom field said base-unavailable — two spellings of one concept across the two instruments a sizing read has to join. Both now come from one exported constant. MUTATION MATRIX (both covering suites, 90 tests, at this commit): M14 self-spend routed to a null base SURVIVED -> KILLED M15 observation hoisted above the write SURVIVED -> KILLED M4 computation invoked before the gate SURVIVED -> KILLED M1 ceiling moved, its one pin deleted SURVIVED -> KILLED M1' + the relationship assertion alone SURVIVED -> KILLED M13 comment-only (NEGATIVE CONTROL) SURVIVED (as required) harness positive control (default flat 1->2) KILLED, 7 tests Each kill was read from the runner's own per-test counts, not an exit code, and each killing assertion carries its own message naming the property. No behaviour change at any reachable slice-1 input: the derived ceiling is 10_000 exactly as before, and the only percentages production can produce are 0.05 and 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(blocks): apply round-2 audit findings to the author fee Round 2 verified all eleven prior claims and returned no red findings. Three of the four items here are false sentences in shipped comments — the class this PR has been fixing for two rounds — and one is a latent code issue. F1 — THE PR SAID THE FLIPT FLAG DOES NOT EXIST. IT NOW DOES. app-blocks-author-fee-enabled was created in the flag store at enabled: false after this branch's last commit, deliberately: round 1 found that an ABSENT key makes the evaluation throw, bypass its cache and write a console.error on every App Blocks generation submit, indefinitely. Verified live before rewriting, rather than taken on trust — in the civitai-app environment the flag is BOOLEAN_FLAG_TYPE, enabled: false, with empty variants / rules / rollouts, and a global boolean evaluation returns enabled:false, reason:DEFAULT_EVALUATION_REASON, segmentKeys:[]. Negative control: a fabricated key 404s. Four sites corrected, not three. The brief named author-fee.ts, app-blocks-flag.ts and the PR body; a fourth operator note in author-fee.ts still read "create app-blocks-author-fee-enabled", which is the same stale claim in imperative form. The conclusion survives — as-merged behaviour is dark — but the REASON changes and the safety property is genuinely weaker than the old wording claimed. An absent key had to be CREATED by an operator before anyone could enable the fee; a present base-false flag is one toggle away, with no deploy and no review. So the comments now say the posture is flag STATE, not structure, and "cannot regress open" is withdrawn rather than reworded. F2 — THE LABEL UNIFICATION WAS ONE SITE SHORT, AT THE METRIC'S DECLARATION. packages/civitai-telemetry/src/client.ts still enumerated base_unavailable (underscore) while the emitted value is base-unavailable (hyphen). That comment is the ONLY place in the repo enumerating the label's value set and it sits directly above blockAuthorFeeObservedCounter, where an operator writing the slice-2 sizing join looks: they would query outcome="base_unavailable", get an empty series, and read it as "no generation lacked a base" — exactly the silent empty join the unification exists to prevent. The two other base_unavailable occurrences (author-fee.ts and the test) are historical narration of the form "it used to say X here and Y there" and are deliberately left alone. F4 — THE CEILING CONSTANT ROUNDED THE OPPOSITE WAY TO THE QUANTIZATION IT GOVERNS. BLOCK_AUTHOR_FEE_MAX_PCT_BASIS_POINTS derived with Math.round while toBasisPoints — added in the same commit, a few lines above — deliberately floors, because in the module's own words "every rounding goes toward the viewer". Inert today (MAX_PCT_OF_BASE = 1 gives 10000 either way, and a guard pins it) and live the moment slice 3 or a policy change sets a non-basis-point ceiling: 0.123456 derives to 1235 bp = 12.35%, a ceiling ABOVE the declared policy, the one direction the module says it never rounds. The derivation is now blockAuthorFeeCeilingBasisPoints(maxPctOfBase), which calls toBasisPoints. It is a named function rather than an expression inlined into the constant for a specific reason, recorded at the site: at a declared ceiling of 1 no assertion about the constant can tell floor from round, so inlining it makes the defect unkillable rather than merely dormant. A comment on toBasisPoints now names both callers, so changing its direction is visibly a change to the enforced ceiling too. Carried by a mutant, not by a test merely being added: reverting the derivation to Math.round — which is the pre-change code on the same input — fails exactly one test, with its own message, "the ceiling derivation ROUNDS - it must floor, like the quantization it governs: expected 1235 to be 1234". The guard also pins 1235 as the mutant's answer so the case cannot quietly stop discriminating, and asserts the property at four ceilings that each round up. F3 — A CONTROL MESSAGE NAMED THE WRONG CAUSE ON AN OVER-READ. The positive control asserted toBe(1) under the message "probe wired to nothing — flag ON read nothing", which is right only for 0. Round 2 demonstrated it: an innocuous second read past the gate produced "probe wired to nothing - flag ON read nothing: expected 2 to be 1", sending a maintainer into the probe when the real change is downstream of the gate. Split into a toBeGreaterThan(0) control — the claim the zeros above actually need — and a separate exactness pin naming the other direction. Verified by planting a second args.generationType read past the gate: it now fails with "generationType was read more than once past the gate - a change downstream of the flag, not a broken probe: expected 2 to be 1". TESTING Two covering suites: 90 passed (90) at 3a81dc7 -> 91 passed (91) here, one new test. With the seam guard: 96 passed (96) across 3 files. The telemetry package suite is 38 passed (38). Prettier clean on all four files, validated against a deliberately misformatted copy that it correctly rejected. Mutation sweep, each mutant restored and byte-compared with cmp afterwards: M1 platform table emptied KILLED 6 failed / 85 passed M1' ceiling un-derived, pins deleted KILLED 2 failed / 89 passed M4 resolver exact-match arm deleted KILLED 6 failed / 85 passed M5 dark gate deleted KILLED 3 failed / 88 passed M14 label unification reverted KILLED 2 failed / 89 passed M15 counter outcome label desynced KILLED 1 failed / 90 passed F4 ceiling reverted to Math.round KILLED 1 failed / 90 passed M13 negative control, comment-only SURVIVED 91 passed NC2 negative control, comment-only SURVIVED 91 passed Two negative controls rather than one, in two different files, so a green sweep is not a claim about a runner wired to always-kill. * fix(blocks): wire the fourth submit path, fee-free while its price is a cap A fourth `recordSpendAttribution` call site landed on `main` mid-review — `submitPassThroughStepWorkflow` — passing no base. The seam guard caught it and went red, which is the guard doing its job: nothing in this branch's own commits broke. The new site is WIRED rather than exempted (an exemption is what the ledger exists to make impossible), but it does NOT charge a fee while its price is a CAP. `WorkflowCost.variable` means the quoted price is a ceiling that settles lower: the viewer is charged the maximum up front and refunded the difference once the provider reports the work delivered. A percentage of that is a fee on money they did not ultimately spend. That path quotes a ceiling and refunds `ceiling - actual` on settle, so it is the motivating case. The cap flag is hoisted off the raw orchestrator response and threaded from ALL FOUR submit paths, not just the new one — the predicate lives in one place (`observeBlockAuthorFee`), and a cap-priced generation on any path is the same question. A cap gets its OWN counted skip reason, `price-is-cap`, checked BEFORE the base check. It is deliberately not `base-unavailable`: that is the RECOVERABLE blind spot, one of the two denominators the slice-2 sizing read divides by, and a cap-priced generation would not have charged even with a base in hand. The value is enumerated in `packages/civitai-telemetry/src/client.ts`, the only place in the repo that enumerates that label's value set, in the same hyphenated spelling the Axiom `authorFeeSkipped` field uses so the two instruments still join. "No fee on a cap" is the CURRENT answer, made explicit and testable, not settled policy — slice 2 owns what a cap-priced path should charge, and now has a counted population to decide it against instead of an unlabelled blind spot. Seam guard: ledger 3 -> 4, plus a second relationship over the same population (every call site must thread the cap flag, and it must come from `submitted.cost.variable`, never from `snapshot` — whose `cost` is `{ total }` only, so reading one would be `undefined`, i.e. the fail-OPEN direction). * fix(blocks): apply round-3 audit findings to the author fee Three green findings from the final audit round. 1. The ceiling constant could be re-inlined undetected. Replacing the initialiser of BLOCK_AUTHOR_FEE_MAX_PCT_BASIS_POINTS with Math.round(MAX_PCT_OF_BASE * SCALE) — leaving the named derivation function in place — SURVIVED the whole suite, because every behavioural pin agrees while floor and round agree at the shipped policy of 1. The "DO NOT INLINE IT BACK" comment was the only guard, and prose is not a guard. Adds a source-text assertion, the same technique the router seam guard already uses: the constant's initialiser must name blockAuthorFeeCeilingBasisPoints and must perform no arithmetic of its own. That mutant now goes red with its own message. 2. The .catch comment in buzz-attribution.service went stale. It said a reinstated catch would need "a THIRD skip reason … never flag-disabled and never base-unavailable"; price-is-cap has since landed, so it would be the FOURTH and the "never" list had a hole exactly where the newest reason sat. Restated against BlockAuthorFeeSkipReason as a whole, so the next addition cannot re-stale it. 3. An absent WorkflowCost.variable is treated as a final price, and that is the fail-open direction — undocumented until now. The field is ?: null | boolean, so === true merges "not a cap" with "the orchestrator did not say", and a field that stops being sent silently resumes charging on cap-priced generations. Documented at the txt2img hoist, at the pass-through hoist, and in the seam guard, which pins that the flag comes off the right OBJECT and says nothing about it being absent. Behaviour unchanged: the tri-state policy is slice 2's to decide. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 23:17:30 -05:00
Zachary Lowden	a01dcd4d14	feat(feedback): snapshot the reporter's console and network errors at submit time (#4819) ... * feat(feedback): snapshot the reporter's console and network errors at submit time A moderator opening a report gets a Faro session id and a Grafana deep link, and the link is suppressed on every row in the queue today: `faroSessionLink()` returns null past `FARO_LOKI_RETENTION_HOURS` (72 h). An in-page panel that queried Loki for console/network detail would inherit the same wall. This writes the data into the report instead, at submit time, where retention cannot reach it. The reporter's browser already has it. A bounded ring buffer records the last 10 console errors and the last 10 failed requests, redacted and clipped on the way in, and `handleSubmit` attaches whatever is in it. Bounds and redaction, all on the capture side because the schema REJECTS rather than clips: 10 entries each; 300 chars per console line; 300 chars per URL. Console text goes through the existing Faro `redactText` scrub. Request URLs lose their query string and fragment outright, and a non-http(s) scheme is refused rather than stripped. No bodies, no headers, no stack traces, no `console.warn`/`log`. `fetch` is NOT patched. Network capture is a passive `PerformanceObserver` over resource timings, so nothing this adds can alter, delay or fail a request. The cost is named in the module: status-0 failures (offline, DNS, CORS) are indistinguishable from an opaque cross-origin success and are therefore not recorded, and a browser without `responseStatus` captures nothing at all. Two keys are added to `feedbackContextSchema`. That is the load-bearing half: a `z.object` STRIPS an undeclared key silently, so a capture shipped without the declaration would submit cleanly and store nothing. Ships with a disclosure line on both prompts. `context` already carried the page path, the `/apps` filters including the typed search term, and the session id, none of it disclosed, while the screenshot was the only opt-in; adding console and network capture to that payload is what made the gap indefensible. The drawer's old copy ("so console errors come with the report") was also false — `FaroProvider` excludes the Console instrumentation — and is replaced by one shared constant both surfaces render. Moderator side renders both lists as TEXT. No href, no src, no `EdgeImage`; the existing 26 rows are unaffected, this helps new reports only. * refactor(moderator): resolve svelte-review findings on the browser-error panel Findings from the repo's `svelte-review` (correctness / idiom / abstraction) over the `apps/moderator` half. The producer half under `src/**` is main-app Next code and was not in that review's scope. Correctness: - `isNetworkError` checked three `typeof`s and never the key set, so an entry the producer later grows — `{url, status, initiatorType, method}` — would pass, the renderer would draw its fixed three spans, and `method` would appear NOWHERE: not in the section, and not under "Other context" either, because the key was claimed. That is the one drift direction the "other" bucket cannot cover, and it is silent and total. Now an exact-key check, so a drifted entry dumps the whole array visibly — the same trade `consoleErrors` already takes on element-type drift. - The status was coloured red unconditionally under a "Failed requests" heading. The read side deliberately does not re-impose the producer's `400..599` bound, so a row stored under a future widened bound could carry a status-0 — an opaque cross-origin SUCCESS as often as a failure. Red is now conditional on a real 4xx/5xx. - "last N, oldest first" asserted two things this app cannot observe: N is the stored array's length, not a cap, so a row holding 400 and a row holding 10 both read "last 10" and the operator could not tell a complete snapshot from a tail. Now "N captured", and the unverifiable ordering claim is gone. - Both lists were unbounded inside a component whose sibling dump caps at `max-h-64 overflow-auto`. They now cap the same way. - An empty console string is rejected at the schema (`.min(1)`, as `sessionId` already does) rather than rendered as an empty bordered box. Abstraction: - The two sections are extracted to `FeedbackBrowserErrors.svelte`. The seam is not the line count: the parent's whole `<script>` — clipboard state, the destroy-time timer, `faroSessionLink`, `reconstructFeedbackUrl` — serves section one only, and the moved markup reads none of it. Matches this directory's own precedent. - 🔴 The extraction is only safe because the request-attribute ledger was fixed FIRST. It counted `href=`/`src=`/`EdgeImage` in one named file, and both surviving `href`s live in the section that stayed — so moving `{entry.url}`, the one reporter-chosen string here that looks like it wants to be a link, would have left that assertion reading 2 and PASSING over unscanned markup. It now sums across an explicit file list, with the positive control applied per file. Both halves are mutation-checked: narrowing the list back to one file, and adding an `href` in the extracted file, each go red. - `isString` is used at the `images` branch too, which lets its `as string[]` cast go. Declined, with evidence rather than preference: - `text-red-400` -> `text-destructive`. `text-destructive` has 0 uses in this app; `text-red-400` has 9 and is the documented top of the severity ramp in `$lib/queue-thresholds.ts`. "Reuse the shapes already on the page" wins. - Wrapping the row in `<Badge>`. Its fixed `h-5` pill does not baseline-align with the dense mono row beside it. - A shared snippet for the two near-identical sections, and a handler table for `splitContext` — a table indexed by JSONB keys fails open on inherited properties, which this org has already shipped once. Comments trimmed to the standard's breakage-guard-only rule: the rollout story, the "separate deployable" restatement and one piece of review idiom are gone. Verified by SSR-rendering the panel against absent / empty / populated / hostile / drifted fixtures: hostile markup escapes to text, `javascript:alert(1)` renders inside a span rather than an href, and a drifted entry shows every field in the dump. * fix(feedback): drop the false disclosure, mark clipped strings, collapse console repeats Three round-0 audit decisions on the browser-error snapshot. 1. DROP the telemetry disclosure line entirely. It claimed "Web addresses are stored without their query strings", which is true for networkErrors[].url (sanitizeNetworkUrl clears url.search) and for context.path, and FALSE for a URL embedded in consoleErrors text: sanitizeConsoleMessage -> redactText only rewrites params whose NAME is in SENSITIVE_PARAM_KEYS, so a ?query= or ?prompt= survives verbatim. Console text stays exactly as the browser emitted it; the sentence goes rather than being narrowed, so no weaker claim replaces it. 2. Mark a clipped string so a moderator can tell truncation from completeness. redacted.slice(0, 300) left a cut React hydration message and a complete 300-character one rendering identically. A single U+2026 is now appended only when slice actually cut, and is SPENT OUT OF the bound rather than added to it -- feedbackContextSchema REJECTS an over-long value, so a 301-character result would fail the reporter's whole submission. A string of exactly the max is not truncated and gets no marker. 3. Collapse console repeats into a per-entry count, keeping 10 DISTINCT. The ring buffer kept the LAST 10, so a React cascade (error -> component stack -> boundary re-render -> retry) shipped ten copies of the downstream symptom and zero copies of the originating error. CountingBuffer keeps up to 10 distinct messages, first-seen first, and a repeat increments count on the entry already there WITHOUT refreshing its position -- refreshing recency would let a looping message evict the originating error by another route. The bound stays at 10 distinct: context is a JSONB column fed by a client-controlled array, and the payload derivation stays ~6.6 KB. count is a DECLARED field of the nested z.object. A z.object strips at every level, so an undeclared count would be dropped silently, the producer would count correctly, submit cleanly, and every entry would render as a single occurrence. The schema test reads count back out with an unknown SIBLING field alongside as the negative control. The moderator renderer shows the count as a badge above 1, splitContext gains an exact-key isConsoleError guard mirroring isNetworkError, and the RingBuffer docstring's old rationale ("the first few are usually page-load noise") is rewritten -- it is right for a slow drift and inverted for a cascade. Gates, each with a live control in the same invocation: typecheck 0 errors (control: TS2339); svelte-check 0 errors over 9489 files (control: 1 error); apps/moderator vite build ok; prettier clean (control: a real misformat flagged); eslint clean (controls: prefer-const on .ts, no-debugger on .svelte -- prefer-const is OFF for .svelte and no-debugger is not configured at the repo root, so each tier needs its own live rule). Suites before -> after: unit 96 -> 109, app:moderator 103 -> 114, component 50 -> 46 (the four removed disclosure assertions). 13 mutants, all killed by the test that owns the guard, including: an unconditional marker and a < / <= boundary slip (both caught by the exactly-300 arm), a marker appended past the bound, no collapse, a repeat refreshing recency, eviction leaving the index entry behind, read() handing back live references, count undeclared, the count floor removed, the exact-key check dropped, and the badge rendered unconditionally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(feedback): pin that the repeat index tolerates Object prototype keys The CountingBuffer added in the previous commit indexes repeats by the console message itself, which is untrusted text. As a `Map` that is safe; as a plain object it FAILS OPEN -- `index['__proto__']`, `'constructor'` and `'toString'` all return an inherited truthy value, so `push` takes the "already seen" branch, increments `count` on something that is not an entry, and the message is stored NOWHERE while the first sighting silently vanishes. Not exotic input: `console.error(someObj)` formats to JSON and library errors mention these names routinely. Mutation-checked rather than assumed -- swapping the `Map` for a `Record<string, FeedbackConsoleError>` reddens this test specifically: × records a message that collides with an Object prototype key AssertionError: expected [] to deeply equal [ ...(4) ] i.e. all four messages disappear entirely, which is the fails-open behaviour the `Map` prevents. The test pins the data structure by BEHAVIOUR rather than by grepping the source for the word `Map`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(feedback): clip on code points, install the recorder at import, document the console/network asymmetry Three findings from an adversarial audit of the browser-error snapshot. 1. clip() could split a UTF-16 surrogate pair. 'length' and 'slice' count code units, so a 300-unit bound landing between the halves of an astral character kept a LONE SURROGATE. The value still satisfied feedbackContextSchema's .max(300) (same units), JSON.stringify preserved it as an unpaired escape across the wire, and Feedback.context is jsonb — measured against a real Postgres engine, the insert fails with "invalid input syntax for type json" while the pair-intact twin inserts cleanly. The reporter's WHOLE submission was lost, with an error naming nothing about the snapshot. clip now drops the whole character when the cut would split it, so a clipped result is occasionally one character shorter than max and never longer. 2. The recorder installed too late to catch the case it exists for. 'useEffect(() => installBrowserErrorLog(), [])' runs in the passive-effect flush AFTER the commit that hydrates the tree, so every console.error React emits while hydrating — a hydration mismatch above all, which both docstrings name as the motivating case — arrived before the wrapper existed. The network half is retroactive by construction (observe({ buffered: true }) replays the resource-timing buffer); console.error leaves no such buffer, so installing earlier is the only available fix. BrowserErrorRecorder is replaced by a side-effect module, src/utils/feedback/startBrowserErrorLog.ts, imported first in _app.tsx next to the existing disable-router-prefetch side-effect import. installBrowserErrorLog already returns a no-op without a window, so the call is inert under SSR — now pinned by a node-environment test, because module scope is a code path the effect version never reached. 3. Documented, without changing, the console/network query-string asymmetry. sanitizeNetworkUrl strips a captured request's query string outright; a URL inside console TEXT keeps its query string, and only params whose name matches SENSITIVE_PARAM_KEYS are redacted. That is an operator decision (2026-09-14), and it was recorded nowhere but a commit message, so the next reader assumed the two paths were symmetric. Stated on the capture module and on the schema field. Coverage, each mutation-tested: · clip: guard disabled -> "does not cut a surrogate pair in half when it clips" fails on expect(out.isWellFormed()).toBe(true). Range widened to include low surrogates (over-trim) -> the boundary control "keeps an astral character that ends exactly on the boundary" fails on its toBe, and nothing else does. · install timing: module-scope call removed -> "importing the module is what installs the console patch" fails on expect(capturedAtImport).toContain(SENTINEL) with an empty buffer. · SSR guard: 'typeof window' check removed -> the dynamic import in the node test rejects with ReferenceError and the assertion reports it. * fix(feedback): drop input-borne lone surrogates, not just ones clip makes Round 2 of the audit found the other half of the hazard clip guards. clip can no longer manufacture a lone surrogate, but it never repaired one it was handed, and an input-borne one reached the wire through both branches -- including the early return, where the value is short enough that clip does nothing at all. The consequence is the one clip's docblock already measures: Postgres rejects the jsonb insert and the reporter's whole submission is lost. Deliberately NOT toWellFormed() and NOT a lookbehind regex. This runs inside the console.error wrapper, which must never throw, and the repo declares no browserslist -- both constructs are Safari 16.4+, so a browser below that would throw a TypeError here. The manual scan is ES5 and cannot. Mutation-verified: removing the pass fails both new tests on their own isWellFormed assertions; dropping the pairing branch (over-drop) fails the new well-formed-astral control plus the two existing boundary tests. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 22:35:33 -05:00
Justin Maier	8d1c6db53b	feat(hubs): let a tag source require several tags at once (#4927) ... * feat(hubs): let a tag source require several tags at once A hub's tag sources are ORed, so there was no way to ask for images carrying BOTH of two tags. A tag card now holds a set: tags sharing a `groupKey` are ANDed, and the groups are still ORed against every other source. A single-tag source is a group of one, so every hub that exists keeps the filter it had. Grouping means opposite things on the two sides, and the wording is the only place that says so: grouping tags you want NARROWS the feed, while grouping tags you want gone REMOVES LESS, because `NOT (x AND y)` keeps an image carrying only x. The migration is additive and nullable — it needs applying by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(hubs): make a session toggle drop a whole tag group, and bound groupKey Review found that subtracting ONE member of an AND-group turns `A AND B` into `A`, which matches a superset — so a viewer-supplied session toggle could widen someone else's hub past what its owner set. That breaks the invariant the comment above `excludedSources` relies on to justify honouring the list at all. A toggle landing on any member now takes the group with it, which is also what the editor's own switch does. Also from review: - `groupKey` is bounded, so an out-of-range value is a validation error rather than a Postgres INTEGER failure inside the replace transaction. Client-side key minting hands out the lowest free key so it cannot climb past that bound. - `groupTagIds` keys on `exclude` as well as `groupKey`, so folding its two calls into one cannot merge a kept-out tag into a hub's own AND-set. It was relying on being called once per polarity; the client already did both. - `renderHubArm` throws on an empty arm instead of emitting the literal text `undefined` into the filter, which Meilisearch rejects as a 503 far from the mistake. No producer can reach it today. - The group state transitions move into `hub.utils.ts` as pure functions and get tests, including the one that gave a tag added to a KEPT-OUT group the include side — a click meaning "block more" that surfaced content instead. - `sameTarget` is gone in favour of `hubSourceKey`, which the server subtracts session toggles with. - A tag the hub already holds on the same side can now JOIN a group. Refusing it left an owner unable to group two tags they already had: the picker showed the second greyed out as "Added", with no way forward. - `buildDuplicateHubInput` carrying `groupKey` is asserted; dropping it silently un-grouped every copied AND-set. - The resolver is pinned over rows carrying `groupKey: null`, which is what a row written before the column looks like. The compatibility claim was previously asserted at both ends of rows -> resolve -> filter and nowhere in the middle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(hubs): assert the polarity scoping on groupTagIds directly The case routed through `resolveHubSources` passed with and without `exclude` in the map key, because that function already splits its rows by polarity and calls `groupTagIds` once per side. Measured: removing the polarity from the key left the whole resolver suite green. The assertion read as coverage of a guard it could not see. Calling `groupTagIds` with a mixed list is the only way to reach it. With the key mutated it now prints `expected [ [ 77, 90, 78, 91 ] ] to deeply equal [ [ 77, 78 ], [ 90, 91 ] ]`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(hubs): ungroup a tag instead of deleting it, and share one group key Round two of review over the fix round. The one that would have cost a user something: the ✕ on a tag chip is labelled "out of this group" and deleted the source from the hub. That was harmless while every grouped tag had just been added by the same click — but a tag the hub already holds can now be moved into a group, so that ✕ is the only visible way to undo the move, and it destroyed a source the owner had all along. It now clears `groupKey`; the card's trash is what deletes. The rest: - `hubTagGroupKey` lives beside `hubSourceKey` in the schema, and the three places that built that key by hand now share it. The previous round left a third spelling — `toggledOffGroups` keyed on the bare int, correct only because the list it reads was pre-filtered by polarity, which is the exact reasoning the same file's new comment says must not be relied on. - `addTagToHubGroup` refuses a cross-polarity move itself. The caller refuses it too and shows the message, but the caller is component code with no suite, so the tested half now lives where a test can reach it. Without it the move keeps `exclude: true` while taking an include group's key, merging a kept-out tag into an unrelated exclusion AND-set — and `NOT (a AND b)` excludes less than `NOT a OR NOT b`, so an exclusion stops excluding. - `groupMemberKeys` and `findHubSource` are exported and shared rather than restated in the editor. A caller and a helper that disagree about whether a row is held would mutate one the caller believed it had refused. - The comment claiming the client and server grouping "cannot share code" said something a reader can disprove. The real barrier is that `hub.utils.ts` imports `trpc` and so is client-only. - A 🔴 comment on a resolver test claimed it guarded the polarity scoping. It cannot see it — `resolveHubSources` splits by polarity before calling `groupTagIds`. Rewritten to say what it pins. - `groupRule` moved out of `HubSourceCard` so its copy pin does not drag a Mantine component's import graph into a pure unit test. - New coverage: the cross-polarity refusal, the whole moved row (against a fixture that differs from the group on every field a broken move could clobber), the vacated group after a move, a non-tag row carrying a groupKey, and the schema bound. - The mixed-`enabled` group hazard is recorded at the resolver: nothing constrains a group's members to share `enabled`, so a half-enabled group resolves to an AND-set of only the enabled half. Benign only while the owner is the only one who can write that column. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(hubs): tighten the tag chip, and put the chip's X back to removing All from Justin looking at it on a dev server. - The ✕ on a chip removes the tag from the hub again. It had been changed to ungroup, on a review finding that deleting there is a trap now that a tag the hub already holds can be pulled into a group — the ✕ is then the only visible undo. He called ungrouping weird on sight: a ✕ reads as "get rid of this", and leaving the tag behind as a new card does not look like a removal. The label matches, and the tradeoff is recorded where the next reader will find it. - The grey block behind the ✕ during a save was Mantine's `disabled` styling on the ActionIcon. The click is guarded in the handler instead. - The chip's spacing matches the removable-tag chip in `Tags/TagsInput.tsx`. The gap left of the ✕ is Mantine's own right-section margin, which outranks a utility class, so it is overridden through `styles` along with the padding. - Include groups no longer carry a rule line — a row of chips reads as "all of these" unaided. The EXCLUDE line stays: that side removes LESS when grouped, because `NOT (x AND y)` keeps an image carrying only x, and this is the only place in the product that says so. The migration file's own comment repeated the deploy-ordering claim I had to correct in the PR body, so it is corrected here too — with the dev failure that proved it. That comment is the artefact a deployer actually opens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(hubs): correct a comment that claimed coverage the fixture lacks The move test said its fixture "differs from the group on every field a broken move could clobber". It does not differ on `exclude` — and cannot, because the move is refused outright when the polarities differ, so by the time that branch runs the two are already equal. Measured rather than reasoned: adding `exclude: !!first.exclude` to the move branch leaves the file green. The mutation is a no-op while the refusal stands, which is the honest thing for the comment to say, rather than a claim a reader would take as coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(hubs): pin the copy that actually renders, not a dead branch Review caught that cutting the include-side rule line left `groupRule(false)` with no production caller, while its test and its doc comment both still claimed "this wording is the only place in the product that states it". The product stated one side. A test guarding a string nothing renders reads as coverage and is not. The asymmetry does still ship on both sides — in the `+` tooltip — so that pair is what gets pinned, as `groupAddHint`, and the exclude-only rule line becomes a plain `excludeGroupRule` constant with no include counterpart to be tempted by. Two smaller corrections from the same pass: - The MOVES comment said a moved row "keeps the first three" of the fields its fixture varies. It keeps two — `enabled` is the one it takes from the group, which the assertion one line below says. - The cross-polarity refusal asserted `toEqual(value)` against the same array the refusal path returns, so it compared a value with itself. It now asserts the rows, which survives a future edit that mutates in place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 20:58:18 -06:00
Zachary Lowden	774fce94cf	refactor(application-error): same-origin beacon guard on the client-error report endpoint (#4915) ... * feat(application-error): per-IP rate limit on the client-error report endpoint `POST /api/application-error` is wrapped in `PublicEndpoint`, which applies no limiter of its own, so any unauthenticated caller could post to it without bound. What the endpoint produces is not a response but an operational signal — its accepted volume is what tells us the front end is broken — so a single unbounded source can manufacture that signal. Adds `checkApplicationErrorRateLimit`: a fixed-window per-IP counter on `sysRedis`, in the same `SET NX EX` + `INCR` MULTI shape as the sibling limiters already in `src/server/utils/`. Past the ceiling the endpoint answers 429 with `Retry-After`, ahead of the session read, the body parse and the sourcemap resolution — so a limited request sheds the work rather than only changing a status code. THE LIMIT: 30 reports / 60s = 0.5/s sustained per address, sized from the endpoint's own traffic at both ends of the tension, because either end silently destroys something. Too loose buys nothing: the ceiling is an order of magnitude below the aggregate rate that distinguishes a broad breakage from one unhappy user, so no single address can reach that rate alone — it takes eleven or more distinct addresses saturating their budgets, which is the many-users condition the signal is supposed to mean. Too tight deletes the reports we exist to collect: 0.5/s per address is above the busiest single sample ever observed for the WHOLE endpoint across every client (~0.4/s, against a ~0.009/s steady state). One user in a client-side render loop is still reported in full — the first 30 reports of any burst always pass, far more than the one needed to diagnose a repeating stack. KEYED ON `getTrustedClientIp`, not `resolveClientIp`. The attribution predicate reads `cf-connecting-ip` with no corroboration, so a caller would be choosing its own bucket and could rotate away from it at will, which voids the only property this control has. This is the same rule the per-IP limiter on `/api/v1/block-tokens` already applies. The module is added to the derivation ledger in `client-ip-ledger.test.ts` under Enforcement, with the divergence from the two attribution limiters stated there. FAILS OPEN on a limiter error, and logs when it does. Failing closed would delete the front-end-break signal for the duration of an infrastructure incident, which is exactly when breakage is most likely and least visible. No in-process fallback: a per-pod counter cannot bound a fleet-wide rate, so it would carry the complexity of enforcement without the property that makes enforcement worth anything. UNCHANGED: the request schema, the `resolveStack` handling, and the absent-name default — the last now pinned by a test, since it sits directly behind the new early return. TESTS. A limiter unit suite backed by a REAL in-memory counter keyed on the key the module actually builds, so "the budget is per address" is a property of the code rather than of a scripted stub. An endpoint suite driving the real handler through its real wrapper, covering 429, `Retry-After`, per-IP independence, that the shed work really is shed, and fail-open. And the client-side fire-and-forget contract: a 429 resolves rather than rejecting, so it cannot affect a render — plus a ledger asserting the reporting helper is the only module in `src/**` that fetches this endpoint, so that contract covers the whole population. Every guard was watched to fail: thirteen mutations, each killed by a named test with its own assertion message, plus a whole-file inert control. * refactor(application-error): replace the per-IP limiter with the same-origin beacon guard Pivots this PR off the bespoke per-IP redis rate limiter and onto the guard the sibling telemetry beacons already use. The limiter is deleted, not disabled. WHY THE LIMITER WENT. Its key collapsed to a shared upstream address whenever the corroborating edge header was absent, so a whole population shared one budget and the accepted volume was capped BELOW the level that distinguishes a broad breakage from one unhappy user. That is silent in the wrong direction: it suppressed the operational signal precisely during the incident the signal exists to surface. It also penalised every shared-egress population — corporate NAT, mobile CGNAT, VPN and privacy-relay exits, whose addresses are exactly the ones the edge reports — and it bounded a population wider than the signal actually reads. WHAT REPLACED IT. `isSameOriginBeacon`, lifted verbatim from the three beacons that already ship it (`src/pages/api/internal/pulse.ts`, `src/pages/api/track/block-render.ts`, `src/pages/api/track/batch.ts`), whose own comments call it "the established beacon pattern": compare the host of `Origin` — falling back to `Referer` for clients that suppress it — against the request's own `Host`, and reject anything else with 400. All three open-code the block, so this extracts it to `src/server/utils/beacon-same-origin.ts` and uses it here rather than making a fourth copy. The three existing call sites are deliberately NOT rewritten in this commit: that is a behaviour-preserving change to the busiest endpoint in the app and belongs in its own reviewable diff. IT CANNOT REJECT OUR OWN PAGES, and that is the property that mattered most, because a false reject deletes the signal silently — `fetch` does not reject on a 4xx and the reporting helper terminates its promise with `.catch`, so a wrongly rejected report is indistinguishable from no error having happened. Structurally: the reporter POSTs a RELATIVE path, so the browser's `Origin` and the `Host` the request is routed to are the same string by construction, on any host the app serves. The guard enumerates no domain and therefore cannot single one out — adding, renaming or previewing a served host changes nothing. Empirically: the three siblings have run this exact rule in production for a long time at high volume. Measured over a 6h window, POSTs answered 400 were 0.111% on one beacon and 0.029% on another, against 200s numbering in the hundreds of thousands. Those 400s also include the body-parse and schema branches, so the guard's own share is at most that. A served host or browser family being rejected wholesale would have to account for under 0.111% of that beacon's traffic to hide inside the figure. THE DEV BRANCH IS SCOPED TO THE GUARD, not to the whole handler as in the siblings. Theirs returns 200 before their analytics write, which costs them nothing; here it would also skip the schema parse, so a malformed body would answer 200 in dev and 400 in production. The observable dev property is the same either way — the guard rejects nothing locally. DELETED, with nothing left dangling: the limiter module and its unit suite, the `REDIS_SYS_KEYS.CLIENT_ERROR` registry entry, and both rows the limiter added to the client-IP derivation ledger (it no longer derives a client IP). Also the `describe` block asserting that a 429 resolves rather than throwing: its fixture installed a `fetch` double that resolves by construction, so the property was supplied by the fixture rather than by the code. The ledger half of that file — which pins that the reporting helper is the only module fetching this endpoint, with its own positive and negative controls — is kept. TESTS. A unit suite for the predicate, led by the accept cases because those are the load-bearing half, including one that pins the no-enumeration property across six unrelated hosts. An endpoint suite driving the real handler through its real wrapper for the seam the unit suite cannot see: that the handler calls the guard, answers 400, sheds the session read and the sourcemap resolution behind it, and is inert in dev. Eleven mutations were each watched to fail against a named test, plus a whole-file inert control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(application-error): restore the network-rejection case the prune took with it The previous commit deleted a `describe` block whose cases asserted a property their own fixture supplied — a `fetch` double that RESOLVES by construction, so "a 429 resolves rather than throwing" only ever proved that a resolving mock resolves. That reasoning was right, and those cases are staying gone. The deletion was one case too wide. Inside that block sat a case on a REJECTING `fetch`, and it is not the same shape: its fixture supplies a rejection, so the fulfilment it asserts cannot come from the fixture. It can only come from the terminating `.catch(() => undefined)` in `reportApplicationError`, which is the reason every caller — a `catch` block or a React error boundary — can call the helper without handling anything. Nothing else in the tree covered that path after the prune. Recovered verbatim from 8559952f5e (lines 81-89 of that revision), not rewritten. It never used the resolving helper, so the vacuous fixture does not come back with it; the `afterEach` restore and the two imports do, because the case needs them and the prune had taken them as dead. NOT VACUOUS, watched rather than assumed: with `.catch(() => undefined)` removed from the helper, `even a real NETWORK failure is swallowed by the terminating catch` fails with `AssertionError: expected 'rejected' to be 'fulfilled'`, and the other five cases in the file stay green — so the mutation is attributed to this case and not to the file. Restored, the suite is 6 passed (6). The file's header comment is updated to say what is gone and what survived, since it previously claimed the whole block was removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 21:26:33 -05:00
Zachary Lowden	3a60f1151c	App Blocks: denylist-only pass-through arm on the orchestrator bridge (#4909) ... * feat(app-blocks): denylist-only pass-through arm on the orchestrator bridge Adds a second arm to the `kind: 'step'` wire member: `{ kind:'step', $type, input, maxBuzz }`. An app names an orchestrator step type directly and its native `input` is forwarded unmodified; the only thing between the app and the orchestrator is `PLATFORM_INTERNAL_STEP_TYPES`. Before this, `blockStepBodySchema` gated `step` against `REGISTERED_STEP_IDS` (two entries: convert-image, chat-completion) against ~50 live orchestrator step types, and `assertStepTypeAllowed` ran inside `buildBlockStep` on a type a registry entry had already declared — so the denylist only ever saw an already-closed set. Wire shape. The two arms share the `kind` discriminant, so the member is a nested discriminated union on `step` itself: the registry id on one arm, `z.undefined()` on the other. Three alternatives were run against zod 4.0.17 rather than reasoned about, and the two obvious ones both fail — see the comment above `blockWorkflowBodySchema`. `textToImage` and `customComfy` are untouched. Spend. Reuses the inline-comfy mechanism rather than inventing a third: the app declares one `maxBuzz`, the server derives `stepTimeoutSeconds = maxBuzz` and stamps it as the step timeout (the physical per-job Buzz cap), reserves `max(maxBuzz, whatIf quote)` on the per-user, per-app, consent and dev-session counters, and settles to actual on the terminal poll through the existing `persistCustomComfySettle` record. An unquotable `$type` reserves the declared ceiling instead of refusing. Outputs. Image blobs are lifted out of the forwarded output and pushed onto `imageUrls` and `AppWorkflow.images` -- the one channel the publish path and the per-viewer gated read already own -- so a pass-through image cannot reach a viewer around `getBlockGatedImagesByIds`. The rest of the output is forwarded verbatim on a new optional `stepOutputs` snapshot field. No prompt audit, no AIR scan and no entitlement check on this arm, by explicit operator decision. See the PR body for the two consequences worth a reviewer's attention. * fix(app-blocks): review round on the pass-through arm Five review lanes over the first commit. Four substantive changes, two corrections of claims the code made about itself, and one finding kept as a flag rather than a fix. Blob extraction is a SHAPE test, not a key list. The first draft enumerated four property names; measured against the live orchestrator spec those cover 4 of the 19 names that carry a blob across the reachable types. `video`, `audioBlob`, `svg`, `frames`, `tempBlobs`, `draftCache`, `additionalVideos` and seven on polyGen alone would each have ridden out as a raw url inside the forwarded output -- the second image channel the splitter exists to prevent, on ~40% of the media-producing set. It now tests the orchestrator `Blob` shape, plus a top-level case for types like `transcode` whose whole output is a blob. Depth 1 is a stated and tested limit. Extraction is gated on the server-stamped step name, not on "the $type is unrecognised". Those are different sets: the second forwards the output of any step the ORCHESTRATOR put on a workflow, on textToImage and customComfy workflows too. With the name gate the pre-existing "still skips an unregistered, non-native $type" case is restored verbatim rather than inverted, and a sibling pins the other side. The orchestrator tag slot takes a constant. The first draft passed the app-supplied `$type` into the array that also carries `app-block:<appId>` -- the tag the app-scoping guards read -- and defended it with an unmeasured claim about orchestrator rejection. The gated-image test was reconnected: it hand-built an Image row and would have passed with the whole feature deleted. Its url now comes from the projection. Corrections to claims the code made about itself: - "a pass-through chatCompletion returns its prose unscanned" was FALSE. All three consumers look the registry entry up by $type before the pass-through branch, and chat-completion declares `chatCompletion`, so that case IS scanned. The real hazard is the shadowing flip, which is what the comments now describe. - the builder's docstring claimed the suite pins `input` "by reference identity, so a spread cannot pass". It does not and cannot -- a shallow spread is observationally identical and the mutant survives the whole suite. Recorded in place. - PASS_THROUGH_MAX_BUZZ's derivation claimed both arms are developer-only and therefore already clamped by DEV_BUZZ_BUDGET_CAP. Neither half is true; it is the binding bound. - the in-handler buzzBudget narrowing is defense-in-depth and unreachable through the router, measured. Labelled, and so is the test that would otherwise read as covering it. Also adds the price-check counters the duplication had dropped, and tests for the per-app denial exit, the audit-row dimension, the tag slot, the $type bounds, and a mixed native+foreign workflow. Kept as a flag, not a fix: `stepTimeoutSeconds = maxBuzz` bounds a GPU-second-metered step and nothing else, so for the per-unit-priced types this arm opens, an unquotable submit reserves maxBuzz and may bill more. See the PR. * fix(app-blocks): second review round — two real leaks in the output splitter Delta re-review of the fix commit. Two of the findings are live defects the first round's fix introduced, and both are in the one function whose stated job is "no second image channel". 1. A blob LIST leaked every url when any element was not blob-shaped. The predicate used `every`, and `Blob.url` is optional upstream -- so one blocked or not-yet-available sibling disqualified the whole array, left the key unstripped, and forwarded every OTHER element's raw url through `stepOutputs`. Measured. Now `some` + filter: a partly-blob list lifts the conforming elements and the key is stripped either way. 2. A TOP-LEVEL blob list was forwarded whole. The array short-circuit ran before the top-level blob case, so a type whose entire output is a blob list had the same shape as `transcode` and no handling. Both now have tests, and both the `every`->`some` and the drop-the-top-level mutants die. Also from the round: - The submit phase filed its missing-quote outcome under `estimate_absent`. That label is defined as "before any spend exists", so the ONE event with money behind it -- a submit reserving at the declared ceiling because no quote was had -- landed in the no-spend bucket, blended with estimate traffic. The phase is threaded now; the submit records `absent`. - The router's own fake orchestrator reply carried no `name`, so since the name gate landed the router's `snapshotFromWorkflow(submitted)` had been dropping the step on every test in the describe and nothing noticed. The fake now echoes what the real orchestrator echoes, the submit asserts the stamped name, and stamping a different one kills 29 tests. - The settle seam is joined on the Redis key: both persist and settle run for real against one key-addressed store, so a wrong workflow id or a wrong cap key in the persisted record now fails. - Comment corrections the delta made necessary: the "images IS POSTURE-GATED" safety argument at `queryAppWorkflows` and `publishGenerationOutputs` no longer covers the whole array; `detail.step`'s "never client text" contract; and the schema's claim that the router hands over "this same object" (zod rebuilds the top level, so it never could). - The in-handler buzzBudget copy gets a distinct message, so the test pins WHICH gate refused instead of resting on a comment. - Vacuous assertions removed or given controls: the dev-session leg is now driven for real (reserve, deny + refund, and the settle record), the per-app denial asserts the idempotency release, the price-check test pins the emit count, and the gated-image fixture derives width/height from the projection with unequal values. - The 19-key `it.each` collapses to two representatives: the key list left the implementation last round, so the other 17 cases pinned nothing. * fix(app-blocks): third review round — the depth-1 walk missed two live types The output walk is recursive now, bounded at depth 4. A depth-1 walk shipped in the last round and was then measured against the live spec: of the 50 step types, three carry blobs deeper than the top level and TWO ARE ALLOWED on this arm. `polyGen.basicAnimations` is a plain object holding six `Model3DBlob`s, and `training.epochs[]` carries a `model` plus a `samples[]` of media. Every one of those urls was forwarded raw -- reachable by the app, invisible to the publish path and to the per-viewer gated read, i.e. the second image channel the splitter exists to prevent. Worse than the omission: the round-2 comment offered a list of nineteen property names as the measurement that made the key-agnostic predicate trustworthy, and `basicAnimations` was IN that list -- as a blob key, which it is not. The list read as coverage of the thing it missed. The blob predicate keys on `available` + an identity field rather than `available` + `url`. `Blob.url` is optional upstream -- which is the premise the array rule rests on -- so a BLOCKED blob whose `url` key is simply absent failed the shape test and was forwarded whole: no url escaped, but its `blockedReason` and raw `nsfwLevel` did, and the module's "a dropped blob cannot ride out through `rest`" claim was false. Metric polarity. `absent` is documented at the counter as "the submit is REFUSED, nothing is reserved" -- the opposite of what this arm does, which is reserve the declared ceiling and proceed. The counter now documents the third site and its inverted polarity, and the pass-through submit emits a PAIR (`quoted` / `absent`) so `absent` has a denominator: without one it falls when submit volume falls, which reads as healthy. Test fixes, each of which was a fix from the last round passing for the wrong reason: - The router's fake orchestrator HARDCODED the echoed step name instead of reading it off the submitted body, so the read side was fed the right value independently of what the router stamped. It echoes `opts` now. The "29 tests" figure in the last commit message was wrong for that reason: a repo-wide edit of that literal also hit the registry arm. The honest number is one test, and it is now a real join. - The unconditional strip had no control at the whole-value site; `contentRating` was asserted by calling the implementation's own helper on a value whose answer is also the fail-closed default; both dev-session tests sat on `ceiling === maxBuzz` so the reserved amount was unpinned; the dev denial's idempotency release was untested; and the settle seam joined `buzzCapKey` but not `appSpendKey`. Battery re-run: every mutant above dies, including the nine this round named. * fix(app-blocks): fourth review round — the array rule and the recursion disagreed The `some`-qualifies-the-whole-array rule from round 2 and the recursive walk from round 3 interact in a way neither round's tests covered: `[blob, { nested: blob }]` lifted the first element and discarded the second from BOTH sides -- never published, never forwarded. A silent media LOSS on the arm the app paid for, and the one shape that needed both rules to be wrong at once. The walk now lifts PER ELEMENT and recurses into the rest, so a non-blob sibling -- and anything inside it -- survives. Consequence on the wire: an array that held blobs comes back EMPTIED rather than removed, which is also the more faithful reading of "forward everything except media". Two mutants that survived the round-3 suite, both of them that round's own headline claim being the half the tests could not see: - `isOrchestratorBlobLike` -> `'available' in v` survived the whole battery. Both existing controls guard the `available` conjunct; nothing guarded the identity conjunct, so every object carrying an `available` key could be deleted from the forwarded output undetected. - `PASS_THROUGH_OUTPUT_WALK_DEPTH` 4 -> 3 survived: the cap was pinned to the interval [3,4], not to 4. The `epochs` fixture needs >=3 and the too-deep one needs <=4. A blob at exactly level 4 is the fixture that separates them. The metric fix landed in the TypeScript docblock and not in the Prometheus HELP text -- the string an on-call actually reads in Grafana. It still carried the triage instruction the last round identified as INVERTED for this arm ("the submit was refused, no spend, no generation") and did not mention `quoted` at all. Both corrected, along with the "closed 6-value set" and "step is drawn from the registry keys" cardinality notes, which the last round desynchronised. Also: the dev-session fix from round 3 traded one fixture collision for its mirror -- both tests moved to quote 31 where `ceiling === quotedBuzz`, so the opposite mutant survived. One is back at quote 5. The dev-session leg's refund on a submit throw was untested; `appSpendKey` in the settle seam was asserted against a restated literal rather than the returned key. Documented rather than fixed, because closing it is a decision: the "no second image channel" property holds for blob-SHAPED values, and two allowed types -- `blobArchive` and `imageResourceTraining.epochs[].blobUrl` -- carry their url as a plain string. The measurement behind the splitter enumerated blob-TYPED fields, and a reader takes that for the whole population. Both docblocks now say so. * fix(app-blocks): fifth review round — the array branch was pinned for objects only Two survivors of the round-4 change, both on the array branch it introduced: - `depth + 1` -> `depth` in the array branch survived the whole suite. Both boundary fixtures were all-object chains, so the array branch's own increment was unpinned; without it a deep array chain overflows the stack, which is the property the cap exists for. - forwarding an array-valued element verbatim, never descending, also survived. Every array fixture held objects or primitives, so `{ frames: [[blob]] }` sent the url straight out through `stepOutputs`. Both now have fixtures with arrays on the path. Same fixture-constant collision as the last two rounds, one leg further along: the dev-session REFUND test ran at quote 31, where the ceiling and the quote are the same number, so `cost: ceiling` -> `cost: quotedBuzz` survived. Added the quote-5 mirror, plus an emit count so a doubled refund cannot pass. The settle record's `ceiling` had the same gap and now has a quote-5 case. Doc corrections, each one a claim the previous round's own edit falsified: - The depth cap's stated reason was "the input is app-supplied and only size-capped: without it a cyclic or adversarially deep payload walks forever". False, and contradicted by a paragraph twenty lines below it in the same file: the value walked is the ORCHESTRATOR'S `step.output`, which the client `JSON.parse`s -- acyclic and already depth-bounded. It is a walk-COST bound over a response whose shape is open by construction, and the residue is its price. - The Prometheus HELP text, corrected last round for being inverted, had been replaced with a different unconditional claim: "the generation RAN". `absent` fires inside the quote, before every cap and before the real submit, so an orchestrator that rejects the `$type` outright also lands there. Narrowed to "not refused FOR LACK OF A QUOTE; a generation MAY have run", with the discriminator named. - The registry arm's closed-set test had `quoted` added to its allowlist last round -- a value that arm cannot legitimately emit, and whose whole purpose is that the two arms' `absent` mean opposite things. Reverted to its own six. - `a caller cannot invent a fourth value` (the union is seven); the `epochs needs >=3` rationale (stale the moment the walk became per-element); the `appSpendKey` "derivation" justification (the mock's key is itself a constant, so it buys nothing the literal did not -- said so rather than implying otherwise); and a test comment still naming a variable the round-4 commit deleted. Two orchestrator-side questions this round raised are NOT in any file and are not code changes: they are in the PR discussion. * docs(app-blocks): sixth review round — the depth cap IS a stack bound Both round-6 lanes found no executable defect in the delta. Every finding was a claim in a comment, and the sharpest one was mine from the round before. Last round I replaced the depth cap's rationale with "the value walked is the orchestrator's `step.output`, which the client `JSON.parse`s, so it is acyclic and already depth-bounded by that parse", and closed with "removing the cap closes the residue outright". The second half of that is an invitation to a crash. Measured on this repo's pinned node 24.19.0: `JSON.parse` accepts 200,000 levels of nesting, arrays and objects alike, while an unbounded recursive walk of the parsed result overflows the stack between 1,000 and 2,000. The parse is two orders of magnitude looser than the walk, so the cap is the only thing bounding the recursion. That matters because `splitPassThroughStepOutput` runs inside the pass-through submit's post-submit path, where a throw is caught by a handler that refunds every cap leg and rethrows -- on a generation that has already been created and will bill. A `RangeError` there moves money in the wrong direction. Closing the residue means an iterative walk, not deleting the cap. Both numbers are in the docblock now instead of the reasoning, because the reasoning has been wrong twice in opposite directions. Two other claims, same shape: - The `recordStepPriceCheck` docstring still carried the unconditional "a generation RAN" that the HELP text was narrowed away from last round -- the developer-facing copy and the operator-facing copy of one sentence, and only one had been fixed. `absent` fires inside the quote, ahead of the static gate, all three reservations and the real submit, so an orchestrator rejecting the `$type` outright lands there with nothing having run. - A test comment still stated the repudiated "the input is app-supplied and only size-capped" rationale, and its sibling attributed the deep-array hazard to an app payload when the walked value is the orchestrator's response. Both now carry the measurement instead. * docs(app-blocks): stop stating the stack-overflow depth — it is not a stable number Rounds 5, 6 and 7 each corrected this paragraph's figure and each correction was falsified by the next. Four independent measurements of "the depth at which an unbounded recursive walk overflows" disagreed by more than 2x -- 1,669 / 3,050 / 3,383 / 3,418 on the same pinned node, depending on stack size, frame shape, caller depth and whether the probe forked per depth. The quantity has no stable value, so the instruction "say the number when you re-measure" -- correct for the CATALOG-depth measurement, which is a fact about the orchestrator spec -- is what generated three wrong numbers when applied to this one. The docblock now states the ORDER (JSON parses iteratively and takes ten million levels; the walk is recursive and goes a few thousand), says explicitly not to write a number there again, and keeps the paragraph that earns its length: do not "close the residue" by deleting the cap, because this runs on the post-submit path where a throw refunds all three cap legs on a generation that already exists and will bill. Also removes the two duplicate copies of that measurement from the test file -- the duplication was the generator, and one of them was a banner restating the constant's docblock with no claim about the test under it. The surviving comment names the MECHANISM instead (without the array increment the cap never fires at all), which is both truer and not a moving target. Two more copies of the `absent` sentence, found by the same sweep that found the first two: - `quotePassThroughBuzz`'s `phase` docblock still said this arm's `absent` means "it ran, it did not refuse" -- and cited the counter docstring that the last commit changed to say the opposite. Third copy of one sentence; corrected. - the HELP text and the docstring both told an operator to read `absent` against "the submit-failure signal", which names no series in this module. Replaced with what is true: this counter does not separate the two causes, and the workflow status is what does. * docs(app-blocks): remove the generators, not just the wrong claims Eighth round, comments only again, and every finding was the previous round's own fix. - "four independent measurements in this file's history disagreed by more than 2x" -- the file's whole history contains exactly ONE overflow figure; the four lived in commit messages. A maintainer following that sentence finds one number and reads the prohibition as overstated. It is also a figure about figures, and it went stale while it was being reviewed (a fifth measurement came in at 4,096). Dropped the count and the locus. - The paragraph whose entire lesson is that numbers here rot reintroduced one ten lines later: "refunds all three cap reservations". It replaced a count-free true statement, and it is wrong -- `refundBlockBuzzReservation` refunds two keys, so four counters are involved, and two of the three legs are conditional so the real count ranges 1-4. Another comment in the same function already says four. Back to "every cap leg it reserved". - Two opposite instructions about "a number" sat ten lines apart in one docblock, both anaphoric: "say the number when you re-measure" (the catalog depth -- a stable fact about the orchestrator spec) and "do not write a number here again" (the overflow threshold). Each now names its quantity. - The triage pointer that replaced "the submit-failure signal" named no series either: there is no workflow-status metric anywhere in `src`, and in the very cause it must identify -- the orchestrator rejecting the `$type` -- the submit rethrows and no workflow exists to have a status. Kept the limit, dropped the pointer: only whether a submit produced a workflow separates the two causes, and nothing here carries that. - The `absent` polarity fact was stated in four places, and the fourth asserted what another comment says rather than a behaviour -- the exact coupling that broke in rounds 6 and 7. Trimmed to its own fact, so the cross-reference goes with it. Six rounds of this ladder were spent on one paragraph. What ended it was not a more careful number; it was deleting the number, the count of numbers, and the cross-reference that made a third file's edit able to falsify this one. * docs(app-blocks): drop the last counts and the last self-justifying aside Ninth round found no false claim and no executable defect. Three trims, all on text the eighth round introduced: - The parenthetical defending the docblock's two "number" instructions against being read as contradictory. The same commit's fix -- each instruction naming its quantity -- already removes the contradiction, so it defends against nothing; and it called the catalog depth "stable", which contradicts the sentence three lines above it ("a new upstream $type is ALLOWED by construction, so one extra wrapper level reopens this leak"). What actually distinguishes that number is that it is re-derivable from the spec, which is what it says now. - "no series HERE carries that" works in the docblock, where "NOTHING IN THIS MODULE" sets it up two lines earlier. It does not work in the Prometheus help string, which an operator reads in a metric browser with no module in view. Both now say "no App Block series", which is stronger and still true. - The last two counts of the same reservation legs: one comment said "all three reservations" and another, in the same function, said "all four counters". Both are true readings of different things (numbered sites vs refunded keys) and two of the legs are conditional, so neither number holds generally. Both are count-free now. * docs(app-blocks): there IS a series that separates the two `absent` causes Round 9 widened "no series HERE carries that" to "no App Block series carries that" to fix an antecedent, and in doing so made a defensible sentence false in both of its homes. Verified against the code: the pass-through submit persists a settle record only after a workflow exists, and the terminal settle emits `civitai_app_block_customcomfy_wallclock_seconds{engine="passthrough"}` independently of the accrued cost -- declared in the same module the comment said carried nothing. The two `absent` causes differ exactly in whether such a sample can exist: the ran-at-the-declared-ceiling case persists a record, the orchestrator-rejected-`$type` case throws before the persist. So the clause now names that series, with its limits stated (it needs a terminal observation, drops a wallclock past its top bucket, and has no per-event join to the counter). An authoritative "there is no signal" is worse than no note at all: it sends a triaging operator away from the one series that answers the question. Also corrects a comment my own change made stale two files over: the two `customcomfy_*` histograms' cardinality argument rested on their labels being "enum-resolved from the recipe registry". This arm feeds them `passthrough` / `__passthrough__`, which come from no registry -- they are constants precisely because the pass-through `$type` set is open by construction. The bound still holds; the stated reason did not, and that staleness is what made this round's false claim easy to write. * docs(app-blocks): 240 is the top bucket, 600 is the drop threshold Three corrections to the clause the previous commit added, and this is the last of them. - "drops a wallclock over its top bucket" conflated two numbers the same file distinguishes 475 lines above: the top wallclock bucket is 240s, the drop threshold is MAX_CUSTOMCOMFY_WALLCLOCK_SECONDS = 600s, and a 300s sample IS observed in +Inf. The false version makes the lower bound sound looser than it is and hides the number an operator needs -- 600s is reachable on this arm via queue-wait plus the terminal-poll gap even though the step timeout is <=180s. - Deleted the provenance sentence recording that two earlier drafts of the same clause said no series carried it. That is change-log narration and reviewer-justification in one line; the clause now names the series, which is the whole protection. The history belongs in these messages, which is where it is. - "Anything that adds a post-paid arm adds one pair here" is true of the two constant-label arms enumerated above it and false of the next addition the module itself anticipates: flipping postPaidSettle on a registry step entry feeds the resolved variant and the step id, so it adds one pair per (step id x variant). A future editor sizing that change off the old sentence under-counts. * fix(blocks): case-fold the platform-internal denylist — 'XGuardModeration' was allowed On the pass-through `kind:'step'` arm this denylist is the ONLY control, and the value it guards is a caller-supplied string forwarded verbatim to an orchestrator this repo does not own. The lookup was a plain `Set.has(stepType)` — exact and case-sensitive — so `assertStepTypeAllowed('XGuardModeration')` returned ALLOWED while the live spec's key is `xGuardModeration`. Whether the orchestrator matches `$type` case-insensitively is not knowable from here and is not ours to assume in either direction, so the control has to hold for BOTH answers. If it does match case-insensitively, this was a complete bypass of the denylist on the one arm where nothing else stands behind it. Watched RED before the fix, and not by assertion: the six re-casing cases plus the error-message case failed 7/7 against the unfolded lookup while the negative control passed, so the bypass was demonstrated rather than argued. After the fold, 20/20. Mutating the probe back to the unfolded form fails 12 — wider than 7, because the fold has to be on BOTH the set and the probe to be coherent, which is the property worth pinning. Safe by measurement rather than by hope: all 50 `$type` keys in the live `WorkflowStepTemplate.discriminator.mapping` are distinct when lowercased (measured 2026-09-17 — 50 keys, 50 distinct-lowercased), so folding case cannot make an ALLOWED type collide with a denied one. It denies a strict superset of what it denied before. The docstring records that measurement and says to re-take it before adding an entry whose lowercasing could collide. The refusal still names the caller's own spelling, not the canonical one — a guard that silently rewrites its input is a worse diagnostic than one that quotes it, and there is a test for that.	2026-09-17 21:02:21 -05:00
Koen	5613c79d72	feat(feed): serve withMeta and fromPlatform from the feed service (#4920) ... Both are single bits the feed already filters on; they were unmapped only because the flags behind them were a stale one-off load. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-18 03:01:59 +01:00
briant	473b692093	5.1.110 v5.1.110	2026-09-17 19:41:32 -06:00
briant	0baf7e313a	fix(generation): keep Pony, Illustrious and NoobAI on sdcpp by default ... #4746 grouped the SDXL derivatives with Flux1/FluxKrea as comfy-only, forcing them onto comfyui with no toggle. They belong with SDXL: the enhancedCompatibility toggle is back (off = sdcpp, on = comfyui), and they rejoin the 2-for-1 quantity bonus, which derives from the toggle list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 19:39:04 -06:00
Justin Maier	6cb780eb23	fix(buzz): let the reconcile confirmation survive the list repopulating (#4924) ... * fix(buzz): let the reconcile confirmation survive the list repopulating A successful 'Check now' invalidates getDepositHistory, the list stops being empty, and the empty-state branch unmounts. The mutation lived in CheckDepositsNotice, so its success state went down with the branch: 'Found N deposit(s)!' was destroyed in the same tick it was produced and the user was left looking at an idle 'Check now'. That happened on exactly the flow the control exists for. Move the mutation into a hook called by DepositHistory, which spans both branches and does not unmount, and pass the one instance down. The mutation, its scoping and its rate limit are unchanged. Closes ClickUp 868m6j63r. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(buzz): drop a reconcile result once the deposit list moves under it Owning the mutation above the branch fixed the confirmation vanishing, and extended every other terminal state by the same amount. A 'No missing deposits found' does not invalidate anything, so it would sit unchanged while a deposit arrived from the signal and rendered above it. Reset the mutation when the total changes, unless it found something -- that result stays true, and clearing it would undo the invalidation it just caused. Keyed on a previous-total ref rather than on the terminal state, so a zero result is still shown at the moment it arrives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(buzz): stamp the deposit total at click time instead of syncing it in an effect Replaces the previous-total ref and its effect. That version updated the ref before testing the mutation state, so a deposit arriving WHILE the request was in flight consumed the comparison: the label then never went stale, in the one window the user is most likely to be in. It also reset a mutation mid-flight if the condition were ever widened, where disabled/loading read isPending alone. Stamp the total in onMutate, which runs at click time and cannot be outrun by the request, and derive staleness in render. No effect, no ref sync, nothing to order. A found-result stays exempt: it is the one result that always moves the list, because it invalidates the query itself, so treating it as stale would wipe the confirmation with its own refetch. Also close a hole in the guard. It counted occurrences of the text ".useMutation", which extracting the hook had made one by construction - a branch calling the hook itself still counted 1 while fully restoring the bug. It counts invocations now, and names the branches that must not call it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(buzz): move the hook call below the total it now reads The previous commit gave useReconcileDeposits an argument but left the call where it was, 33 lines above the declaration of "total". Same scope, const, so every render of DepositHistory raised ReferenceError: Cannot access 'total' before initialization. The deposit history was dead, not degraded, on a money surface. Typecheck reports it as TS2448; the source guard passed over it in 483ms, which is the clearest statement of what that guard does and does not see. Extract the staleness rule into isResultStale and table-test it, so the "!found" term is pinned by something that runs in CI rather than by a browser test that no CI job runs. The rule moved rather than being copied - a second definition beside the test would change with it and check nothing. Correct the exemption comment again. A found-result does NOT always move the list: reconciling a deposit already listed as Confirming credits an existing row via upsert and adds none. The exemption is still load-bearing for the normal case; the absolute was not true. Also pin two things the guard was not seeing: that the hook stays module-private, which is what makes scanning one file sound, and that the total is stamped in onMutate rather than in render. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * style: prettier the new test files --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 19:16:38 -06:00
Zachary Lowden	7b81489daf	fix(db): re-predicate Post_blockPublishedAppId_idx so the sweep query can use it (#4923) ... The index from 20260913120000 is valid, tiny, and unusable by the query its own header documents. Measured on the production primary with the index live: WHERE metadata->>'blockPublishedAppId' = $1 -> Parallel Seq Scan on "Post", cost 962412 WHERE metadata ? 'blockPublishedAppId' AND metadata->>'blockPublishedAppId' = $1 -> Index Scan using "Post_blockPublishedAppId_idx", cost 2.34 That is the exact full scan the index was created to prevent, ~411,000x apart. It is a provability limit rather than a costing preference: with enable_seqscan off the planner marks the sequential scan Disabled and uses it anyway, so there is no viable index alternative. Postgres cannot derive "metadata ? k" from "metadata->>k = v" - semantically equivalent, since ->> yields NULL for an absent key and NULL = x is never true, but predicate_implied_by reasons over operator rules and knows no such relationship between those two jsonb operators. Fixed structurally rather than by comment. "(metadata->>k) IS NOT NULL" IS provable from "metadata->>k = v" - measured with both variants present, where the planner selects the IS NOT NULL one and cannot use the ? one - so re-predicating makes the obvious query the fast one. Documenting "remember to restate the predicate" would leave a guard someone has to read and obey, and the moment it exists for is an incident, which is the worst moment to be relying on that. Semantic delta runs the right way: ? matches a key present with a JSON null value and IS NOT NULL does not, but the server writes a non-null OauthClient id unconditionally and a sweep matching = $1 could never return such a row anyway. Drop-then-create is safe here specifically because the index covers zero rows and has no code reader yet - nothing in src/ queries the marker, the sweep is run by hand. The header says what to do instead if it ever becomes load-bearing. The superseded migration keeps its reasoning but gains a banner, because its bottom line is a query nobody should copy. Applied by hand per the repo's migration policy. The header carries the statement_timeout trap (300000 ms on this cluster, and a killed CONCURRENTLY build leaves an INVALID index) and the regclass-quoting trap that makes the obvious "is it there?" check silently match nothing. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 19:30:25 -05:00
briant	e1c65a602b	5.1.109 v5.1.109	2026-09-17 16:54:26 -06:00
briant	e90aabeb98	feat(seo): give tags a display name, and fix model and comic page meta ... Tag names are stored lowercase, so the tag page rendered "Lora AI Models" for our largest tag. Tag.displayName carries the casing people recognise; the page capitalises the name when it has none, which is right for almost every tag. 6,207 tags were seeded from the base model and ecosystem names. The column rather than a lookup table, because base models and ecosystems are moving into the database, and because casing a capitalise-fallback cannot reach ("3d" -> "3D") is a property of the tag, not of any model list. Also: /models gets a title and description aimed at non-brand queries, and a comic that green cannot show no longer serves an indexable "not available" page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-17 16:48:33 -06:00
Justin Maier	1f3e70fbbe	fix(creator-shop): stop a pack save re-snapshotting member payouts, and give moderators a pack edit path (#4910) ... * feat(cosmetic-store): give moderators an edit path for pack products A pack is a CosmeticShopItem with cosmeticId null, and the generic product form requires one, so packs were given no Edit control at all rather than a form that fails on every save. Route a pack row to CreatorShopPackModal instead, which is the editor the creator manage page already uses for them. The save path needed no change: updateCreatorShopPack already accepts a moderator, and it computes ownership, the resale check and the price floor against the pack's owner rather than the editor. updatePack now also invalidates the queries a pack edit makes stale outside the manage page - the moderator list, the creator storefront and the community hub. A price or contents edit drops the pack to PendingReview, so all three were showing a listing that is no longer on sale. Not covered: availableFrom, availableTo and archived, which the pack editor has no fields for. No pack in production has ever had any of the three set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(creator-shop): stop a pack save rewriting what its members' creators are paid The pack editor sent price and memberCosmeticIds on every save. Both are meant to be omitted when unchanged - the schema comment on memberCosmeticIds says so - and sending them unconditionally made a title-only edit: - re-snapshot every member's floorAmount, which computePackPayouts uses as the basis each member's creator is paid on, so a member repriced since the pack was built silently changed a third party's payout - rewrite meta.packMemberCount, which is the count assertPackPurchasable compares against precisely so it cannot shrink with the pack, disarming the guard that refuses a pack whose member has gone - drop the pack to PendingReview, taking a live listing off sale Send both only when they changed, and name any member the pack has lost so editing the contents no longer drops it without saying so. Two more the same editor carried, now reachable from the moderator list: the four scalars were seeded from the caller's row and never reconciled against getPack, and the query client runs at staleTime: Infinity, so a stale row could write an old price back over the creator's own change. Hydrate them from the server and keep Save disabled until it has answered. Rejected and archived packs are refused up front rather than at save. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(creator-shop): let a pack with an unavailable member still be title-edited Omitting memberCosmeticIds made the server fall back to the pack's stored member list, which includes members whose listing has since been archived, and then re-assert bundlability over it - so a title-only edit threw on exactly the packs unavailableCount exists to describe. Previously those saved, by silently pruning the missing member, which is the payout bug the previous commit fixed. The three asserts validate an INCOMING membership, so they now run only on one the caller supplied. Bundlability still applies when the price moves, because the floor it is checked against can only be summed over members that resolve. Also from review: a failed getPack left a filled-in form whose Save could never enable and which said nothing, and hydration silently overwrote anything typed before the query landed. Surface the load failure and keep the fields disabled until the server has answered. Measured before deciding: 0 of 40 production packs currently have an unresolvable member, so this was latent rather than live. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(creator-shop): stop a title-only pack edit failing on checks it never touched Three checks ran on every update regardless of what changed, so an edit that moved only the title was refused over a member's later, unrelated decision - the same "form that takes what you type and fails on save" this editor was added to remove. - the price floor is re-summed from TODAY's list prices, so a foreign member raising their listing blocked a typo fix, and the only way out was raising the pack's price, which re-snapshots every member and unlists it - the Blue Buzz check dead-ended it outright: the switch is disabled while blockers exist, so Save went grey with no way to clear it and an alert that explained blue rather than why - the Add button stayed live during the getPack window, and hydration replaces the selection rather than merging, so items added there vanished Both server checks now run only on the edit that makes them meaningful, and the four derivations of "this edit invalidates the stored floors" collapse to two named atoms. They used two truth functions that part company on an empty member list - reachable from anything that is not the zod-validated router - and split they would have run the checks while skipping the write. Resale consent is deliberately NOT re-checked on a price move: revoking sellableByOthers withdraws an offer from future resellers and was never a term of an existing listing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(creator-shop): let the Blue Buzz switch be un-ticked, and pin what the gates mean The switch was disabled whenever a member blocked blue, but a pack saved as blue-accepting hydrates it ON, and canSubmit refuses that combination - so the one control that clears the refusal was frozen and a title fix could not be saved at all. Un-tickable, never un-un-tickable. acceptsBlueBuzz now follows the same rule as price and memberCosmeticIds and is sent only when it changed. Without that the server-side scoping added last commit was inert for its only caller, which sent the field every time. The guard pinned that `reSnapshot` is USED but never what its operands mean: `const repriced = false` left every pinned spelling intact and silently reverted the whole price path - no floor check, no floorAmount re-snapshot, no drop to PendingReview. Measured green before this commit, red after. The floor gating itself was likewise unpinned. Also removes a comment of mine that was wrong: `[]` is truthy, so the two spellings of that condition never disagreed, and the collapse is a readability win rather than a fix. Three slices gain an empty-guard so a drifting marker fails naming the marker instead of blaming the code it was reading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(creator-shop): make the Blue Buzz toggle wait for hydration, and pin the decision The switch was the only interactive control in the modal without `awaitingPack`. Before getPack lands there are no members, so no blockers, so it was enabled - and a moderator who ticked it had the toggle overwritten by hydration. Since the field is now only sent when it differs from that same hydrated value, the change was then discarded silently. The guard could not see any of this. Pinned, each reproduced green first: - the `?? !!meta.acceptsBlueBuzz` fallback. `?? false` flips a blue-accepting pack to blue-declining on a TITLE edit, and it only became reachable because the previous commit made the payload conditional - the server-side scoping of the blue check, which is the other half of the invariant that payload change implements on the client - the floor comparison DIRECTION, for the same reason canSubmit pins its `&&` - the switch and the canSubmit term it exists to unblock, which are one invariant and were pinnable only as a pair - a positive for the blue payload spread, which had only a negative, so deleting it outright passed and made the setting permanently unsavable The hydration slice now covers the whole effect and pins all six seeds. Its comment claimed the cover and members were "pinned by their own cases"; they were pinned by nothing, which is the second comment of mine this branch has had to correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(creator-shop): pin the contentsChanged predicate, and say why Save is blocked Dropping the `!` from contentsChanged makes it true whenever a member is UNCHANGED, so the contents go on every title edit and every floorAmount is re-snapshotted - the money decision this guard exists to protect. Both pinned substrings survive that, so it was green; `.some` to `.every` was too. The predicate is now pinned whole and both mutations redden. The blue-buzz alert now says saving is blocked and what turning it off costs. On a pack stored blue-accepting whose member has since opted out, the client refuses a title-only save that the server would accept, so unticking is the only route to fixing a typo. Leaving that stricter-than-the-server behaviour as it is - the state is invalid against the invariant - but a greyed-out Save beside a message about members left the connection for the moderator to make. Scoping canSubmit to match the server exactly is a behaviour change; it is on the follow-up ticket instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(creator-shop): stop the client refusing edits the server would accept Three more of the same shape, found by the final review round. canSubmit refused a title fix on a pack that already accepts Blue Buzz whose member has since opted out - even though the field is not sent and the server never checks it, so turning Blue Buzz off on someone else's listing was the only route to fixing a typo. That is this ticket's own bug in miniature, and the previous commit had added copy EXPLAINING the block rather than removing it. Scoped to `blueChanged`, the predicate the payload and the server already use, and the copy is gone with it. The Remove button had none of the guard the Add button gained: hydration replaces the selection, so a removal made during the load window was silently undone. Same list, opposite button. uneditableStatus restated the rejected-vs-archived rule and dropped its history term, so a pack archived AFTER a rejection was told to "Restore it before editing" - which the server refuses as REJECTED_IS_FINAL. Uses the canonical wasLastReviewARejection, as the Manage list already does. Also: members are now resolved only for the edits that read them. Every consumer sits behind one of the scoping predicates, so a title-only edit was paying for a query - measured 1.1ms and a seq scan of every shop item - whose result nothing looked at. The comment claiming the resolve had to be unconditional was describing a floor check this branch had already moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(creator-shop): pin the rejected-vs-archived rule and the Remove button guard wasRejected was new code with no assertion at all, and it is what the commit that added it exists to fix: drop the history term and the alert tells a moderator to restore a pack the server refuses as REJECTED_IS_FINAL. Reddens now. The Remove button's hydration guard was pinned by a COUNT of `disabled={awaitingPack}` occurrences, which is worthless - there are six, so deleting the one being guarded still cleared the threshold. Measured green against exactly the mutation it was added for. Replaced with a slice of the Remove button itself. Left green, recorded on the follow-up rather than dropped: inverting the ternary that `needsMembers` guards, and the pre-existing `memberIds` fallback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 16:33:28 -06:00
Briant Diehl	e174d3ba00	Merge pull request #4925 from civitai/feat/generator-step-warnings ... feat(generation): surface orchestrator model-retirement warnings in t…	2026-09-17 16:05:30 -06:00
Luis Rojas	23c4e072d8	fix(training-studio): make tile videos actually play ... Reported by a dev alongside the tester feedback: dataset video tiles were static, and videos imported from the generator never played at all. Two causes. The DataStep tile and the generation picker rendered bare <video muted> — no loop/playsinline/preload and no play trigger, unlike the sample components that already hover-play correctly. And the generator import mapped every item to previewUrl, which for a video is a STILL thumbnail — a <video> pointed at a JPEG can never play. Video and audio imports now keep the real blob URL (preload="metadata" holds the cost to a first frame until hovered; images keep the resized preview), which also means video captioning receives the actual video, matching the upload path. The hover-play handlers existed as identical private copies in SampleImage and SampleGrid, and the two broken sites would have made four — extracted to $lib/video-preview.ts and shared by all four. Verified live: uploaded webm tile shows its first frame, plays on hover (currentTime advancing, unpaused), pauses and rewinds on leave. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DV3Ku4Eu9qTtzd61zt19dZ	2026-09-17 18:00:25 -04:00
briant	0069dd5d5d	feat(generation): surface orchestrator model-retirement warnings in the generator ... The orchestrator reports deprecation on a workflow step (`WorkflowStep.warnings`, `@civitai/orchestration-client` beta.106). whatIfFromGraph now carries them to the client, one per distinct code + message, omitted when there are none — so the notice reaches the user before they spend, not after the model stops working. Both generator footers render the message in the priority alert space, above the sdcpp branch: that branch always claims the slot (its MultiController decides internally whether to draw), so anything after it would never render. A warning therefore suppresses the 2-for-1 banner while a retiring model is selected, and the banner returns on the next estimate once the model changes. Message only. The warning names no resource, so there is nothing to tie it to a checkpoint row or a picker card without guessing which resource it means. A malformed payload skips the banner rather than rejecting the estimate — a rejected whatIf disables submit, which is not a price worth paying for a notice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-17 15:46:07 -06:00
Justin Maier	70de6e548a	fix(utils): decode entities on the first normalizeText call, and keep the decoder off the feed path (#4912) ... * fix(utils): decode HTML entities on the first normalizeText call normalizeText resolved `he` through a fire-and-forget dynamic import that assigned `decode` in a `.then` callback while returning an identity function in the meantime. `getHe()` is only reached for input containing `&`, so the first such input in a process was normalized without being decoded, and two callers normalizing the same input either side of an `await` could observe two different normalisations of it. Several audit paths do exactly that. Import `decode` statically instead. `he` is ~98KB of source, almost all entity table; the tradeoff is recorded in the file so the next reader does not have to re-derive it. The two tests are named for the decision rather than the mechanism, and both go red if the import is made lazy again. Verified by reverting the module and re-running the file: 2 failed, exit 1, each an assertion naming the wrong value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(utils): anchor the decode tests to values, not to each other The before/after-await case asserted only that the two reads agreed. A decoder that warms up after the wait leaves both reads undecoded, so parity holds and the case went green on a variant worse than the one being fixed. Both reads are now anchored to the decoded value. Adds a third case pinning what is specific to this module rather than to `he`: decoding runs before the accent fold, covers numeric and hex references, and is single-pass. Trims the module comment to the load-bearing facts and stops the test header from re-deriving the same rationale. Mutation results, one file each, exit codes read from log files: - lazy import restored: 3 failed (3), exit 1 - warm-up delayed past the wait: 3 failed (3), exit 1 (previously 1 green) - `he` swapped for a compact `&amp;`-only decoder: 1 failed | 2 passed (3) - decode and fold swapped: 1 failed | 2 passed (3) - as committed: 3 passed (3), exit 0 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(utils): pin the absent-input branch and say what the post-await read catches Adds the one unpinned branch in the function: absent input must come back as an empty string, not as the value it was handed. Removing that guard reddens the new case alone (`expected undefined to be ''`) and nothing else in the file. Rewrites two comments that pointed at the wrong thing. The post-await read was described as not independently protective; it is the only assertion here that catches a decoder correct on the first call and wrong later, which is the shape bundle pressure takes once lazy loading is closed off. Measured: an eviction mutant (correct on load, identity after 5ms) reddens that case and passes the other three. Also notes why the fourth case keeps its inputs in one instance, since splitting them would let a memoizing decoder satisfy each in turn. Mutation results, one file per run, exit codes read from log files: - as committed: 4 passed (4), exit 0 - lazy import restored: 3 failed | 1 passed (4), exit 1 - decoder evicted 5ms after load: 1 failed | 3 passed (4), exit 1 - absent-input guard removed: 1 failed | 3 passed (4), exit 1 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(utils): keep the entity decoder out of the chunk the feed loads `hasNsfwWords` was the only thing putting `he` in the shared client chunk, and it never needed the entity decode: every one of its eight callers passes a title or a name, and those render as text, so a stored `&eacute;` displays literally and decoding it would make the check disagree with what the reader sees. Splits the accent fold into its own dependency-free module. `normalizeText` composes that with `he` for the prompt paths, so the fold has one definition and the decode has one definition. Nothing about how prompts are handled changes. Measured on the prod replica across the four surfaces `hasNsfwWords` is called on - Model.name, Article.title, Post.title, Tag.name - 5 rows of 25.7M contain any HTML entity, and all five are `&amp;`, which cannot change a word match. The bare-ampersand counts in the same query are in the tens of thousands, so the zero is a real absence rather than a filter that could not match. Guard test, with both controls run: - decoder import re-added to audit-base: 1 failed | 1 passed (2), exit 1 - accent fold removed: 2 failed | 4 passed (6), exit 1 - as committed: 6 passed (6), exit 0 The guard reads source text, so it catches a direct re-import only, not a decoder arriving transitively. That limit is written into the test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(utils): harden the fold-only guard and trim the comments Behaviour of the shipped code is unchanged. Two review findings on the guard, both of the shape "this test can stop working without going red": The guard blessed utils/fold-diacritics.ts by name and constrained nothing about it, so the likeliest reintroduction - teaching that helper to decode entities so the two stop disagreeing - left every case green. It now asserts that module imports nothing at all. The guard matched prose, so a double-quoted import, a require, a bare import() or a deep path all evaded it. It now extracts import specifiers and compares them, which also removes its dependence on stripping comment lines. The accent fixture picked a word whose first `e` could be word-final, and prepareWordRegex ends every expression with a not-followed-by-alphanumeric lookahead that a trailing combining mark satisfies. Such a pick would have passed with the fold removed. It now selects an `e` followed by a letter and asserts the mark did not land word-final, so a word-list edit that breaks the property fails loudly instead of quietly. Adds the unaccented word as a legibility control and splits the negative control into its own case. Comments trimmed to the edits they protect: the prod row counts move daily and belong in the commit record, not in the file, and the size fact now has one copy. New controls, one file per run, exit codes read from log files: - foldDiacritics taught to decode: 1 failed | 3 passed (4), exit 1 (was green) - audit-base importing `he` with double quotes: 1 failed | 3 passed (4), exit 1 - accent fold removed: 1 failed | 3 passed (4), exit 1 - as committed: 8 passed (8), exit 0 The word-final-fixture assertion has no control of its own; it guards against a future data change, not against a mutation of this tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 15:38:08 -06:00
Justin Maier	19bb665f45	feat(hubs): allow moderation tags as hub sources, both directions (#4919) ... * feat(hubs): allow moderation tags as hub sources, both directions Reverses a deliberate decision rather than filling a gap. HUB_TAG_SOURCE_FILTER excluded Moderation and System in both directions — Justin's call, 2026-09-04, recorded in the constant's own comment, on the reasoning that the browsing level already enforces it. It does not. The browsing level is a coarse nsfwLevel bitmask ANDed across the whole feed, so it cannot express "keep this band but drop images tagged sexy". That is a per-tag exclude, and it was the only thing a hub had no way to say. Justin reversed the call on 2026-09-17, for both include and exclude. System tags stay out on arithmetic rather than judgement: both System image tags are `unlisted`, and unlisted rows are dropped by a separate clause in getTags and in hubTagWhere, so listing the type would offer 0 of 2. Moderation is 51 of 53 by that same count. Measured against prod 2026-09-17. hub-moderation-tag-vocabulary.test.ts is named for the decision so that the next review recommending a narrower vocabulary finds the argument rather than the conclusion. Closes 868m67cqe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(hubs): answer the half of the old rationale the reversal did not refute Review round on #4919. Three defects, all in the record rather than the behaviour, plus one guard that could not fail. The 2026-09-04 call gave two reasons for keeping moderation tags out. The reversal comment rebutted only the first (the browsing level already enforces it — it does not, being a coarse bitmask ANDed across the feed). The second — that a tag exclude is a WEAKER control than the level, so offering it may leave a user believing they set something stronger — is still true, and nothing here makes it false. It was overridden, not refuted, and the comment now says so and names the trade: a cap the user cannot aim is worse than a filter they can. A future reviewer who reaches for that argument now finds an answer instead of silence. The picker's own comment still asserted the pre-reversal rule ("not the moderation or system vocabularies") one file from the constant that reverses it. It was the first thing a reader would hit when asking whether moderation tags are pickable. The two Moderation tags the 51-of-53 count leaves out are now named and characterised — `self injury` and `extremist`, both `unlisted`, not a carve-out — so the gap cannot read as a deliberate exception nobody documented. The vocabulary rationale and the entityType warning were two adjacent docblocks, so only the second attached as the constant's JSDoc and hover hid the argument. Merged. The picker guard asserted the source CONTAINS the spread and does not retype the array as a literal. Both pass over `[...HUB_TAG_SOURCE_FILTER.types].filter((t) => t !== TagType.Moderation)`, which re-narrows the client to exactly the pre-reversal vocabulary — the plausible regression, since whoever narrows it edits the spread line rather than retyping the array. Added a reshape assertion and verified all three against mutants: the real file passes; .filter(), .slice() and a retyped literal each fail. The guard also resolved its path from process.cwd(), so a run from another directory failed with ENOENT rather than an assertion. Resolved from import.meta.url instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 15:31:20 -06:00
Luis Rojas	f96dc9d1fa	fix(training-studio): first pass over consolidated tester feedback ... Eight fixes from the #civitai-testers thread (ClickUp 868m67xr5): - Remix/reuse no longer downloads every dataset image before navigating — the reported indefinite "Loading…" — it hands off {air, caption, workflowId} instantly and DataStep hydrates previews lazily behind placeholder tiles (pool of 4, per-tile failure tolerated, object URLs reclaimed on stale tiles and flow teardown). - A label-format switch no longer destroys labels: text that came with the dataset (zip .txt, reused captions) is kept on Img.sourceLabel and re-applied verbatim in the new format; a switch that would discard machine/hand labels asks first (Convert / Re-label / Cancel). The switch aborts an in-flight auto-label drain so old-mode results can't mark post-switch tiles labeled with empty labels. - Spending beyond Blue requires an explicit confirmation naming the Yellow/Green amount, failing safe to "up to the full price" when the balance is unreadable. The element now seeds balances through a new getBuzzBalances host capability (implemented by the embed page via buzz.getBuzzAccount), and Train further's confirm names its non-Blue share too. One derivation: nonBlueSpend in $lib. - Video models accept image datasets again (the on-site trainer always did): dropzone/picker/zip all take stills, each tile typed by its own file so previews and zip round-trips stay correct. One ext<->media<->mime table in $lib/media.ts — extracting it surfaced and fixed a real drift (zip import minted audio/mp3 while the zip download knew audio/mpeg, and the download's naming table was missing mov/mkv/bmp/flac/ogg/m4a). - Krea 2 text-encoder training is locked (AI-Toolkit fails those runs after ~20-30 min with no abort): TE LR input disabled with a tooltip, and zeroed again at submit so stale param state can't smuggle it in. - "Base model trained on" resolution order fixed: the picked card (meta.cardType) now beats the coarse ecosystem fallback, so Illustrious and custom-checkpoint runs stop displaying as generic "SDXL". - Finished runs show "Expires <date> (N days)" — 30-day retention from completion (fallback: creation, which only ever warns early), amber inside the final week. RETENTION_DAYS imported from orchestrator-core, not re-declared. - "Train further" input relabeled "+ checkpoints" with copy stating a checkpoint is a save point, not one pass over the dataset. Not addressed here (tracked on the task): the Krea 2 orchestrator-side root cause and abort, whatif pricing non-linearity, the homepage box CSS (screenshot still needed), and the nine feature requests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DV3Ku4Eu9qTtzd61zt19dZ	2026-09-17 17:18:24 -04:00
briant	b16346d214	feat(models): show Hugging Face imports as grouped cards, not a table ... The row actions were unreachable. A filename is one unbroken word, so the source column widened the table past its scroll container and Cancel, Restart and Delete sat off-screen to the right — present, working, and never found. Each import is a card now, in a list of groups. Nothing has a fixed width and every group wraps, so no size can push the actions out of view; a test renders a card in a 240px column and fails on any overflow. The group header carries what belongs to the batch — name, rename control, file count, total size, age — and links to the repo at the revision the files came from, which is the commit rather than a branch that moves. The Unattached tab shows the same cards instead of one cramped line per file, so it now has progress, the stored URL, attach and detach. A transferred file no version has claimed can also be deleted straight from the queue, where before it could only be deleted from that tab. The four per-import actions moved into `useImportActions`, so the "could not free storage — Delete anyway?" prompt behaves the same wherever it is raised. The page is wider, with transfer settings in a sticky sidebar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015cyrXpr87t9Tj3bnzRrUhp	2026-09-17 14:54:32 -06:00
Justin Maier	eecd5b5987	fix(buzz): make the crypto deposit reconcile control visible (#4918) ... * fix(buzz): make the crypto deposit reconcile control visible The self-service reconcile was an UnstyledButton inlined inside dimmed xs helper text, so depositors missed it and opened support tickets for deposits that had simply not been reconciled yet. Promote it to a compact Button in its own bordered strip. The mutation, its scoping and its 1/60s rate limit are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(buzz): stop the deposit notice contradicting its own button The prompt was folded into the helper sentence, so after a check the sentence still asked 'Missing a deposit?' while the button answered 'No missing deposits found'. It also sat in the dimmed line users skim, which is the line the ticket says they miss. Give it its own weighted line and drop it once a result exists. This and 5e867c0b5f together reverse 7770455aa9, which deliberately made this control an inline text link. That decision is being revisited, not overlooked: the minimal version is what the ticket reports users missing. The button it replaced in April was variant=subtle color=gray, so this is not a revert to that either. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 14:45:31 -06:00
Koen	402090624c	feat(feed): carry meta-derived image flags in a replicable table (#4917) ... The feed service filters withMeta/fromPlatform on flags it cannot derive: Image.meta is outside the replication column list, being the largest column on the table. A narrow table keeps them replicable without the full rewrite a stored generated column on Image would cost. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-17 21:38:13 +01:00