Manuel Emilio Urena
ae0e86e4d7
* fix(account): correct the account settings v2 pane on tester feedback Testers on the v2 redesign reported eight issues in a Discord thread; this fixes all of them. - Overview rendered an arbitrary owned badge and nameplate rather than the equipped ones. `userProfile.get` returns every owned cosmetic, unlike `userWithCosmeticsSelect`, and `Username` takes the first of each type. - The Buzz total on Overview and in the mobile index counted only the domain's own account type, so blue was missing from the number itself, not merely from the breakdown. - The CivBot Assistant toggle and its personality select were separated by the Chats toggle, because the feature list renders in flag declaration order. They now share a section of their own. - The "Verify your email" alert claimed an unverified account cannot publish models, withdraw Buzz, or recover the account. Buzz withdrawal is not gated on email and there is no account recovery feature. It also keyed off `emailVerified` instead of `requiresEmailVerification`, so it showed to accounts that are gated on nothing. `VerifyEmailBanner` already covers the accounts that ARE gated, on the same page and with a working resend button, so this is removed rather than reworded. - Profile & Account masks the email behind a reveal toggle; the Overview card no longer shows an address at all. - Profile & Account gained a "Verify email" button wired to the existing `resendEmailVerification` procedure, which was previously reachable only from a banner that unstamped accounts never see. - Content & Browsing leads with Ads and moves the eye-button note inside the Mature content section; "Blur mature content" now sits directly below "Show mature content". - The Creator pane leads with Metric visibility instead of burying it under the sticker and remix sections. Also drops a commented-out mutation in AdContent and the `disabled` props that depended on it: the browsing-settings store persists itself. `useAvailableBuzz`'s JSDoc claimed `baseTypes` defaults to `['blue']`; it defaults to `[]`, which is what made the Buzz bug easy to write. Corrected, along with the two docs that a reviewer found pointing at the old shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NoyDuuoL1BM1KvSShYBWT * fix(account): tell email verification apart from an email change The verification link is minted by two flows, and everything downstream assumed the second one. `sendEmailVerification` mints a token carrying the address the account already has, so `newEmail` equals the current email. `/verify-email` read that as a change and rendered "From: x → To: x" over a "Yes, Change Email" button, and the email that sent the user there opened with "You requested to change your email address". Both flows are now told apart by comparing the token's address against the current row, rather than by a new field in the payload — tokens already issued keep working, and the page cannot disagree with what the write will actually do. A verify-only token gets "Confirm Your Email Address", the one address, and a "Verify Email" button; a change keeps the From/To it had. Also on this path: - The verification email carries the Civitai logo, using the header row the other templates already use, so it reads as official. - `/verify-email` refreshes the client session on success and on the way back to account settings. The server already busts its own cached session; the tab that clicked the link was the stale half, and the Email tile kept reading "Unverified" until a hard reload. - `emailVerificationEmail` is exported from the templates index. It was the only template missing, so `/api/testing/email/emailVerification` answered 404 and the template could not be previewed at all. Unrelated, on the Overview pane: the Buzz balance tile renders the shared `UserBuzz` component rather than summing the accounts itself, so it shows one blended total with the per-type split on hover. `UserBuzz` sets `lh={0}` on its number, which lifts it off the baseline the other three tiles share, so the tile overrides the line-height at that one call site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NoyDuuoL1BM1KvSShYBWT * refactor(utils): generalize maskEmail into a reusable maskString `maskEmail` was a one-caller module under `components/Account`, and nothing about it was specific to email — it keeps a prefix, keeps a suffix, and puts a fixed run of dots between them. `maskString(value, { start, end, mask })` in `string-helpers` does that for any string, and `maskEmail` is now a caller of it that works out where the domain starts. The old module is gone; `ProfileCard` imports from `~/utils/string-helpers`. The fixed-length run is the point and is now pinned by a test: a mask emitting one character per hidden character passes every other assertion while still telling a reader how long the secret is. Two behaviours worth naming, both covered: - Asking to keep more than the string holds returns the mask alone rather than the input untouched, so no caller can reveal a value by over-specifying. - An email whose local part is a single character masks entirely. `a•••••@example.com` would show the whole address behind decoy dots, which is worse than an obviously hidden value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NoyDuuoL1BM1KvSShYBWT --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
21 KiB
Feature Flag Truthy-Only Migration Plan
Goal
Stop sending false flag values from server → client. The wire payload becomes a sparse object containing only true flags, and the type forces every consumer through a truthiness check. Two wins:
- Smaller payload + smaller cache footprint — currently every flag for every user is serialized on every page load
- Compiler-enforced grep safety — when a flag is removed from the registry, the type system catches every consumer (including destructure sites that text grep misses)
Why this broke last time
Two consumer patterns silently changed behavior when the payload became partial:
- Strict-equality on
false—if (features.X === false)flips meaning whenfalsebecomesundefined - Existence checks —
if ('X' in features)orfeatures.X !== undefinedflip meaning the same way - TypeScript widening — anything passed to a function typed
(b: boolean) => ...stops compiling once the type istrue | undefined
We also have a unique-to-this-codebase wrinkle: getFeatureFlagsLazy in feature-flags.service.ts:422 defines getters via Object.defineProperty that route through obj.features. Need to confirm JSON.stringify walks those getters cleanly when the underlying object is partial.
Consumer landscape (audit data)
There are 203 useFeatureFlags() callsites across 179 files, plus server-side ctx.features reads.
Audit snapshot, 2026-05-12. Paths and counts below are not maintained — re-run the grep before relying on a row.
Destructure sites (36 total) — most vulnerable to type changes
These are the ones that aren't caught by grep "features\.X" — when removing a flag, you must also grep the destructure variable name in scope.
Client-side (useFeatureFlags()) — 34 files
Server-side (ctx.features) — 2 files
| File | Destructured flags |
|---|---|
| server/controllers/tag.controller.ts:44 | adminTags |
| server/controllers/buzz-withdrawal-request.controller.ts:101 | buzzWithdrawalTransfer |
Alias-pattern sites (~167 files)
Use const features = useFeatureFlags() then features.X. These behave identically under the destructure pattern at runtime, but they ARE caught by a features\.X text grep, so the search-and-delete workflow is safer for these.
Migration plan
Phase 1 — Audit & normalize (no behavior change) ✅ DONE
Goal: make every consumer treat the value as a truthy check so changing the type later is mechanical.
Step 1.1 — Forbid the dangerous patterns. Grepped for:
features\.\w+\s*(===|!==)\s*(false|true|undefined|null)
features\?\.\w+\s*(===|!==)\s*(false|true|undefined|null)
flags\.\w+\s*(===|!==)\s*(false|true|undefined|null)
'\w+'\s+in\s+features
Result: only two real matches, both in src/utils/training.ts:
| Was | Became |
|---|---|
if (features && features.kohyaTraining === false) (L530) |
if (features && !features.kohyaTraining) |
return features.kohyaTraining !== false; (L543) |
return !!features.kohyaTraining; |
These were the only call sites that distinguished "Flipt explicitly disabled" from "absent/undefined." Safe to swap because features.kohyaTraining is always present in today's payload (the flag is registered), so the semantic change is unrealizable in production.
The === true matches in generation.service.ts:591,779 and process-enqueued-comic-panels.ts:160 are on config.isGreen / metadata.isGreen — separate object types intentionally tri-state, not the FeatureAccess payload. Left alone.
Step 1.2 — getFeatureFlagsLazy audit. ✅ All callers are server-side (createContext.ts:83,105,132, image-scan-result.ts:177, image-scan-result.service.ts:120) and use only property access (features.X). No iteration, no JSON serialization, no === false. The non-enumerable getters won't cause serialization issues because nothing serializes the lazy object.
Step 1.3 — ENV overrides. ✅ getEnvOverrides() in feature-flags.service.ts:517 sets availability to a parsed array. The empty array case evaluates false for everyone today; under truthy-only that becomes "absent in payload" — same falsy semantics. No behavior change required.
Step 1.4 — Adjacent patterns confirmed safe.
- user.controller.ts:1284,1320 —
Object.keys(features)and...featureshere operate on the user-settings toggleable-preferences object (DB-stored), not the wire payload. Different namespace, safe. - user.controller.ts:1297 —
!ctx.features[key]already truthy-style - trpc.ts:217
isFlagProtecteduses!features[flag]— already truthy-style - constants.ts:1708,1718 already uses
features?.X ?? false— partial-aware (good pattern; recommend for new code) - data-graph context.ts:21 already typed as
Partial<FeatureAccess>— data-graph nodes already partial-aware
Step 1.5 — Convert destructure sites to alias pattern. ✅ All 36 destructure sites converted:
// Before
const { canViewNsfw } = useFeatureFlags();
if (!canViewNsfw) doStuff();
// After
const features = useFeatureFlags();
if (!features.canViewNsfw) doStuff();
This eliminates two risks ahead of Phase 3:
- Grep-detection blind spot: every flag read is now
features.X, so removing a flag from the registry can be confidently audited via text grep - Type widening at destructure: when
FeatureAccessbecomesPartial<Record<FeatureFlagKey, true>>in Phase 3, destructured locals would silently widen totrue | undefined. Property-access reads keep their type at the call site
Server-side ctx.features destructures (2 files: tag.controller.ts, buzz-withdrawal-request.controller.ts) were collapsed to inline ctx.features.X reads. Identifier collisions (helper-function parameters with same name in different scope, common-word substrings in URLs/strings, object-literal keys) were left alone where they don't reference the wire payload.
Step 1.6 — Lint rule (deferred to Phase 2). Add an ESLint rule banning === false / !== false / === undefined / 'X' in features against FeatureAccess-typed values, plus a rule banning destructure of useFeatureFlags() / ctx.features. Deferred because there are no current violations to lint against; the rule prevents regression once Phase 2 lands.
Phase 2 — Server: ship sparse payload ✅ DONE
getFeatureFlags now skips assignment for false values:
return keys.reduce<FeatureAccess>((acc, key) => {
if (hasFeature(key, ctx)) acc[key] = true;
return acc;
}, {} as FeatureAccess);
getFeatureFlagsAsync delegates to getFeatureFlags, so it inherits the change. getFeatureFlagsLazy reads from getFeatureFlags for its cached object, so the underlying lookup also returns undefined for absent keys (the getter still returns the value at that key — JavaScript-wise this is fine; the cast hides the type lie).
The FeatureAccess type intentionally stays as Record<FeatureFlagKey, boolean> for one deploy so client + server agree on the wire shape during rollout. The payload shrinks immediately; the type lies temporarily. Phase 3 will tighten it.
Why this is safe given Phase 1:
- Every consumer uses truthy checks (
if (features.X),!features.X) —undefinedevaluates the same asfalsein those expressions - No
Object.keys(features)/...featureson the wire payload (verified in Phase 1 — those patterns only appear on the user-settings preferences object) - No
=== false/!== falseconsumers (Phase 1 normalized the only two) isFlagProtectedmiddleware uses!features[flag]— works identically withundefinedgetUserFeatureFlagsHandlerat user.controller.ts:1297 uses!ctx.features[key]— also identical
Phase 3 — Client: tighten the type ❌ DECIDED AGAINST
Tried it. Changed FeatureAccess to Partial<Record<FeatureFlagKey, true>> and surfaced 13 type errors across the codebase — all boolean | undefined not assignable to boolean at boundaries where flag values flow into typed slots (context props, function params, predicate return types).
Why we backed out: the fixes all required !!features.X coercion, which adds friction at every boundary without a corresponding behavior gain. The runtime is already sparse from Phase 2; consumers already use truthy checks (Phase 1). Forcing every boundary to coerce was noise without payoff.
What we're keeping instead:
export type FeatureAccess = Record<FeatureFlagKey, boolean>;
The type "lies" — it claims every flag is present and boolean, when at runtime absent flags are undefined. The lie is benign because:
if (features.X)works for bothfalseandundefined!features.Xworks for bothfalseandundefined- Truthy ternary works the same way
- The wire payload is still sparse (Phase 2 stays — half the win was the bytes)
- The flag-removal safety net still works:
FeatureFlagKeyis the keyspace, so removing a flag from the registry shrinks the union and produces a type error at every consumer (including destructure sites)
What we lose: the type doesn't enforce write safety (features.X = false compiles even though wire payload never produces false). Acceptable — no consumer writes to the cache except SettingsCard.tsx:335, and that one assignment is internal optimistic-cache state, not the wire payload.
Phase 4 — Benefits realized
- Wire payload is smaller (most users have ~25 of ~80 flags true)
- Cache hit ratios improve since the payload is more uniform across users
- Removing a flag from the registry produces type errors at every consumer (destructure-blind-spot bug from Tier 1 is fixed) — this works because
FeatureFlagKeyis the keyspace, regardless ofRecordvsPartial<Record> - Consumers stay terse — no
!!coercion required at boundaries
Risks & gotchas
| Risk | Mitigation |
|---|---|
Some consumer is intentionally checking === false to distinguish from "not yet evaluated" |
Phase 1 audit; if found, refactor to use a different signal |
Server-side ctx.features.X checks in tRPC handlers |
The two known destructure sites (tag.controller.ts, buzz-withdrawal-request.controller.ts) get the same Partial type — type errors will surface on touch |
getFeatureFlagsLazy getter pattern + JSON serialization |
Test in dev that SSR hydration produces identical payloads in alias and lazy modes |
Third-party tooling reading __NEXT_DATA__.props.pageProps.features |
Probably nothing reads this externally, but worth confirming with the team before Phase 2 |
Recommended sequence
- Land Phase 1 as a single PR (audit + lint rule + any normalizations) — no behavior change, low risk
- Wait one deploy cycle, watch for any unexpected breakage in the lint warnings
- Land Phases 2 and 3 together as a single PR (server + type) — this is where the wire shape changes, and shipping them together avoids a brief window where the type lies about the payload