mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
main
905 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Justin Maier
|
fdc1437da1 |
fix(users): scrub name, profile and links on account deletion; stop persisting the OAuth name (#4970)
* fix(users): scrub name, profile and links on account deletion; stop persisting the OAuth name Account deletion is a soft delete, so no FK cascade fires. On prod, of 1,330,849 deleted accounts, 733,857 still carried `name` and 192,791 a UserProfile row. - apps/auth: stop writing User.name on OAuth signup. It is unverified, user-controlled data that outlives a soft delete. It still seeds the generated username from the transient profile. - deleteUser: also null `name` and delete the UserProfile row and every UserLink row, inside the transaction. - Move the paddleCustomerId purge out of the transaction into a `finally` after the subscription cancels. cancelSubscriptionPlan falls back to reading it, so while it was nulled in the transaction that fallback could never fire on a deletion. The `finally` keeps the purge unskippable when an earlier unwrapped await throws. customerId is deliberately NOT purged here: deleteUser's own cancelSubscription triggers a Stripe webhook that resolves the user by customerId and throws before deleting the CustomerSubscription row, so nulling it would leave the row `active` forever. It is purged by the GDPR scrub, which must reach Stripe first. Pinned by a test named for it. Staff accounts created after this change file NCMEC reports without a reporter firstName; the live report path reads `name` for nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(users): keep paddleCustomerId in the deletion transaction; harden the scrub tests Reverts the paddleCustomerId ordering change from the previous commit. Moving the null after the subscription cancels let cancelSubscriptionPlan's no-row fallback run, but with seven live Paddle subscriptions (none on a deleted account) it only added a live Paddle API call per deletion, a false cancel-paddle-subscription error for nearly every one, and an unbounded wait on a client with no timeout. paddleCustomerId is nulled inside the transaction again, exactly as on main, and the try/finally that existed only to protect that later null is removed, restoring main's tail. deleteUser's net change is now only the GDPR scrub: null `name` and delete the UserProfile and UserLink rows inside the transaction. Tests, from the five-lane review: - the customerId scan serialises BigInt instead of falling back to String(call), which turned an object into "[object Object]" and reported a false absence; a CONTROL pins it - the scan's uncovered write paths are listed (kyselyWrite, updateManyAndReturn), alongside pgDbWrite and interactive transactions - tests that only made sense for the reverted ordering are removed, and one pins paddleCustomerId inside the transaction Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(users): pin the soft delete inside the transaction; cover interactive transactions From the test-lane re-review of #4970: - Nothing asserted that the soft-delete user.update is itself one of the $transaction ops. Awaiting it outside the array passed every test, including the ones named "inside the transaction". Both the transaction test and the paddleCustomerId test now assert its identity in the ops array. - A test-local $transaction override returned its argument unrun, which hid customerId writes made inside an interactive transaction. The shared mock runs the callback, so the override is removed and a CONTROL proves the scan now sees that route. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Justin Maier
|
7e15bca183 |
chore(clickhouse): upgrade @clickhouse/client from 0.2.10 to 1.23.1 (#4972)
Upgrades @clickhouse/client from 0.2.10 to 1.23.1 in the root package.json and packages/civitai-clickhouse. apps/event-engine was already on 1.x, so the workspace now holds one version of the driver. A version upgrade, not a fix for the ClickHouse socket hang-ups. The pre-upgrade rate was recorded before this change so the post-deploy rate can be compared. - ResultSet.json<T>() returns T[] in 1.x: 13 call sites, 3 in src/ and 10 in apps/moderator, which the root typecheck does not cover. - host -> url; keep_alive.socket_ttl + retry_on_expired_socket -> idle_socket_ttl. - Three 1.x default changes held at their 0.2.x values: max_open_connections Infinity, request_timeout 300000, response compression on. - keep_alive.eagerly_destroy_stale_sockets: true is a deliberate non-default, more permissive than 0.2.x's retry, standing in for it. It confounds the before/after comparison. - 1.x adds ~1ms per request (await sleep(0)); not configurable. - apps/moderator ships on its own release. - New src/server/clickhouse/__tests__/client-config-pins.test.ts pins the values. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
briant
|
5ff986be11 | chore(moderator): release moderator-v0.0.69 | ||
|
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> |
||
|
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> |
||
|
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 |
||
|
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
|
||
|
ZacxDev
|
5de4ce490d | chore(moderator): release moderator-v0.0.68 | ||
|
ZacxDev
|
a2eeefb8be | chore(moderator): release moderator-v0.0.67 | ||
|
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>
|
||
|
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 |
||
|
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
|
||
|
Luis Rojas
|
9970a2f377 | chore(training-studio): release training-studio-v0.0.9 | ||
|
Luis E. Rojas Cabrera
|
9204f00617 |
Merge pull request #4875 from civitai/feat/generation-air-resources
Training Studio: generate with epochs in place + publish a run to a model page |
||
|
ZacxDev
|
f0e123bc66 | chore(moderator): release moderator-v0.0.66 | ||
|
ZacxDev
|
3bf6d5b1a6 | chore(moderator): release moderator-v0.0.65 | ||
|
ZacxDev
|
a060fb841c | chore(moderator): release moderator-v0.0.64 | ||
|
Luis Rojas
|
50bf730eea |
feat(training-studio): publish a run as a model page, linked both ways
The studio's Publish button (standalone and embed) hands workflowId + the
selected epoch to /models/train/from-orchestrator, which builds the Draft
chain and performs the wizard's "Select Model File" step unattended — copies
the epoch blob into our storage, creates the Model file, marks the version
Approved, seeds the post form with the epoch's samples — then lands on the
MODEL wizard's "Edit model" step (the model-version wizard never offers
title/description/tags). Re-entry with the same epoch redirects server-side
straight to the wizard; a different epoch re-finalizes onto the same file;
the manual epoch picker stays reachable as the failure fallback.
The workflow and model are linked both ways: the publish entry stamps
{ modelId, modelVersionId } into the workflow's metadata when the draft is
created, and the publish handler adds published: true when the model actually
goes public (owner-token mint with cross-user cache bypass, merge-write
because the orchestrator replaces metadata wholesale, best-effort so an
unreachable orchestrator never fails a publish). The studio renders "View
draft" / "View your model page" off those fields via the new modelPageUrl
host capability — branching on run state first so a published run can never
fall back to a Publish CTA, and keeping the draft link visible after blob
retention expires.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DV3Ku4Eu9qTtzd61zt19dZ
|
||
|
Manuel Emilio Urena
|
62a7ee9ff1 |
fix(trainer): stop bare "fu"/"fk" tags blocking LoRA submission (#4883)
* fix(trainer): stop bare "fu"/"fk" tags blocking LoRA submission obscenity's `fuck` phrase carries the patterns `|fu|` and `|fk`, so a bare `fu` or `fk` token matches. Legitimate Danbooru dataset tags hit that: a creator with "fu manchu mustache" was hard-blocked from submitting, by a toast reading "Reason: fuck" over a tag list containing no such word. - `LIBRARY_OVERMATCH_TOKENS` excuses the exact tokens `fu` and `fk`, unioned into the filter's whitelist set. Deliberately not in `whitelist-words.json`, because `moderatorWhitelist` REPLACES that file: a list-only fix would reach the prompt audit and not the search gate, and a moderator emptying the row would re-break it. Only the bare token is excused; `fuk`, `fkin` and every `f?ck` spelling still fire. - The profanity block now reports the word the INPUT carried rather than the dataset word it matched, so a `fagus` tag is blocked for `fagus`, not `fag`. - The trainer splits severity through the existing `isSoftBlock`, so a profanity-only failure becomes the click-through the rest of the app already offers rather than a wall. The decision moved out of the component into `auditTrainingLabels`, which is unit-testable. Docs: the whitelist section named one source of three, the minimum-length rule was scoped to our own list only (obscenity's dataset is added wholesale and does ship 2-character patterns), `analyze()`'s documented return shape omitted `matchedWords`, and Compromise was claimed as a dependency in five places with zero imports anywhere in `src/`. ClickUp 868m5agjq, Freshdesk 72556. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(trainer): name the flagged field in the soft-block modal A trigger-word-only profanity hit marks no image card, so a modal worded "These labels look like they might be inappropriate" pointed the creator at labels that were all fine. The title, body and cancel button now name whichever of labels / trigger word was actually flagged. Also narrows the moderator blocklist copy: it claimed the code-level token exemption applies "on every path", but `clean()` consults no whitelist at all, so rendered text is still censored. That contradicted both the feature doc and the test pinning it in this same change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(profanity): drop "fk" from the overmatch exemption Measured against the Tag corpus: 143 rows carry a standalone `fu` — `fu hua`, `fu xuan`, `fu'ri'na`, `fu manchu`, `fu dog` — so excusing it buys real tags. `fk` has 3 rows (`fk`, `fk zero`, `sexy attire fk`), none used on any model, so it excused nothing and only let the abbreviation through. `fk` is blocked again, which the tests now pin alongside `fkin`/`fking`/`fkn`. The constant records what was rejected and why, so the next token is measured rather than guessed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Zachary Lowden
|
fb2089bf16 |
feat(moderator): bulk triage from the feedback queue, and a searchable known-issue picker (#4852)
* feat(moderator): bulk triage from the feedback queue, and a searchable known-issue picker
Operator asks 7 (row selection + bulk actions bar) and 2 (known-issue picker).
Selection reuses the shared SelectionSet/SelectionCheckbox, so shift-click ranges
work; the header checkbox is tri-state. A fixed bar offers the four verdicts,
derived from FEEDBACK_STATUSES rather than hand-listed, plus an optional note.
The bulk action carries PER-ROW expected status, not a bare id list. The
single-row path is scoped on the status the operator was looking at so two
moderators reaching opposite verdicts cannot produce a silent overwrite; a bulk
action posting bare ids would be a hole straight through that guard, and a
selection spans rows at different statuses so the expectation cannot be one field
on the form. Refusals are per row: the rest of the batch still goes through, and
the outcome names how many moved, how many did not, and how many were already
there — asserting no cause for the refusals, because the service does not spend a
read per row and therefore cannot tell a conflict from a deleted report.
The wire format REFUSES what it cannot read in full rather than dropping it.
A dropped pair is a report the operator ticked and watched the bar count, which
then silently keeps its old status while the screen reports a number that
included it.
The issue picker lists non-disabled issues in a combobox, searchable by title
because the shared Combobox now forwards Command's `keywords` (it filters on
`value`, which has to be the id). It is NOT filtered to open issues: measured
against production, 31 bugs, 6 disabled, exactly 1 Open — an open-only picker is
a one-item list. The number box stays: bounded and disabled-excluded issues are
reachable by number and by nothing else.
Verification:
- typecheck 0 errors / 0 warnings; build passes (the run that catches the
`?`-in-signature trap typecheck cannot see); eslint clean, validated with a
planted violation because --cache is on
- 987 passing. The 39 red are pre-existing ECONNREFUSED:15432 EXPLAIN tests in
two files byte-identical to main
- 13 pglite tests exercise the per-row guard against a real Postgres
- mutation battery 17/17 killed across three rounds, pristine control green
before and after each
Two things this deliberately does NOT do, both recorded in the PR body: extract
a shared selection-bar shell (there are four, and it would touch three unrelated
queues), and adopt recordModActivityBatch at its four existing open-coded callers.
* fix(moderator): drop the bulk note, and with it the Enter-key hazard class
Round 0 of the audit ladder (requirements & deletion). Two operator decisions
and three defects it surfaced.
DELETED — the bulk triage note. It had no author of record: not in the operator's
ask, not in the design record, not in any standard. Measured against production,
`triageNote` is null on all 26 rows — the per-row note it copies has never been
used once since the surface shipped. And being a text field in a form whose first
submit button is Reopen, it was the sole cause of the Enter-key hazard: the guard,
its exported handler, its unit tests and its docstring all existed to serve it.
Removing it closes that hazard STRUCTURALLY rather than guarding it — a form with
no text field has nothing for implicit submission to fire from. The invariant that
replaces it is pinned: a bulk verdict never touches `triageNote`, mutation-checked.
KEPT — per-row `expectedStatus`. Round 0 questioned it, correctly: the race has
never had two actors, and all three sibling bulk bars in this app post bare id
lists via `parseIdList`. Operator's call, and the call was to keep the heavier
contract; the siblings being weaker is an argument for fixing them.
CORRECTED — the unknown-status comment claimed a value outside FEEDBACK_STATUSES
"is representable". Measured false: `Feedback_status_check` enumerates exactly
those four, so Postgres already enforces it. The filter stays as the string ->
FeedbackStatus narrowing plus a second-order defence, and now says so.
CORRECTED — the bulk-max pin test's claim. It catches a SMALLER literal only;
`= 100` passes it exactly as the derivation does.
CUT — provenance narration that `docs/svelte-app-standard.md` forbids outright
("No narration, no provenance, no explaining your work to a reviewer. Say that in
the PR."). The ladder's changelog belongs in the PR body, which carries it.
Also fixes a bug this round's own simplification introduced: `onSuccess: onclear`
captured the prop's initial value (`state_referenced_locally`, WARNING-only — the
one svelte-check signal the standard says to read as a real bug).
Re-verified: typecheck 0/0, build ok, lint clean, 982 passing with only the
pre-existing ECONNREFUSED:15432 failures, mutation spot-check green.
* fix(moderator): stop a bulk 403 rendering twice, and stop two messages claiming the database
Round 1 of the audit ladder — the nine correctness axes. Seven findings, all fixed.
🟡 A bulk 403 rendered TWICE. `requiresGrant` stamps `scope: 'denied'`, not
`FEEDBACK_BULK_SCOPE`, so the page's double-render guard — which tested that
string — did not recognise the bulk action's own denial: it rendered at page
level AND in the bar, the exact outcome its comment said it existed to prevent.
Gating on position instead (`barMounted`) closes it for every scope, because
while the bar is on screen it owns its own failure whatever `fail()` site
produced it. The residual gap — a bulk refusal arriving after the bar unmounted
AND with a row open — is now stated in the code rather than left to be found.
🟡 Two messages asserted a database state nothing had read. `actionable` is
derived entirely from the POSTED expectations, so "Every selected report is
already reviewed" was a claim about rows the request never looked at. Measured
against a real Postgres: a row sitting at `new`, posted as `reviewed` against a
target of `reviewed`, returns `actionable: 0` and is untouched — the operator is
told it is already reviewed, it is not, and that is the sentence that stops them
retrying. Both the refusal and the success sentence's skipped clause now claim
the SCREEN ("was already showing X"), which is true by construction, and the
refusal points at staleness.
🟡 A comment stated a false mechanism, in the direction that makes the guard look
deletable. It claimed the already-at-target UPDATE "could not match". Measured by
removing the guard: `RETURNING id` hands the row back — Postgres matches
`SET status='reviewed' WHERE status='reviewed'` and writes a new tuple. Without
the skip such a row is re-stamped with the acting moderator, counted in `changed`
and earns a spurious `ModActivity` row. The comment now says what the guard
actually does.
🟢 The transaction docstring claimed it prevented the error boundary unmounting an
open draft. It does not — nothing catches, so a throw still reaches it. Corrected
to what the transaction does cover.
🟢 The picker's empty state said "No open issues", re-asserting the open-only
design this PR measured its way out of. It is the sentence someone would cite
when re-adding that filter.
🟢 The three-way `knownIssues` load condition had no test; widening it to "always"
is a Bug query per page turn and would have shipped green. Four cases added.
🟢 `bulkTriageFeedback` is exported and its unique-ids precondition lived only in
the parser. Documented at the signature.
Re-verified: typecheck 0/0, build ok, lint clean, 986 passing with only the
pre-existing ECONNREFUSED:15432 failures. Both reworded claims are
mutation-pinned (one of the two mutants "survived" until I asserted it had
actually applied — prettier had rewrapped the template).
* fix(moderator): the picker empty state said "no OPEN issues" — it is not open-only
Round 1 finding 5, missed by the first fix commit: the edit was in a script that
aborted on an earlier anchor and never applied. `getKnownIssues` deliberately
lists every non-disabled issue — measured, exactly 1 of 31 is Open, which is why
an open-only picker was rejected — so this sentence contradicted the design and
is what someone would cite when re-adding the filter.
* fix(moderator): route refusals through one answer, so two surfaces cannot both render
Round 2 of the audit ladder. Its headline finding was a regression round 1's own
fix introduced — the same double-render defect, one state over.
🟡 F1. Round 1 traded `!bulkFailure` for `!barMounted` in `pageError`. Those are
not equivalent, and `!bulkFailure` was the only thing keeping `pageError` and
`orphanedBulkFailure` mutually exclusive. Deterministic repro, no race: select
two rows with no row open, get a 409, untick both — `barMounted` goes false,
`form` is untouched, and BOTH render the same sentence. Before round 1 that state
rendered once.
Patching the condition again is what produced this, so the predicates are gone:
`feedbackRefusalTarget` returns exactly ONE of `bar` / `orphan` / `page` / `none`,
and each surface renders if and only if it is named. Two surfaces cannot both
match a single answer — the exclusion is structural rather than maintained.
It is also now TESTABLE, which is the other half. Both shipped defects in this
class were invisible: a page-level `$derived` has no test tier in this app. The
routing function is a plain module, so the 403 case (M22) and this regression
(M21) both have mutants that die, plus an exhaustive over-all-16-inputs property
with a positive control proving all four surfaces are reachable.
🟡 F2. The sentence round 1 declared "MEASURABLY FALSE AND WOULD LICENSE DELETING
THE GUARD" survived verbatim in the docstring of the test that guards it — the
copy a reader meets when they go looking for where the guard was measured. Swept
now; round 1 fixed the service and not the test, which is the same
sweep-every-claim failure the ladder keeps finding.
🟡 F3. Round 1's rationale block was orphaned between two declarations, so JSDoc
attached it to `bulkMessage` — a success string documented as "whether the
selection bar is on screen". Second silently-misapplied scripted edit in two
rounds; both are now folded into the routing docstring where they belong.
🟢 F4. "Same argument `promoteFeedbackToBug` makes one screen up" — it is ~170
lines below. Caught by the once-per-ladder cross-reference sweep.
🟢 F5. Two unpinned claims. The 409's staleness pointer — the sentence that makes
the refusal actionable — could be deleted with the suite green; now asserted
(M23). And the "exactly 1 of 31 is Open" production count had been copied into a
component, unverifiable from the tree and with nothing to detect it going stale;
it lives in `getKnownIssues`' docstring alone.
Re-verified: typecheck 0/0, lint clean, 991 passing with only the pre-existing
ECONNREFUSED:15432 failures, and three mutants killed each for its own assertion.
* fix(moderator): one if/else chain for every page-level refusal, and retract an overstated claim
Round 3 of the audit ladder. One restructure closes three findings, and one of
them is a claim I made and could not support.
🟡 F3 — RETRACTED: "the exclusion is structural rather than maintained" was not
what shipped. The function returns one value, but the two consumers compared it
against distinct string literals in a file with NO test tier, so the invariant was
still hand-maintained. Measured by the auditor: changing one consumer from
`=== 'page'` to `!== 'bar'` re-opens the exact double-render this PR exists to
close, with all 990 tests green.
What makes it structural is on the other side, and is what this commit does: the
two page-level refusals and the filter hint are now branches of ONE `{#if}` chain,
so at most one renders however the conditions are spelled. The docstring says this
plainly now, including that `bar` has no consumer — it exists to DENY the page a
refusal the bar is already showing, and deleting it as unused re-opens the 403.
🟡 F1 — the round-2 refactor silently broke "a refusal or the filter hint, never
both". `pageError` used to be truthy for any refusal with no row open, which
suppressed the `{:else if}` hint; routing a bulk refusal to `orphan` made it null,
so both rendered. Reachable on `?open=N` where N is filtered out. The hint is now
gated on `refusalTarget === 'none'`, which is what "there is no refusal to show"
actually means.
🟡 F2 — the sentence licensing the bar-gone gap said the orphaned refusal "renders
above the table". It rendered after the pager, below nine columns of rows —
plausibly off screen for an operator who has scrolled to the rows they selected,
which is precisely the "visible, not silent" claim it was making. Rather than
correct the sentence, the alert moved: it now renders where the sentence said,
above the table, so the gap really is tolerable.
🟢 F4 — "deleting the guard makes exactly this test red" reddens two.
🟢 F5 — `bulkFailure`'s "ONLY used to…" comment predated two more consumers.
Plus a test title that said "to the page" while asserting `orphan`.
Re-verified: typecheck 0/0, build ok, lint clean, 991 passing with only the
pre-existing ECONNREFUSED:15432 failures.
* docs(moderator): scope the "structural" claim to what the chain actually covers
Round 4 of the audit ladder. No 🔴, verdict "safe to merge"; all four findings
are prose, and the one with a decision consequence is me over-claiming again.
🟡 The chain makes the three PAGE-LEVEL branches mutually exclusive. It does
nothing about `FeedbackBulkBar` and `FeedbackDetail`, which render their own
`FormState` errors and are not in it — and those are the two surfaces in BOTH
historical instances of this defect. So exclusion against them still rests
entirely on the page comparing `refusalTarget` against 'page' and 'orphan'
EXACTLY, which is what the previous paragraph had already retracted as not
structural. Two paragraphs, one contradicting the other, and the more
general-sounding one was the false one.
Measured in the shipped tree, and re-measured here rather than carried over:
widening the orphan consumer to `!== 'page' && !== 'none'` re-opens the 403
double-render with the suite byte-identical green (991 passing) and svelte-check
0/0. That example replaces an earlier one (`=== 'page'` → `!== 'bar'`) which was
measured before the chain existed and which the chain has since made impossible —
a stale worked example for a warning that is still valid.
🟢 The history sentence was wrong about the shape of the base defect: it was the
filter HINT above the table plus an orphaned refusal below it — two DIFFERENT
messages at once, not one refusal rendered twice.
🟢 The justification for moving the alert refuted itself. It said a refusal below
the fold is silent "for anyone who has scrolled to the rows they selected" — but
those rows are inside the table, so the new position is off screen for exactly
that reader too. It is a lateral trade, better near the top and worse near the
bottom; the gap is narrowed, not closed. Said that way now.
This round swept the SHAPE rather than the reported sites: every claim of the
"structural / cannot / by construction" form in the PR's own additions was
re-read and scoped. Three existed; all three are corrected.
typecheck 0/0, 991 passing, lint clean, no executable line changed.
* docs(moderator): count the instances instead of asserting them, and un-retire a live hazard
Round 5. Two 🟡, both prose, both mine, and both wrong in the direction that
devalues the fix the previous round shipped — the mirror of round 4, where I was
wrong in the direction that overstated it.
🟡 F1. "The two surfaces in BOTH historical instances of this defect are not in
that chain — FeedbackBulkBar and FeedbackDetail." Counted, and false on every
part. This PR produced THREE instances, and the chain fully covers two:
|
||
|
briant
|
94ea11e49e | chore(moderator): release moderator-v0.0.63 | ||
|
Luis Rojas
|
b641043cc8 |
feat(training-studio): open the sidebar generator in place for embedded epoch handoffs
Embedded at civitai.com, an epoch's Generate no longer navigates: the host provides an optional generate() capability that seeds the epoch's raw-AIR resource and opens the globally-mounted sidebar generation panel — URL untouched, no reload. The synthetic-resource construction moved out of the ?air= ingestion effect into a shared seedRawAirResource, so the URL entry and the panel path can't drift. RunDetail prefers the callback (button) over generateUrl (link, still the standalone's behavior); both absent hides the affordance. Contract documented. Verified live: embed click opens the seeded panel in ~50ms with whatIf pricing (reload sentinel intact); standalone keeps the absolute _blank link. Element build + both typechecks + 30 targeted tests green; maintainer tested both hosts and OK'd. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DV3Ku4Eu9qTtzd61zt19dZ |
||
|
briant
|
46a0228855 |
fix(moderator): keep buzz form mode across submits, confirm the write
The buzz panel cleared every field on success, so a moderator working a batch re-picked action, reason and colour on every row. Keep those; the transaction itself — amount, description, entityType, entityId — still clears, since a kept amount is a double grant one stray click away and a kept entityId attaches the next adjustment to the previous grant's entity. Add the confirmation the form never had. It is built from what the action parsed, not from the form's own state, so it names what was actually filed. Tint the panel and the submit button on `deduct`. The mode now survives a submit, so the panel itself carries which way the money is about to move. Also fixes a live bug. Keeping fields meant dropping `update()`'s reset, which restores each input to its `defaultValue` — and Svelte's `set_value` only ever writes `element.value`, so `defaultValue` is `''` for every bound input, including the hidden `userId`. `set_value` then early-returns while the value is unchanged, so it was never repopulated: a second buzz transaction without re-navigating posted `userId=''` and was refused by `userIdSchema`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
briant
|
c0c5391ff8 |
feat(moderator): show JobQueue health on the dashboard, by overdue rather than depth
An image whose scan strands is visible on /images/to-ingest, but nothing shows whether the other eight JobQueue types are draining at all. Measured on prod while building this: every lane is healthy, and three of them look alarming — BlockedImageDelete alone holds 63k rows with a 7-day tail, because a blocked image waits BLOCKED_IMAGE_RETENTION_DAYS before anything may destroy it. So depth is not the signal, and a panel that showed it would cry wolf on day one. Each type gets its own deliberate wait plus slack for the cron that drains it (JOB_QUEUE_OVERDUE_MINUTES); a row past that is OVERDUE, which means the drain has stopped. UpdateMetrics and UpdateSearchIndex have no producer and no consumer anywhere in the workspace, so their figure is 0 — any row is stranded on arrival, and the panel says so rather than waiting out a window that will never elapse. The retention constants move to @civitai/shared so the number a moderator reads and the one the draining cron honours cannot disagree, exactly as STUCK_PENDING_MINUTES already does. Client-fetched, like the moderation board: it is a grouped scan of every JobQueue row (~48ms, no index to use) and the dashboard's first paint should not wait on it. The status indicator is three-valued and deliberately NOT queueSeverityClass. That scale is ordered by magnitude, so one overdue row renders lime — a step nobody reads as a problem. Healthy/Overdue/Stranded answers the question the panel exists for; the magnitude scale still colours the Overdue column beside it. Tests are in two tiers because the first cannot see what broke this in review: the overdue cutoff is a CASE whose every THEN is a bind parameter, so without a per-branch ::timestamptz Postgres resolves it to `text` and the statement will not PLAN. Compiling the SQL proves nothing about that, so job-queue-health.explain.test.ts plans it against a real schema. ::timestamp is not a substitute even though the column is `timestamp without time zone` — pg serialises a Date as local wall-clock digits plus an offset, which that cast discards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PvFRuFjhSMfxWr1aZyewBg |
||
|
Luis Rojas
|
200c93dc53 |
feat(training-studio): per-epoch Generate links via the generateUrl host capability
Every epoch with downloadable weights gets a Generate link (featured header + checkpoint rows) handing off to the main app's generator with /generate?air=<epoch blob AIR>&workflowId=&name=. The AIR comes from the same loraBlobAir builder train-further uses, off a shared EpochModelOutput/epochModelKey so "usable weights" can't fork between the two paths. generateUrl is an optional host capability: the standalone shell links absolute to CIVITAI_URL (new tab); the main-app embed provides a relative same-tab URL only when both generationAirResources and formGraphGenerator are on; absence hides the affordance entirely. Contract documented in docs/training-studio-web-component.md. Three svelte-review lanes run over the segment; findings applied. Link shape and both-host behavior verified live in the browser. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DV3Ku4Eu9qTtzd61zt19dZ |
||
|
Zachary Lowden
|
efb111ad7d |
chore(event-engine): delete the nested build-and-deploy workflow — it is never scheduled (#4851)
* chore(event-engine): delete the nested build-and-deploy workflow — it is never scheduled `apps/event-engine/.github/workflows/build-and-deploy.yml` came in with the lift-and-shift from the standalone repo. GitHub reads workflows only from the repo-root `.github/workflows/`, so a nested copy is structurally never scheduled — it has never run and cannot run where it sits. Verified against GitHub rather than asserted: the Actions API lists 10 workflows for this repo and ZERO under `apps/`, while all four root workflow paths ARE listed, which is the control proving the query sees registered workflows. Nothing in the tree references the file. It is also superseded: the app is built by the Tekton tag-webhook, and MIGRATION.md item 7 already records that wiring as done. Worth stating because it is the reason not to just leave it: the file triggers on `push: tags: 'v*'`. Inert nested, but this monorepo pushes tags routinely, so anyone who ever relocated it to the root would get an unexpected image build on every release tag — using `secrets.PAT`. Deleting it removes a latent trap rather than only tidying. Deleting one .yml takes the tracked population 14 -> 13, clear of the floor of 10 in src/__tests__/source-nul-bytes.test.ts (the guard that #4850 had to adjust). * chore(event-engine): also delete scripts/release.mjs — the live half of the same class Round 0 of the audit ladder found this PR deleted the INERT member of the standalone repo's release machinery and left the EXECUTABLE one behind. Its requirement was scoped too narrowly: the real content is "retire the standalone repo's release machinery from apps/event-engine", and that has two members. apps/event-engine/scripts/release.mjs — 437 lines, wired to no package.json script, referenced by nothing in the tree, and executable as-is. Verified what it does rather than assuming: RELEASE_BRANCH = 'release' (:25), checkout release (:229), rebase onto main (:257), bump this app's version, tag a bare `v<version>` (:112), push the tag (:129) and push release (:275). In this repo `release` is the production deploy branch — root package.json's `release:base` performs exactly that checkout/rebase/push dance — so one run is an unreviewed production deploy of whatever main holds, plus a tag in the ROOT release namespace. This app's real release path is `pnpm release:event-engine:*` -> scripts/release-app.mjs, which tags `event-engine-v*`. That is a strictly larger blast radius than the workflow this PR already deletes: the workflow needs someone to RELOCATE it before it can fire; this one runs today. Also from round 0: - MIGRATION.md still described `.github/` as "kept as legacy reference only" while this PR removes the directory entirely (it held exactly one tracked file). It now records the deletion in the same shape #4850 used for `k8s/`, keeping the structural-inertness reasoning in the tree rather than only in a commit message, and naming why release.mjs was the dangerous half. - The `.yml` floor comment is refreshed 14 -> 13 to match the walk. The file's own docstring says those comments are the measured counts, and #4850 round 2 established that a stale one in this table is not cosmetic: it is what led that round to cite a wrong precedent. 13 is still clear of the floor of 10. * docs(event-engine): correct three claims this PR's own prose got wrong Round 1 returned no blockers and no should-fixes; all five of the PR's claims hold. Its three findings are all defects in prose the previous round wrote, which is the documented failure mode of a fix round, so they are fixed here rather than filed. 1. The hazard sentence overstated the blast radius by one condition. `:264` ran `git add package.json` RELATIVE TO THE PROCESS CWD while the version write resolved the app's own manifest, so invoked from the repo root it staged the untouched root manifest, the commit exited non-zero, and it never reached the branch switch. The push required `cwd = apps/event-engine`. The blob was also 100644, so `./scripts/release.mjs` would not run. The deletion was right either way; the claim is now stated with its precondition, because a later reader re-deriving it would have found it wrong and had no way to tell which half. 2. `docs/reference/release-script-example.js` was justified as retained because it is "not wired to anything" — the exact criterion the bullet four lines above declares insufficient. It is a 446-line near-copy of the file just deleted, sharing its branch constants and its createGitTag/pushRelease path. What actually stops it is an ENOENT in the version bump (its helpers resolve `<dir>/../<name>/package.json` against a `docs/` parent that holds none), thrown after an ensureOnMain checkout but before the release branch is touched. Verified: apps/event-engine/docs/package.json is absent, with the app's own package.json as the positive control. Recorded, and the keep-or-delete question is marked OPEN rather than left reading as settled — otherwise the next reader concludes the runnable-release-script class was cleared from this app while a runnable copy sits in the same tree. 3. "listing all four root paths" was the TREE's count, not the API's. The registry retains rows for deleted workflow files, so it returns more root entries than the tree has and a re-verifier cannot reproduce the four. The load-bearing half — ZERO entries under `apps/` — is unaffected and is what the text now quotes. * docs(event-engine): the correcting sentence was itself wrong about the call order Round 2 found the sentence the previous round added to fix an overstatement was inaccurate in the other direction. It said the repo-root invocation "never reached the branch switch". Verified against the deleted blob's own main(): ensureOnMain() at :351, updatePackageVersion() at :359, commitVersionBump() at :362 (the call that fails from the repo root), setupReleaseBranch() at :365. So it DID check out main and DID write a bumped version into the app's manifest before failing — leaving a dirty tree — and what it never reached was the switch to `release`. Worse than a wrong detail: the same commit described the near-identical twin correctly twenty lines below ("after an ensureOnMain() checkout but before the release branch is touched"), so one commit described identical code two inconsistent ways. That reads as the retained copy being more invasive than the deleted one in a respect where they behave the same — and the paragraph's own stated purpose was that a later reader should not have to re-derive and disbelieve it. This is the third consecutive round whose only finding was in prose the previous round wrote while fixing the round before it. Recorded here because the pattern is the point: the code in this PR has been correct since the first commit. * docs(event-engine): sweep the same shape at every site, not just where it was reported The prose-ladder stop criterion requires the recurring SHAPE be swept everywhere it occurs before stopping, so this closes the three remaining instances the last round listed but did not file. All three are the same defect: a stated cause narrower than the effect it explains. 1. "the registry retains rows for workflow files that have since been deleted" explained one of two extra rows. Measured: the Actions registry returns 6 root rows against 4 tracked root workflow files; `pr-bot.yml` was deleted, but `retool-sanitiser-guard.yml` never existed on the default branch at all. The cause is now stated as what it is — rows that do not correspond to the default branch's tree, from either source — with the measured pair beside it. 2. "throws ENOENT inside the version bump" named the wrong call. The first failing resolve is in `getCurrentVersion()` — the version READ, before any write — and the `<dir>/../<name>/package.json` notation described a different helper. Both corrected; the conclusion (unreachable tag, unreachable push) is unchanged, which is why it was reported as an imprecision rather than a defect. 3. "the same criterion four lines above" did not land on the criterion it meant, and a positional cross-reference rots on the next edit regardless. It now names the bullet. Stopping here, and naming why rather than leaving it implicit: no round of this ladder has produced a 🔴 or a 🟡; the blast radius of everything remaining is "the document contains a false sentence"; and with this commit the shape has been swept at every site rather than only where it was reported. The last three rounds found defects exclusively in prose the previous round wrote while fixing the round before it, which is the non-terminating shape that criterion exists for. The PR's code has been correct since its first commit. |
||
|
Zachary Lowden
|
0485098106 |
chore(event-engine): delete the legacy k8s/ manifests — they deploy nothing (#4850)
* chore(event-engine): delete the legacy k8s/ manifests — they deploy nothing
`apps/event-engine/k8s/` was carried into the monorepo as part of the
lift-and-shift and MIGRATION.md kept it "as legacy reference only", noting it
should be relocated to the ops repo. That relocation has since happened: the
Kafka/Debezium manifests, the Kafka UI and the app's own Deployment all live in
the ops repo now and are what actually deploys. These nine files deploy nothing.
Verified before deleting rather than assumed:
- nothing outside MIGRATION.md referenced the directory (0 files repo-wide);
- no Dockerfile, package.json or build config reads from it — `port-forward-k8s.ts`
is a script under `scripts/`, unrelated to these manifests;
- the live counterparts exist in the ops repo, including the Kafka cluster,
Kafka Connect, the connector and the Kafka UI.
MIGRATION.md is updated in the same commit so it does not end up citing files
that no longer exist: the three places that pointed at `k8s/` now say what
happened, and the DevOps item that said to port from `k8s/09-metric-watcher-app.yml`
is struck through and marked done. The historical record is kept rather than
rewritten.
Left alone deliberately: the runtime identifiers (consumer group, `mew_` metric
prefix, `app` label) stay as they are — that is what let the cutover resume from
existing offsets, and it is still true.
* fix(event-engine): correct the docker-compose claim, and delete the stale CI/CD task-prompt
Round 0 of the audit ladder caught two defects in the previous commit, both in
prose I had just written.
1. MIGRATION.md called `docker-compose.yml` inert legacy CI. It is neither. It is
the live local-dev Kafka/Debezium harness, driven by README.md,
scripts/produce-comic-event.ts and scripts/setup-digitalocean.ts. The previous
commit REWROTE that line and narrowed the parenthetical to "(the old CI)",
re-asserting the error rather than inheriting it — a reader applying the PR's
own rule (inert legacy gets deleted) would have removed the documented local
setup. The two files are now stated separately, with the reason `.github/` is
genuinely inert: GitHub reads workflows only from the repo root.
2. `docs/plans/ci-cd.md` is deleted. It is a task-prompt for the CI/CD work that
MIGRATION.md item 7 now marks DONE, and it referenced the deleted manifest as
`k8s\09-metric-watcher-app.yml` — a BACKSLASH separator, which is why the
previous commit's `k8s/` grep reported zero external references and why the PR
body's "0 files repo-wide" claim was wrong by exactly one. Deleting the file
is what makes that claim true rather than patching a stale document.
Re-verified after the change with BOTH separators, plus a positive control
proving the backslash grep matches at the pre-deletion tree: 0 dangling
references either way.
* fix(tests): lower the .yml population floor — deleting 7 manifests crossed it
Round 1 of the audit ladder found this PR turns CI red, and confirmed it in the
real tier rather than inferring it: `Unit tests (4)` on head
|
||
|
Zachary Lowden
|
028af195da |
feat(moderator): tri-state server-side sort on the feedback queue, with a compound keyset (#4820)
* feat(moderator): tri-state server-side sort on the feedback queue, with a compound keyset Clicking a column header cycles that column ascending → descending → none, and the ordering is applied in SQL by `getFeedbackList` — never in the browser. Why server-side matters here: the list is keyset-paged at `FEEDBACK_PAGE_SIZE = 50`. A client-side `.sort()` orders the page that happens to be loaded and presents it as an ordering of the queue — right on every screen and wrong for any queue past its first page. Against 26 live rows it is also indistinguishable from a correct implementation, so it would have shipped clean. The keyset is compound, `(<sortColumn>, f.id)`, because every sortable column except the id repeats: `area`, `status` and a handler's username all tie freely, and a keyset on a non-unique key either repeats or skips the rows sharing a boundary value. The id half stays as the unique tie-break, so `?cursor=` still means what it meant before and `?cursorValue=` carries the boundary row's value in the sorted column. No sort key is a timestamp, deliberately. The value half round-trips through the URL, and `createdAt`/`handledAt` are `timestamp WITHOUT time zone` whose driver handling is asymmetric between production and this app's PGlite test tier — a boundary shifted by the local offset skips rows and the test tier cannot see it. So Age sorts on `f.id` (which IS arrival order on this insert-only table, the argument the default ordering already makes) and Handled sorts on the HANDLER, which is also what that cell renders. The 📎 column is deliberately not sortable: the count is derived from JSONB by `feedbackAttachmentCount`, so a SQL ordering means an unindexed expression over `context` AND a second implementation of that arithmetic that nothing makes agree with the first. Sort state lives in the URL, reached by real links. Every successful write calls `invalidateAll()`, so component state resets under the operator; and `replaceState` does not update `page.url` in `@sveltejs/kit@2.66.0`, so shallow routing here would be inert while looking live. Tests: the ordering and the keyset are executed against a real Postgres over a MANUFACTURED page boundary — 60 rows with heavy ties and real null blocks on every sortable column, paged all the way through at the real page size and at 7, asserting the union of the pages is the whole set exactly once, in an order computed independently from the fixture. Production never crosses a boundary, so nothing else could have seen a wrong one. * fix(moderator): resolve the feedback-sort review findings `svelte-review` (correctness, idiom, abstraction) plus an adversarial audit over the diff. Everything below is a finding that was reproduced before it was fixed. CORRECTNESS * The next-page link kept the PREVIOUS page's `?cursorValue=` when the new boundary's value was null, because it only ever SET the param. That is the ordinary transition into the trailing null block — `handled` is null on every untriaged row — and the server then read a non-null boundary, whose predicate admits the whole null block with no id bound. The same page came back, its own boundary row included, and `Next →` never advanced. The builder now always writes that half, and it moved into `$lib/feedback-sort.ts` so a test can reach it at all. * Ascending Age showed the OLDEST report first. The cell renders a duration, which grows as the id shrinks, so `ORDER BY f.id ASC` put `12d` above `10m` under a header reading `Age ↑` and an `aria-sort="ascending"` a screen reader has no way to check. The map now carries an `invert` flag and both the ORDER BY and the keyset operator read one function, so they cannot disagree. * `Number` is not a parser: `''` and `' '` became the boundary 0, `'0x10'` 16, `'1e3'` 1000. The shape is matched before `Number()` now. The test that named `''` had been passing for the wrong reason — every seeded `bugId` is ≥ 1, so a boundary of 0 on `issue asc` admits every row and looks like page one; it runs both directions now, and the two coercion classes are separate cases so a mutant cannot die on its neighbour's input. * A REJECTED `?cursorValue=` was indistinguishable from an ABSENT one after `.catch(undefined)`, and those are opposite instructions: absent means "the boundary's value is null", a real position. It now drops the whole cursor. * The queue header said "newest first" unconditionally over twelve sorted states. IDIOM / ABSTRACTION * The sort links carried none of the three `data-sveltekit-*` modifiers the tab strip on the same page documents, so every sort click scrolled the operator away from the row they had open and dropped keyboard focus to the top. * The sortable header moved to `FeedbackSortHeader.svelte`; `+page.svelte` is back under the standard's threshold. `colspan` reads `COLUMNS.length`. * `clearFeedbackPaging` was a second door onto a rule seven files already reach for through `clearPaging` — the delete moved into `$lib/paging` beside `IMAGE_PAGE_PARAM`, and the wrapper is gone. * The `↑`/`↓` marker is `aria-hidden`; `aria-sort` already announces it. * Comments trimmed to the standard's bar (breakage guards only), and the claims that were wider than the code they described were corrected — `satisfies` does not check that `ref` and `field` name the same column, and the fixture's "no cycle length divides PAGE" was false for the pair it mattered for. Two new tripwires, both mutation-checked: every `FEEDBACK_SORT_COLUMNS` member must have a clickable header (and no header a column the server refuses), and both link-driven controls must keep their navigation modifiers. * fix(moderator): close the delta-audit findings on the feedback-sort fix round A re-audit scoped to commit 2 — the fix round itself, since a fix made in response to a review is a code change like any other. All six fix areas came back clean on substance; what follows is what it found around them. One real guard defect: * The `?open=` scan could not see the spelling a new `$lib` writer would use. It matched `open:` only, while the choke point writes `{ [FEEDBACK_OPEN_PARAM]: id }` — harmless while the scan was `.svelte`-only, and a false NEGATIVE the moment commit 2 widened it to `$lib/feedback*.ts`, because copying that line out of `feedback-tabs.ts` is exactly how a second `.ts` writer gets added. Both spellings are matched now and the choke point is excluded by FILENAME rather than by being unmatchable. One seam nothing covered: * Every test in this arc was scoped to one surface — the href builder in isolation, the keyset in isolation (threading `cursorValue` as a variable and never as a URL), the loader's arguments in isolation. All three were green over the stale-`cursorValue` bug commit 2 fixed, because none of them handed one surface's output to the next. Two cases now walk builder → loader → service args, including the null-boundary transition that was broken. Three comment claims that were wider or wronger than the code: * The fixture's group-size paragraph replaced one false quantitative claim with two — "every group is 12–20 rows wide, far wider than either page size, inside a group in every ordering" is wrong for `issue` (26/25/9), wrong against a page size of 50, and wrong for `age`, whose groups are singletons. It now carries the measured table and states plainly that `age` is the exception and why that is benign (its sort ref IS the tie-break column, so the tie arm is dead rather than merely unvisited). * "seven files reach for `clearPaging`" is ten. It was doing argumentative work, so a reader re-deriving it got a different number. * The colspan pin's title claims it derives every colspan; it pins a spelling. `colspan="9"` walks past it, and a correctly written third colspan reddens it. Said so, rather than leaving a guard that reads wider than it checks. And two placement/clarity fixes: the column-ledger docstring was orphaned above the colspan test, describing a test two `it`s away; and the extraction's `inline-flex` → `flex w-full` change went unremarked — it buys a full-cell hit target and overrides the cell's `text-align`, so the first right-aligned sortable column would silently left-align. Left as-is, deliberately: `age asc` is now byte-for-byte the default ordering, so an operator's first Age click changes only the arrow. That is correct — the default view IS newest-first — but it makes the `age`/`asc` arm of the paging loops unable to distinguish "applied" from "ignored", which is now stated where it is read. The `desc` arm discriminates. * fix(moderator): the service refuses a bad sort by throwing, and Age loses its no-op state Two operator decisions from the round-0 audit. The compound keyset is unchanged and deliberately so. 1. getFeedbackList THROWS on a sort state it does not have, where it used to degrade to the default ordering. A silent degradation returns a page of real rows in an ordering the caller did not ask for, with no signal — the same silently-wrong shape this PR's own headline argument condemns on the client, where a .sort() over one loaded page presents itself as an ordering of the whole queue. At the service layer the input is an argument, not a URL, so an unreachable state is a programming error. The URL layer is UNCHANGED and still degrades: a hand-typed ?sort=garbage must not 500 a queue nobody can then open. The two layers are now deliberately different and the code says so at both ends. What holds the asymmetry safe is a new relationship guard — everything parseFeedbackSort can emit, isFeedbackSortState accepts — since nothing in the type system links them. This also closes the M7 complaint properly rather than papering over it. The advertised fallback was unpinnable: the mutant that removed the guard died on a TypeError out of the map read before any fallback assertion could be reached, so the behaviour had no test that observed it. Deleting the advertisement removes the unprovable claim; the refusal is now asserted by ERROR CLASS, which a TypeError fails. The guard's clause order is load-bearing and is now pinned: the reachability clause indexes an object literal with the untrusted column, and FEEDBACK_SORT_DIRECTIONS['__proto__'] is Object.prototype while ['toString'] is a function — neither has .includes. The allowlist check short-circuiting before it is the only reason the lookup is total. 2. Age collapses to a two-state toggle: default <-> oldest-first. Measured against production, age's ascending state was byte-identical to the default ordering — "youngest first" IS id DESC — so the first column in the table cycled click, arrow appears, nothing moves; click, reverses; click, arrow vanishes, nothing moves. Oldest-first is the one view the default cannot express, so it is the one state this column keeps. The other five keep the tri-state. Falling out of that, and the reason the collapse was worth making: - the service's `invert` flag is GONE, and with it sqlAscending. The direction token is now the SQL direction on every column without exception. - the one display flip lives in READS_INVERTED, next to the arrow and the aria-sort it exists for. It reaches a glyph and an ARIA token and nothing else, where the flag it replaces was consulted by the ORDER BY and by the keyset's comparison operator — so applying it to one and not the other produced an ordering the cursor walks backwards through. That hazard pair no longer exists to be tested. - the SQL direction is read ONCE into a local shared by both readers, so the two cannot be changed independently. - the age/no-op arm of the paging loops is gone, so no arm of that suite is left unable to distinguish "the sort was applied" from "the sort was ignored". A one-state column writes no ?dir= at all: the token would say `asc` under a down arrow and an aria-sort of "descending", and a direction param that contradicts the screen is the defect this column already shipped once. Suite: 928 -> 937 passed, 40 skipped unchanged, 0 failures. svelte-check 0/0, vite build green, prettier and eslint clean, each with a live negative control. 13 mutants run against the changed guards: 12 killed on their own assertion; 1 survives by design and is documented as a type narrowing whose runtime check is subsumed. |
||
|
Zachary Lowden
|
d2218f54da |
fix(images): stop /api/v1/images coercing an all-digit username to a number (#4839)
Server half of civitai/cli#513 / #4768. Pairs with the submodule fix civitai/event-engine-common#13 (5da5cc6467), which this pins. The coercion was never in Meilisearch, which is what #4768's body still says. The index document's user.username never reaches the response: getImagesFromFeedSearch -> ImagesFeed.populatedQuery builds `user` from the Postgres-backed userData Redis cache and spreads it AFTER the doc, overriding it. A Redis hash stores only strings, so createCache serialised each field on write and guessed the type back on read -- isNaN(Number(v)) ? v : Number(v). Number('0222') is 222. Discriminating observation, cache-busted with cf-cache-status MISS on every row: ?username=0222&limit=11 returned "0222" (Redis miss, raw Postgres row) and limit=12..16 returned 222 (Redis hit, decoded). A Meilisearch document cannot change between two requests seconds apart. Ships both halves: the field-type declarations in the in-repo fork of the cache (apps/event-engine/src/common/caches/, which writes the SAME Redis keys), the submodule pin, and a belt-labelled String() cast at the emit site. 447 of the lines are tests. Consumer audit posted on #13: the only LIVE defect is the username one. The 'false'-as-truthy-string and array-as-raw-text cases are real in the decoder but latent -- nothing reads modelData.nsfw from the cache (all three .nsfw reads come from a direct pg.query) and nothing calls Array.isArray on the cached arrays. NOT verified: nothing was exercised against a real Redis or a deploy, and the consumer audit covers static field reads only -- services/cache.ts uses a namespace import, so a dynamic access would not have appeared. /api/v1/blocks/images shares runImageSearch but returns 401 Block token required, so it is covered by code identity, not measurement. |
||
|
Luis E. Rojas Cabrera
|
1216d71d16 |
Merge pull request #4781 from civitai/elise/training-studio-polish
feat(training-studio): design pass on the Select step, base-model cards and run list |
||
|
ZacxDev
|
4daf83e288 | chore(moderator): release moderator-v0.0.62 | ||
|
Zachary Lowden
|
790a2eb765 |
feat(moderator): decode browsingLevel, tab the feedback detail panel, add an attachment lightbox (#4809)
* feat(moderator): decode browsingLevel, tab the feedback detail panel, add an attachment lightbox Display-only pass over the moderator /feedback triage queue. No write path, pagination or sorting changes. 1. browsingLevel renders as labels, not a raw number. It is a BITMASK — the values live rows carry are 1, 3, 7, 28, 30 and 31, so a direct browsingLevelLabels[value] lookup is right for 1 and silently wrong for the other five. Decoded through the shared parseBitwiseBrowsingLevel, via a per-(area, key) formatter registry rather than a global key match, because browsingLevel means a bitmask on bitdex-image-feed and nothing anywhere else. 2. FEEDBACK_AREAS moved into @civitai/shared with a re-export shim at the old main-app path, and the moderator app's hand-written FEEDBACK_KNOWN_AREAS mirror is deleted. That mirror's own comment named this fix. 3. The detail panel is now Message / Context / Attachments / Triage / Issue, with the tab in the URL. Triggers are links, not bits-ui Tabs, so the no-JS surface survives. Refusals render once, above the tab strip, naming the owning tab. 4. Attachments open in a lightbox with Esc, focus restore, arrow paging and the provenance caption carried into the large view. It is fed only from the already IMAGE_KEY-filtered context, never from raw row.context. 5. Narrow widths scroll rather than collapse; the ultrawide cap already exists in +layout.svelte and is not duplicated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhF8uK5A5xoKjYV3qFNNAR * fix(moderator): key the attachment each-block by index, not by image id `splitContext` deduplicates `images` among THEMSELVES; nothing compares `screenshotId` against them. So a row where the reporter attached the same file the opt-in page capture produced carries the id twice, and Svelte THROWS on a duplicate `{#each ... (key)}` in production as well as in dev - making that report permanently unopenable. That is the exact failure `splitContext`'s dedup exists to prevent, reintroduced one layer up by folding the two fields into a single list. Index keying is safe here: the list is derived from immutable row data and never reorders. Both frames are kept rather than collapsed - they are two different claims about the same file, and the captions are what say so. A test pins the PRECONDITION (ids in the item list are not unique), which is what makes the keying decision necessary; the template keying itself is not reachable from the node test tier. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhF8uK5A5xoKjYV3qFNNAR * fix(moderator): control the lightbox dialog with a function binding bits-ui declares `open` as `$bindable` and WRITES to it on every interaction (bits-ui@2.18.1, dialog/components/dialog.svelte) - Escape, an overlay click and the close button all land there. Handed a plain `open={...}` prop, that write becomes a child-local override which Svelte only discards when the parent yields a DIFFERENT value, so any close the parent does not observe leaves `openIndex` set against an already-closed dialog and re-clicking the SAME thumbnail then does nothing. FeedbackFilters.svelte already records this exact hazard about the same primitive family; the lightbox now follows it. The getter makes the parent the only source of truth and the setter is the single place a close becomes state. Verified by a full `vite build` of apps/moderator (7,926 modules, SSR + client, prerender analysis) - the only gate in this app that compiles .svelte at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhF8uK5A5xoKjYV3qFNNAR * fix(moderator): let bits-ui own the lightbox focus restore, and key paging by index Two corrections to the lightbox, both found by reading bits-ui rather than assuming. 1. The hand-rolled focus restore is removed. It rested on the premise that a dialog with no `Dialog.Trigger` gives the library nothing to restore to, and that premise is FALSE: focus-scope-manager.js:14-26 captures `document.activeElement` at `register()` - which `mount()` calls BEFORE `#handleOpenAutoFocus` - and focus-scope.svelte.js:72-90 focuses it again on unmount, guarded by `document.contains` and a try/catch. No trigger is involved; whatever had focus when the scope opened gets it back, which is exactly the thumbnail button that was clicked. Preventing that to run our own replaced a guarded implementation with an unguarded one whose capture point raced the library's `requestAnimationFrame` focusFirst. The comment asserting otherwise was a false claim and is replaced by one citing the source. 2. `{#key}` now keys on the index, not `current.id`. Ids are not unique - same reason the thumbnail each-block is index-keyed - so paging between two frames that share an id was a no-op. Verified by `vite build` of apps/moderator (7,926 SSR + 7,697 client modules). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhF8uK5A5xoKjYV3qFNNAR * refactor(moderator): cut the filter-formatter registry, put attachments back beside the message Two operator decisions on the feedback display pass. 1. The per-(area, key) formatter registry is gone. It was a two-level ReadonlyMap with prototype-key hardening on both axes, serving exactly one entry. Measured this round: the live feedback-context builders are appsStoreFeedbackContext.ts (kind/category/sort/query) and FeedbackDrawer.tsx (path only) - neither writes browsingLevel - and BitDex was decommissioned 2026-09-01, so the second axis served only the 23 historical bitdex-image-feed rows. Replaced by two === checks in formatFeedbackFilterValue, which is still the single entry point the panel calls. No keyed lookup on an untrusted key survives, so there is no longer a prototype chain to harden; if one ever comes back it must be a Map, and the file says so. The bitmask decoder and every one of its tests are untouched, including the 1/3/7/28/30/31 production fixtures, the +<bit> unlabelled-bit case, the int4 bound and the null fall-through. A new hand-typed table pins the exact rendered output for every (area, key, value) triple the panel can be handed today, so the removal is a refactor and not a behaviour change. The prototype-key cases are kept but relabelled as an invariant guard: with the registry gone they no longer have a live hazard behind them. 2. Attachments are visible with the report message again. Message + attachment is the core triage pairing and the tab split cost two navigations for something that previously cost none; the containment it bought was marginal, because thumbnails already only mount once a ROW is expanded. Shape chosen: no attachments tab at all - FeedbackAttachments renders on the default Message tab, below the message. An old ?tab=attachments link degrades to the default via feedbackTabFromUrl, which is now where the attachments are. Everything load-bearing survives: tab triggers stay links (the no-JS surface), the refusal banner stays above the strip with no auto-switch, the lightbox is still fed only from feedbackAttachmentItems(splitContext(row.context)), the {#each} stays index-keyed, and the Dialog open stays a function binding. Mutation checks on the replacement guard, each restored byte-identical after: - area half dropped -> 9 failed / 43 passed; own assertion "leaves the SAME key alone under a different area", expected { text: 'R, X, XXX', title: '28' } to deeply equal { text: '28', title: null } - key half dropped -> 6 failed / 46 passed; own assertion "leaves a different key alone under the decoded area", same shape The key-half run found a real gap first time round: every fixture on the decoded area was a string, and formatBrowsingLevel rejects non-numbers, so none of them could reach the decoder at all and none could see the mutant. A numeric fixture was added to make that assertion reachable. Gates: typecheck 0 errors; svelte-check 9483 files / 0 errors (negative control went red); apps/moderator 61 passed | 1 skipped (62) / 844 passed | 40 skipped (884); main-app feedback suites 7 files / 159 tests passed; vite build 7926 modules transformed, done; prettier clean with a named control file still warned; eslint rc=0 with a prefer-const negative control going red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(moderator): stop a tab click destroying typed text, hiding the newer refusal, and following a row Round-1 audit findings on the feedback detail panel. All three share one cause — a tab is a destructive navigation, and the panel still reasoned as if both forms were on screen at once — but they need three separate fixes, not one. F1 A tab switch silently destroyed the operator's typed triage note. The note box was an unbound value= and the issue title/summary/bugId were uncontrolled, so the text lived only in DOM nodes that the {#if activeTab} chain destroys. Both drafts now live in FeedbackDetail, which survives the navigation, and are bind:value-d. The promote draft is handed down as a $state proxy. The note is re-seeded from the reloaded column in onSuccess, so reset:false keeps its meaning without depending on the bound expression changing. The trade-off (a draft now shadows another moderator's concurrent edit to the same column) is written down at the declaration rather than left to be discovered. F2 Two refusals could be live at once and the older won, hiding the newer. Each form's onSubmit now clears the other's error, so at most one is live; and the selection moved into $lib/feedback-refusal.ts, where it prefers the ACTIVE tab's own form and is testable. The old comment claimed both errors "cannot be" set and its argument did not reach the case; that claim is retracted in the new docstring rather than replaced with another one. F3 ?tab= was sticky across rows, so a report could open straight onto the triage buttons with its text off-screen. New feedbackOpenHref() is the single choke point for opening a row and deletes ?tab=. Both call sites now use it — rowHref, and FeedbackPromote's siblingHref, which built its URL by hand and only renders ON the Issue tab, so every sibling link opened the next report onto the one panel showing none of what that reporter wrote. F5 The (area, key, value) table's docstring claimed to enumerate "every triple the panel can actually be handed today". It cannot: filters is schemaless JSONB and area is free text, so the population is open. Sentence narrowed to what the list covers; the two shapes the audit read off production (a period key, 'Most Collected'-shaped sort) added as rows. Tests: +31 in the node tier. feedback-refusal.test.ts is real behavioural coverage of the selection rule; feedback-panel-tripwires.test.ts is explicitly labelled TRIPWIRES, NOT COVERAGE — normalised source-text pins over .svelte files, which have no test tier in this app, walkable by rewording and unable to certify that anything works. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(moderator): bind the promote draft, and retract the unreachability claim a second time Round-2 audit findings on the feedback detail panel. 1. The hoisted draft raised `ownership_invalid_mutation` on every keystroke. `FeedbackPromote` mutates `draft`, which `FeedbackDetail` passed as a plain prop; Svelte's dev ownership validator wants a SETTER on the props descriptor (`is_bound_or_unset`, svelte@5.56.3 internal/client/dev/ownership.js:71-80) and a getter-only prop has none. `$bindable()` in the child plus `bind:draft=` in the parent; `promoteDraft` becomes `let` because `bind:` over a `const` is the compile error `constant_binding`. Measured in a compiled two-component repro: 2 keystrokes gave 2 warnings unbound, 0 bound. Production was never affected. The measurement needs `--conditions=browser --conditions=development`, as two separate flags: `--conditions=browser` alone resolves esm-env to production, `DEV` is false, and the count is 0 whatever the code does. 2. The "the two refusals can no longer both be live" claim is FALSE, and it failed the same way the sentence it replaced did — a true statement about one ordering read as a claim about another. "Every submit starts by clearing its counterpart" does not order a submit start against a previously started submit's response. The reachable path, walked through the sources: a triage submit starts; the tab links are not disabled, so the operator moves to Issue; the branch is destroyed but `use:enhance`'s `destroy()` only removes the submit listener (kit@2.66.0 runtime/app/forms.js:227-231) and nothing aborts the AbortController it handed to the submit hook; they submit promote; both responses land and set both errors. Behaviour is already correct, so this is a prose fix. Aborting was considered and rejected: `enhance` returns early on AbortError without invoking the callback, so a committed write would produce neither a confirmation nor a refusal. 3. A tripwire titled "never writes the open param by hand" asserted one spelling and was false in the file it scanned. Widened to the property worth holding — clearing by hand is fine, SETTING an id must go through `feedbackOpenHref` — over the whole route directory rather than a hardcoded pair, which is how the existing third site (`FeedbackFilters.svelte`) had gone unscanned. Positive control is per pattern. 4. Recorded both halves of the cross-clearing trade, including the half it cost: a standing promote refusal is discarded by an unrelated triage save that SUCCEEDS, leaving a pre-filled form with no explanation on screen. 5. The success re-seed no longer discards text typed while the request was in flight. The decision is `reseedTriageNote` in `$lib/feedback-drafts`, a pure function with its own tests, because the panel has no test tier. 6. `feedback-filters.test.ts`'s `reachable` table renamed `pinnedRenderings`: the identifier asserted in one word the completeness its own docstring had already retracted. Also recorded the precondition nothing stated: draft survival across a tab click needs the row to come back in `data.items` under the same id, so a concurrent status change that evicts it from the active filter destroys the panel anyway. Gates: svelte-check 0/0 with a negative control taken to 5 errors in 3 files; apps/moderator suite 875 -> 884 passing (63 -> 64 files); vite build green; prettier clean against a deliberately misformatted control in the same invocation; eslint clean against controls using rules enabled for each path (`no-debugger` for .svelte, where `prefer-const` is off; `prefer-const` for .ts). Every new or changed guard was mutation-checked and killed by its own assertion — including one that SURVIVED first time, because it matched its own docstring prose rather than the template. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(moderator): strip comments before scanning, and sweep the two recurring comment shapes Round 3 of the audit ladder, and the last one. The two reported findings are one-sentence corrections; the work that matters is sweeping the two shapes that have each now recurred, at every site rather than only where reported. Shape A — a text pin satisfied by PROSE rather than CODE (3 instances). Fixed structurally instead of one string at a time: `source()`/`componentSource()` in feedback-panel-tripwires.test.ts now strip markup, block and line comments before scanning, so every assertion in the file is a claim about code only. The stripper carries two controls — a synthetic fixture covering all three comment syntaxes, and a real-data one that watches `reset: false` in FeedbackDetail.svelte go from 5 witnesses to 2 and `bind:draft={promoteDraft}` from 2 to 1. Neutering the stripper reddens both. F-R3-2: the `re-seeds the note ... on a successful save` pin asserted a bare `row.triageNote ?? ''` (4 witnesses, 2 of them prose) and survived deletion of the whole re-seed. Repointed and retitled to `seeds the note box from the stored column`, which its one code witness actually supports and which was pinned nowhere before. The re-seed itself stays covered by the sibling `re-seeds the note through reseedTriageNote` pin, verified to fail on exactly that deletion. Shape B — a docstring asserting reachability or impossibility whose named evidence does not establish it (7 sites). - feedback-refusal.ts (F-R3-1): the 5-step walk proves two errors can be live at once; it does NOT reach the `?? raised[0]` fallback, because step 5 ends on the tab that owns the refusal. Decoupled: the fallback needs only ONE refusal read from a tab owning neither form, which is the ordinary case and is already tested. - FeedbackDetail.svelte, feedback-drafts.ts, FeedbackPromote.svelte: "the row's `{#if open}` never goes false" narrowed to "stays true across a tab click", which is what `feedbackTabHref` preserving `?open=` actually buys. The wider claim was contradicted by FeedbackDetail's own precondition paragraph. - FeedbackDetail.svelte: "the only thing that can change under it is the stored note" is false; status/bugId/handledAt all change under the instance. - FeedbackDetail.svelte: the auto-switch rejection cited a reload "that would discard" the message. It would not — the message is component state that survives a load re-run. Race retracted and deliberately not replaced. - feedback-filters.ts: "the only area that can carry that key AT ALL" softened to "no live producer writes it"; a repo enumeration cannot bound a schemaless JSONB column. - FeedbackAttachments.svelte, feedback.test.ts: "no shape of this page can route raw JSONB" replaced with what a structural type actually buys. Also adds the missing half of `+page.svelte`'s "unreachable with JS" claim, citing the SvelteKit line that nulls `form` on navigation. Prose, pins and comments only. No behaviour change. Gates: svelte-check 0 errors (control: 7); apps/moderator 884 -> 886 passed, 0 failed (+2 = the new stripper controls); vite build clean; prettier clean with a named control flagged in the same run; eslint clean with a no-debugger control firing on both a .svelte and a .ts file. 17 mutants run against the tripwire pins, all killed, each on its own assertion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Zachary Lowden
|
31d768fba2 |
fix(feedback): bound attached image ids by shape, not just length (#4801)
`context.images` and `context.screenshotId` on a feedback submission are ids the CLIENT says it uploaded. They were bounded by LENGTH ALONE (`z.string().trim().min(1).max(100)`, no format), so any string reached the JSONB column. They are now `z.uuid()`. Checkable because every mint on this surface emits `randomUUID()` — the presign, the multipart route, and the relay's own server-side mint — so a legitimate id is always a uuid, and every image id currently stored is one. Rejects nothing real. NOT an ownership check. Nothing records which user a key was issued to, so a user who learns another user's id can still cite it; closing that needs a persisted grant at mint time, which is a change to a shared upload route. Defence-in-depth at a cross-deployable seam, NOT a live-hole fix: the moderator's own `IMAGE_KEY` regex already closes the arbitrary-origin class against the only consumer, and it must NOT be deleted as redundant — this schema binds writes only, nothing revalidates a stored row, and every pre-existing row is unvalidated. That is now stated in both files and pinned by a behavioural drift guard. Tests: 13 hostile shapes, 12 red at base (the whitespace-only case is an invariant guard and is labelled as one). Includes a 36-character absolute URL — the case that separates uuid-SHAPED from uuid-LENGTHED, without which a `z.string().length(36)` substitution passes the whole suite. The case ledger is asserted, not written in prose, because that count went stale three times during review. Audited over three rounds (requirements/deletion + nine axes + delta), zero deploy-blocking findings; every finding was a false sentence rather than a defect in behaviour. Detail in the PR comments. Drops `FEEDBACK_IMAGE_ID_MAX_LENGTH`, now read by nothing. |
||
|
ZacxDev
|
09dbb93545 | chore(moderator): release moderator-v0.0.61 | ||
|
ZacxDev
|
1a836ce523 | chore(moderator): release moderator-v0.0.60 | ||
|
Zachary Lowden
|
c56e46ab90 |
feat(moderator): a /feedback triage surface for the Feedback table (#4785)
* feat(moderator): a /feedback triage surface for the Feedback table The onsite feedback prompts have been writing to a table nothing reads. Three of the four statuses its CHECK constraint allows were unreachable by any code path that existed. This is the reader. - `/feedback`: one page, status + area filters in the URL, keyset paging, and an inline `?open=<id>` detail panel. - Context is rendered as a CLAIM, not evidence. `context.path` is a bare pathname, so the panel rebuilds the URL from `path` + `filters` (omitting the store defaults) — `path` alone links to a different page than the one the report is about. - The Faro session id becomes a Grafana Explore link, and only while the rows still exist: past the 72 h Loki retention there is no link and an explicit expiry note, because "no logs found", "this session produced no telemetry" and "the link is broken" are three facts with one observable. - Two permissions, `feedback.status.set` and `feedback.bug.promote`, composed with the page grant rather than welded to it. - Promoting mints a `Bug` with `publishedAt` NULL and `content` NULL, so a reporter's words cannot reach the public board and no unsanitised text reaches an HTML-rendered field. Both writes are scoped on the state the operator was looking at — the triage UPDATE on the status they saw, the link on `bugId IS NULL` — and zero affected rows is a 409, never a success. The `20260911120000_feedback_triage` migration is merged but NOT APPLIED to any environment; a human applies it per environment as every migration here is. This commit is the `schema.full.prisma` model edit plus the regenerated files. * fix(moderator): resolve the /feedback review findings Three review agents over the segment, then two delta audits over the fixes. The defects, in the order they cost an operator something: - Attachment ids were rendered through `getEdgeUrl`, which returns its argument VERBATIM when it starts with `http`/`blob` — and the producer bounds them by length only. A reporter could point a moderator's browser at any origin and collect a read receipt naming who opened their report. `splitContext` now requires a Cloudflare-key shape; a rejected value is shown as text rather than dropped. - A duplicate id in that same client-supplied array THROWS `each_key_duplicate` in production, making a report permanently unopenable. Deduplicated. - The triage note was blanked on every save and the next click destroyed it. `update()` resets the form before `invalidateAll`, and Svelte does not rewrite a bound value that has not changed — so the box sat empty over a column that still held text. `FormState` grew a `reset` option; the default is unchanged for its other 37 call sites. - Two refusals had no surface at all: the panel is unmounted before the message is assigned whenever the row leaves the view, and a no-JS submit has no panel to begin with. A page-level alert now covers exactly the cases the panel cannot, and both panel alerts sit outside the branch that the reload flips. - "Attach to an existing issue" 404'd on an issue created seconds earlier — the existence check read the replica. It reads the primary, as does the check that decides whether a refusal says "already linked" or "that report is gone". - An empty attach form refused with "Give the issue a title" over a form showing no title field. The mode is posted explicitly instead of inferred. - The canonicalising redirect fired on the action POST too, so a no-JS client would re-run the action and be told a save that worked had conflicted. Also: `//host` rejected in the reconstructed URL, the keyset cursor parsed once in the query schema rather than twice across two layers, `MAX_INT4`/`isInt4Id` and `clearPaging` taken from their canonical homes, `issuesUrl` given one definition, the sibling row type exported rather than hand-copied, `$bindable` filter controls moved to function bindings, and the page split into a filter bar, a detail panel, a context panel and a promote panel. Tests: 33 added over the round, including a `load` suite that pins the GET-only redirect and the cursor bound, and a PGlite case for the promote path's rollback. * fix(moderator): blind-audit findings on the /feedback queue Three comments asserted a mechanism SvelteKit does not have. They said a refusal re-runs `load` before `FormState` assigns its error, unmounting the panel. In `@sveltejs/kit@2.66.0` both the reset and `invalidateAll()` sit inside `if (result.type === 'success')` (`runtime/app/forms.js:99-107`), so no reload happens on any refusal. The behaviour was right and the reason was invented — and it is the sentence a maintainer would use to delete the panel-level alert as redundant, which is exactly the defect an earlier round introduced. Corrected to what is true: the page-level alert is the NO-JS surface, the panel alert is the JS one, and the two cover disjoint paths. Where hoisting those alerts out of their branch chains no longer has a reason, they now say so rather than carrying a replacement rationale. - The keyset cursor was untested at any page size but one, so the mutant returning `items[0].id` instead of the last row's SURVIVED a green run — at `FEEDBACK_PAGE_SIZE` 50 that repeats 49 rows and strands 49. The fixture now seeds three rows and pages at two, where the first and last of a page differ, and the mutant dies on `expected 3 to be 2`. - The page 500'd for the one role that could already reach it. `allows()` short-circuits for `moderator:admin`, so the nav entry and its badge are live before any `/admin` tick, and the badge counts on `status` alone — a real number against an unmigrated database, then 42703 out of `load` on every click. It now catches that one code and says which migration to apply. The migration header claimed the opposite and is corrected; it is the sentence someone would pick a deploy order from. - `handledById` NULL rendered two different wrong ways: the detail view printed "Handled by #null" for a deleted account, and the list called a live account with no username "deleted account". Two independent nullables, one helper. - A row reopened to `new` kept its `bugId`, which put it in the unhandled queue showing the linked-issue panel instead of the promote form — and `linkInTransaction` refuses any row whose `bugId` is set, with no unlink control anywhere. The link is now cleared alongside the handler columns, for the same reason: `new` means untriaged. Also: `reset: false` on the promote form, whose hidden `id`/`mode` inputs a reset would blank; sibling links drop the keyset cursor so they cannot land on "not in this view"; the copy button returns to idle. Two things are recorded rather than fixed. The list read stays on the replica — it fails closed and matches every other queue here — with the cost written down at the call site. And the PGlite tier binds `dbRead` and `dbWrite` to one client, so no test pins which of them a call site uses; that is now stated in the suite instead of being implied by a green run. * fix(moderator): stop clearing bugId, and narrow the 42703 degrade Reverts the `bugId` clearing added last round, and corrects a claim that had been repeated three times without anyone tracing it. THE REVERT. Clearing the issue link when a row returns to `new` traded one bad state for a worse one. `handledById`/`handledAt` record WHO ACTED, which a reopen retracts; the link records that this report is ABOUT that issue, which a reopen does not make false. Clearing it destroyed that with no way back — the number is stored nowhere else, `ModActivity` has no column to put it in, and `getSiblingFeedback` silently dropped the row from every sibling's list — and left a `Bug` with nothing pointing at it, the state `promoteFeedbackToBug` runs a transaction rollback rather than create. Two comments in one file disagreed about whether that matters; the existing rule wins. What that leaves open is recorded rather than papered over: a linked row can never be re-linked, because the promote path requires `bugId IS NULL`. That is a missing unlink control, it is missing at every status, and reopening is not a sensible back door to it. Not added here. `docs/moderator-app/` §5 now states the rule and the gap, so the doc and the code agree. THE FALSE CLAIM. A test comment said the surviving cursor mutant "makes 49 unreachable". It does not. Walked against the harness: seven rows at `limit: 3` page 7-6-5 / 6-5-4 / 5-4-3 / 4-3-2 / 3-2-1 — every id still reached, each turn repeating all but one row. The cost is a queue that drains 50× slower, not one with holes in it. The fixture's justification is unchanged, which is the point. Also: `isMissingTriageColumns` now requires the message to name one of the four columns that migration adds, not merely the `42703` code. The page answers with one specific instruction, and `42703` is raised by any absent column — so the code alone would give that instruction to an unrelated typo while the `catch` suppressed the error that would have named it. Matched on the column name rather than on "does not exist", which does not survive `lc_messages`. And the degrade is now exercised end to end: a new pre-migration PGlite fixture runs the real query against the real pre-migration table and asserts on whatever the driver actually raises, rather than on a hand-built `{ code }`. Measured: `code` is top level and the message is `column f.handledById does not exist`. * docs(moderator): record the frozen-link gap at the guard that enforces it Round 3 of the audit ladder: the note explaining why a linked row can never be re-linked sat inside triageFeedback, 150 lines from linkInTransaction's `bugId IS NULL` clause -- which is what a contributor adding an unlink control would actually open. Cross-referenced, and the ON DELETE SET NULL cascade named so it is not mistaken for a control. Also replaces a count in a test comment with "every other", after the count drifted when this round added a case. Fix the form, not the number. Comment-only: no executable line changes. * docs(moderator): qualify the error-shaped claim, and drop a wrong lesson Round 4: "Every OTHER case constructs { code, message }" was false -- the last case in that block deliberately feeds non-error values. Qualified, and the qualifier is now called out as load-bearing so it is not trimmed later. Also drops the parenthetical claiming the earlier count had drifted. It had not: traced the block across the three fix commits -- 3 cases at |
||
|
ZacxDev
|
607dd2db5a | chore(moderator): release moderator-v0.0.59 | ||
|
ZacxDev
|
5b85a02573 | chore(training-studio): release training-studio-v0.0.8 | ||
|
Luis Rojas
|
d928a320a1 |
fix(training-studio): run svelte-kit sync before the element build
The build script chained the element build first, but esbuild resolves the app tsconfig, which extends the GENERATED .svelte-kit/tsconfig.json — absent in a fresh checkout, so the Docker image build died at `failed to resolve "extends"` while every local build passed (a dev server had always generated it). Sync first covers both builds. Verified by deleting .svelte-kit and running the full build from scratch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Justin Maier
|
fe93a06d1d |
fix(training-studio): drop a version label that only repeats the model name
A single-version card whose version carries the family name rendered "Illustrious · Illustrious" — in the run list, and as the selected-model title in the flow. `versionSuffix` returns the label only when it differs from the card name, and both call sites use it, so the rule is stated once rather than copied into the flow and the orchestrator row mapper. Found while explaining to Justin why the sample rows say "SDXL · Illustrious": those fixtures predate Illustrious and Pony becoming their own cards, and answering what real data WOULD say surfaced this. Not pushed yet at the time of writing. This is Luis's open PR (#4701); how it lands is his call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t (cherry picked from commit 40d09455ff4d2e26a1f7980c211e0c5e792b9ae2) |
||
|
Justin Maier
|
ae86199046 |
fix(training-studio): stop the run list truncating, and tell the truth about retention
The state badge shared a row with the name AND the subtitle, so the subtitle was squeezed by the badge's width and clipped on every card. It now sits on the title's line and the subtitle spans the full card. Nothing in the list truncates at this viewport any more — measured, not eyeballed — which matters most for a failed run, where the line carries the refund. The base model is bold and in the title's colour, so "Flux" or "SDXL" reads at a glance rather than as the head of a grey run-on. The heading claimed every run stays here. Runs are kept 30 days; publishing puts the model on Civitai permanently but does not keep the run in this list. Saying "stays here" to someone deciding whether to publish gets that backwards. Not pushed. This is Luis's open PR (#4701); how it lands is his call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t (cherry picked from commit 670e2862e91d6c7d9e9f45f090e9c642a67e5408) |
||
|
Justin Maier
|
cfdb8d8c2e |
fix(training-studio): drop the 2K claim on H3, free up room in the run list, green Published
Three from Justin's pass:
MiniMax H3's tagline claimed 2K video. That resolution is only available through
MiniMax's own API endpoints, not through what we train and serve, so the card
was promising something a user could not get here.
The run list dropped its two-letter code tile, which was taking the width the
name and subtitle needed. Two subtitles still clip at this viewport, but on the
long tail ("40 images · 1.2k downloads") rather than mid-word on the base model.
Published was a Buzz-orange badge, which reads as a warning next to Failed's
red. It is a success state like Ready, so it is emerald now.
Not pushed. This is Luis's open PR (#4701); how it lands is his call.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t
(cherry picked from commit ff4fba0dc817ca951dc7c2d93861e74c120f7929)
|
||
|
Justin Maier
|
e435d32e4f |
fix(training-studio): give base-model taglines two lines, always two lines
At one clamped line most taglines were cut mid-sentence, which made them read as decoration rather than information. Two lines fits every one of the 22, and the block reserves both lines whether or not the text needs them, so the price row sits at the same height on every card in a row. Note on how this was missed: the earlier check for truncation compared scrollWidth against clientWidth, which cannot detect it. `line-clamp` wraps the text normally and hides the overflow VERTICALLY, so the widths match on a clipped element and the test passes on every card. Measured by height instead, every card was clipped at 17px of the 33px it needed. Height is the right axis for a line clamp. Not pushed. This is Luis's open PR (#4701); how it lands is his call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t (cherry picked from commit 499a388df1e13539dbbd5826ecafcd573db6aad6) |
||
|
Justin Maier
|
ae430d1d73 |
feat(training-studio): rewrite every base-model tagline around capability
The old ones named the vendor and the parameter count — "Krea AI's in-house image generation model", "Microsoft's 4B native-resolution image model" — which tells a user nothing about whether to train on it. Each card now says what the model is for and what it is good at: the anime pick, the realism pick, complex text in English or Chinese, video with synced audio, native resolution at any aspect ratio. Researched from vendor docs, HF cards and release posts rather than the Civitai model pages, which are uploader marketing and in several cases stale. Two things that changed as a result: SDXL is no longer described as a safe default, which stopped being true, and LTX is no longer "moderate quality" — 2.5 does 4K with joint audio. Deliberately absent, per Justin: licence names and VRAM figures. Both are real and both were in the drafts; neither helps someone choose a base model, and the licence question is more complicated than a card can carry honestly (Flux.2 Klein alone ships 4B permissive and 9B non-commercial under one card). All 22 measured against the card's clamp at the rendered width — none truncate. Not pushed. This is Luis's open PR (#4701); how it lands is his call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t (cherry picked from commit da990f30aa1eb0a38db49b25858e53bef462315a) |
||
|
Justin Maier
|
340a32cecf |
style(training-studio): shorten the price caption so it fits one line
"price changes depending on data and settings" wrapped to two lines once the sub-12px type tier moved up to 12px. Measured after the change: 16px tall against a 16px line-height, so one line. Not pushed. This is Luis's open PR (#4701); how it lands is his call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t (cherry picked from commit 675021edebf1373fb4ad4625aa94d1cc9975098c) |
||
|
Justin Maier
|
8f68b2460d |
fix(training-studio): date ACE-Step from the checkpoint we actually offer
Cross-checked every hand-researched date against the publish date of the exact Civitai model version each card's AIR names — the checkpoint a user trains against, rather than the family's upstream announcement. Eleven of the sixteen checkable cards agreed to the month. ACE-Step did not: all three of its selectable versions published 2026-04-30, which is the 1.5 XL line, not the 1.5 release the research dated to February. The card offers XL, so the card is dated from XL. Four other disagreements are left alone deliberately: pony, sd15 and wan carry Civitai upload dates well after their upstream release, because these are re-hosted checkpoints and the question the card answers is how old the MODEL is. LTX's Civitai versions are 2.0 and 2.3; its 2.5 entry is a HuggingFace repository AIR with no Civitai record, so the researched date stands and is correct — 2.5 is genuinely the newest selectable version. Not pushed. This is Luis's open PR (#4701); how it lands is his call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t (cherry picked from commit 82b67906860373ce3bc578854546935f977bc2c5) |
||
|
Justin Maier
|
5e8f26fd04 |
feat(training-studio): show how old each base model is, and feature the video three
How current a base model is decides whether it is worth training on, and the
cards said nothing about it. Each card now carries the public release date of
its newest selectable version, shown relative inside six months ("4 weeks ago")
and as a month after it ("Jul 2025") — past six months the reader wants a date,
not arithmetic.
Dates are hand-researched from vendor releases, HF repos and GitHub tags, not
from the Civitai pages. Three are month-only (mageflow, illustrious, acestep)
because no day could be established; `releasedLabel` anchors those to the 1st,
which is immaterial relative and invisible absolute. Two cards where a family
date would mislead: zimage is dated from Base (Jan 2026), two months after
Turbo; hunyuan is 1.5, not the Dec 2024 original that shares the name.
Video now features MiniMax H3, Wan and LTX with Hunyuan behind "show more",
matching how image already worked — MiniMax H3 was last in a list of four.
The show-more label also stops saying "1 more models"; video is the first list
that can hide exactly one.
Not pushed. This is Luis's open PR (#4701); how it lands is his call.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t
(cherry picked from commit 3b0e16d9c3da441155a0b60a35e38c2e130acacc)
|
||
|
Justin Maier
|
ea12f087ad |
feat(training-studio): recommend MiniMax H3 for video, not Wan
Justin's call, resolving the disagreement the per-card flags exposed: the RECOMMENDED badge sat on MiniMax H3 while the "We recommend X" sentence still named Wan, four inches apart on the same screen. The flag is now the authoritative one and the recommendation follows it, across all four types. This also moves the DEFAULT video selection, which is the larger half of the change: a user starting a video LoRA now lands on MiniMax H3. Web research could not confirm MiniMax H3 has open weights or a LoRA training path. The orchestrator disagrees: its whatIf prices a MiniMax H3 training run at 2,750 Buzz against a live token, which is a stronger signal than the absence of public documentation. Not pushed. This is Luis's open PR (#4701); how it lands is his call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t (cherry picked from commit cf0e88ff07d0512ec43d9ab996eae3fb280e09ed) |
||
|
Justin Maier
|
2127ddf9ea |
feat(training-studio): per-card flags in place of the single Recommended badge
The card badge was hardcoded to whichever model `TYPES[].recommended` picked for the current type and media, so exactly one card could ever be labelled and the label could only ever say "Recommended". A card now carries an optional free-text `flag` and the badge renders it, so any label works without a code change. Seeded from Justin's pass: zimage/minimaxh3/acestep 'recommended' (one per media), anima 'anime', krea2 'latest'. The star icon is kept for 'recommended' only. `TYPES[].recommended` is untouched — it still drives the default selection and the "We recommend X" sentence. That is now a separate thing from the badge, so the two can disagree; the flags above were chosen to agree per media. Not pushed. This is Luis's open PR (#4701); how it lands is his call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t (cherry picked from commit 1ec916431ad4d8ee18b840dc16734f2fb84ed49f) |
||
|
Justin Maier
|
5a931464c2 |
feat(training-studio): select-step polish from Justin's design pass
Ten changes requested by voice while reviewing PR #4701 on a running instance: - Switching media keeps the chosen type when the new media offers it, instead of resetting to the first one. The model still resets; a dataset can't span media. - Drop the two-letter code badges from the base-model cards, the selected-model panel and the multi-run rows. - Version heading matches the "Base model" heading, with space above it, so the version row reads as a second choice rather than part of the card above. - Drop the version sub-lines ("all purpose", "anime + realism", "pick a model"). - "Custom" rather than "Custom...", with its surcharge beside the label. - Tighten the sidebar gap between Labeling and the divider; hide the Models row when there is only one; shrink the warning icon to its line's size. - "Starting at" in white against a grey caption, reworded to say what it means. - Move "Nothing is saved..." below the summary panel and make Continue taller. Not pushed. This is Luis's open PR (#4701); how it lands is his call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VMK1EQxkfREacD6PWFU7t (cherry picked from commit 8de328a317f304a0d7218b952f9d1958cba5600c) |