skills/civitai__civitai
1
0
Fork 0
mirror of https://github.com/civitai/civitai.git synced 2026-09-20 22:08:18 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor(apps): drop the frozen sub-nav predicate oracle (#4841)

Browse Source 
The test carried ORIGIN_MAIN_PREDICATES, a hand transcription of the six
`visible` expressions from AppsSubNav.tsx's SUB_NAV_LINKS as they stood at
merge-base 4aab099c91, and asserted the apps-sections.ts registry agreed across
a 1,024-case cohort sweep. That was the right guard for the migration and is
worthless now #4813 has landed: it deleted AppsSubNav.tsx, so the oracle has no
source to be a transcription of. Any legitimate future predicate edit reds it,
and the only fix would be hand-editing the oracle to match the implementation it
is supposed to be independently checking.

Also repoints the apps-sections.ts docstring that referenced it.

What still pins these predicates, verified rather than assumed:
appsSectionRowsMatchPageGates.test.ts evaluates the Marketplace, Build and
Activity predicates against their real page resolvers over the full flag space,
plus appsBuildAccess.test.ts and appsBuildGateCallSites.test.ts.

Two gaps named in-band rather than papered over: Invites, Revenue and Review are
in neither node-tier set — their only per-row behavioural pin is the report-only
browser tier — and the "visible to nobody" direction is no longer asserted. The
replacement guard was deliberately NOT widened here; that is a guard change and
does not belong in a deletion.

Unscoped unit project run against a control at origin/main: delta exactly
-1 file / -10 tests, the deleted file's own contents and nothing else.

Closes #4821
This commit is contained in:
Zachary Lowden
2026-09-14 18:45:54 -05:00
committed by GitHub
parent 95562f7755
commit d0d01fbc60
2 changed files with 15 additions and 195 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/components/Apps/__tests__/appsSectionsPredicates.test.ts
-193
View File
@@ -1,193 +0,0 @@
import { describe, expect, test } from 'vitest';
import type { AppsNavContext, AppsNavSummary } from '~/components/Apps/apps-sections';
import { appsSections, visibleAppsSections } from '~/components/Apps/apps-sections';
/**
* 🔒 THE PORT GUARD — `appsSections`' visibility predicates are BEHAVIOURALLY IDENTICAL
* to the `SUB_NAV_LINKS` table they were moved from.
*
* 🔴 WHY THIS FILE EXISTS. The `/apps` nav's tab strip became a left rail, which moved
* the table from `AppsSubNav.tsx` to `apps-sections.ts`. Every predicate in it is its
* page's own access gate restated as a viewer fact, and this table has shipped the SAME
* defect twice — #3899 ("Create" store-gated while `/apps/submit` is author-gated) and
* again on PR #4668 — both times because a predicate was written or rewritten separately
* from the gate it mirrors. A LAYOUT CHANGE IS THE MAXIMUM-LIKELIHOOD MOMENT FOR A THIRD
* INSTANCE: predicates get "cleaned up" while the file they live in is being retyped, and
* a widened one looks exactly like a tidier one in review.
*
* 🔴 THE ORACLE IS TRANSCRIBED FROM `origin/main`, NOT DERIVED FROM THE NEW MODULE.
* `ORIGIN_MAIN_PREDICATES` below is a hand-copy of `SUB_NAV_LINKS`' six `visible`
* expressions as they stand at `4aab099c91` — the commit this change is based on. It is
* an INDEPENDENT WITNESS: a test that re-ran the registry's own predicates would agree
* with any edit to them, which is precisely the failure this guard exists to prevent.
* When a predicate is deliberately changed, this file's oracle must be edited in the SAME
* commit and the change is then visible in the diff — which is the whole point.
*
* 🔴 THE WHOLE COHORT SPACE, NOT A SAMPLE. Seven summary booleans and three context
* booleans is 2^10 = 1024 viewers, and every one of them is compared. A sampled matrix
* cannot see a predicate that is wrong only for the cohort nobody thought to write down
* — which is the shape of all three historical instances.
*/
/** The seven `getNavSummary` booleans, in a fixed order so the enumeration is stable. */
const SUMMARY_KEYS = [
'hasInstalls',
'hasActivity',
'hasSubmissions',
'hasApprovedApps',
'isReviewer',
'hasEditableApps',
'hasPendingInvites',
] as const;
/** The three viewer capabilities. */
const CONTEXT_KEYS = ['isAuthor', 'canBuild', 'canSeeStore'] as const;
/**
* 🔴 THE ORACLE. Hand-transcribed from `SUB_NAV_LINKS` in
* `src/components/Apps/AppsSubNav.tsx` at `4aab099c91` (the merge-base of this change),
* keyed by the href each row pointed at. Do not "simplify" any of these — a simplified
* oracle silently stops being a witness to what shipped.
*/
const ORIGIN_MAIN_PREDICATES: Record<string, (s: AppsNavSummary, c: AppsNavContext) => boolean> = {
'/apps': (_s, c) => c.canSeeStore,
'/apps/activity': (s) => s.hasInstalls || s.hasActivity,
'/apps/invites': (s, c) => c.isAuthor && s.hasPendingInvites,
'/apps/revenue': (s) => s.hasApprovedApps,
'/apps/build': (_s, c) => c.canBuild,
'/apps/review': (s) => s.isReviewer,
};
/**
* The ORDER the rows appeared in at `4aab099c91` — discovery → manage → revenue → build
* → moderate. Pinned separately from the predicates because a re-order changes which
* destination reads as "what this nav is for" and is a product decision, not a detail.
*/
const ORIGIN_MAIN_ORDER = [
'/apps',
'/apps/activity',
'/apps/invites',
'/apps/revenue',
'/apps/build',
'/apps/review',
] as const;
/** `section.path` → the href it resolves to, mirroring `getAppsSectionHref`. */
const hrefOf = (path: string) => (path ? `/apps/${path}` : '/apps');
/** Every (summary × context) viewer, as a flat list. 2^7 × 2^3 = 1024. */
function everyCohort(): Array<{ summary: AppsNavSummary; context: AppsNavContext }> {
const out: Array<{ summary: AppsNavSummary; context: AppsNavContext }> = [];
for (let s = 0; s < 1 << SUMMARY_KEYS.length; s += 1) {
const summary = Object.fromEntries(
SUMMARY_KEYS.map((k, i) => [k, Boolean(s & (1 << i))])
) as unknown as AppsNavSummary;
for (let c = 0; c < 1 << CONTEXT_KEYS.length; c += 1) {
const context = Object.fromEntries(
CONTEXT_KEYS.map((k, i) => [k, Boolean(c & (1 << i))])
) as unknown as AppsNavContext;
out.push({ summary, context });
}
}
return out;
}
const describeCohort = (summary: AppsNavSummary, context: AppsNavContext) =>
[...SUMMARY_KEYS.filter((k) => summary[k]), ...CONTEXT_KEYS.filter((k) => context[k])].join(
'+'
) || '(nothing)';
describe('the extractor and the oracle (validate the instrument first)', () => {
test('the cohort enumeration really is the whole space', () => {
// A loop over an empty or truncated set is a clean pass over nothing — the reassuring
// zero this whole file would otherwise be vulnerable to.
const cohorts = everyCohort();
expect(cohorts).toHaveLength(1024);
// …and the cohorts are DISTINCT, so 1024 is not one viewer counted 1024 times.
const seen = new Set(cohorts.map((x) => describeCohort(x.summary, x.context)));
expect(seen.size).toBe(1024);
});
test('🔴 NEGATIVE CONTROL: the comparison can go RED', () => {
// Feed the oracle a deliberately-wrong predicate for one row and confirm the
// cell-by-cell comparison below would notice. Without this, "every cohort agrees"
// is indistinguishable from a comparison wired to nothing.
const broken = {
...ORIGIN_MAIN_PREDICATES,
'/apps/invites': (s: AppsNavSummary) => s.hasPendingInvites,
};
const disagreements = everyCohort().filter(({ summary, context }) => {
const actual = appsSections
.filter((x) => x.visible(summary, context))
.map((x) => hrefOf(x.path));
const expected = ORIGIN_MAIN_ORDER.filter((href) => broken[href](summary, context));
return actual.join(',') !== expected.join(',');
});
// The mutant widens Invites to every non-author with a pending invite, so it must
// disagree on a non-trivial number of cohorts — not merely "at least one".
expect(disagreements.length).toBeGreaterThan(100);
});
test('the registry rows are exactly the rows the oracle knows about, in order', () => {
// The per-cohort comparison below is keyed by href; a row that vanished from the
// registry would simply never be compared. This is what makes the set exact.
expect(appsSections.map((s) => hrefOf(s.path))).toEqual([...ORIGIN_MAIN_ORDER]);
expect(Object.keys(ORIGIN_MAIN_PREDICATES).sort()).toEqual([...ORIGIN_MAIN_ORDER].sort());
});
});
describe('🔴 every predicate is behaviourally unchanged from `origin/main`', () => {
test('all 1024 cohorts resolve to the identical section set', () => {
const disagreements: string[] = [];
for (const { summary, context } of everyCohort()) {
const actual = visibleAppsSections(summary, context).map((s) => hrefOf(s.path));
const expected = ORIGIN_MAIN_ORDER.filter((href) =>
ORIGIN_MAIN_PREDICATES[href](summary, context)
);
if (actual.join(',') !== expected.join(',')) {
disagreements.push(
`${describeCohort(summary, context)}: registry=[${actual.join(', ')}] ` +
`origin/main=[${expected.join(', ')}]`
);
}
}
expect(
disagreements.slice(0, 10),
`${disagreements.length} cohort(s) resolve to a DIFFERENT section set than the ` +
'`SUB_NAV_LINKS` table this registry was ported from. Every predicate here is its ' +
"page's own gate restated as a viewer fact; #3899 and PR #4668 were both a " +
'predicate rewritten separately from that gate. If the change is deliberate, edit ' +
'ORIGIN_MAIN_PREDICATES in this same commit so the diff shows what moved.'
).toEqual([]);
});
/**
* 🔴 PER-ROW, SO A FAILURE NAMES THE PREDICATE. The test above proves the SET matches
* and would report a single cohort touching two rows as one line; this one attributes a
* disagreement to the row that caused it. Both are cheap and they fail differently.
*/
test.each([...ORIGIN_MAIN_ORDER])('%s: the predicate itself is unchanged', (href) => {
const section = appsSections.find((s) => hrefOf(s.path) === href);
expect(section, `${href} is missing from appsSections`).toBeDefined();
const oracle = ORIGIN_MAIN_PREDICATES[href];
let sawTrue = false;
let sawFalse = false;
for (const { summary, context } of everyCohort()) {
const actual = section!.visible(summary, context);
expect(
actual,
`${href} @ ${describeCohort(summary, context)}: the ported predicate answers ` +
`${actual} where \`origin/main\` answered ${oracle(summary, context)}`
).toBe(oracle(summary, context));
if (actual) sawTrue = true;
else sawFalse = true;
}
// 🔴 AND THE PREDICATE IS STILL CONDITIONAL. A row that became `() => true` (or
// `() => false`) would agree with an oracle mutated the same way, and every
// assertion above would pass. Marketplace was `() => true` for months and that was
// exactly the #4668-class exposure.
expect(sawTrue, `${href} is visible to NOBODY`).toBe(true);
expect(sawFalse, `${href} is visible to EVERYONE`).toBe(true);
});
});
src/components/Apps/apps-sections.ts
+15 -2
View File
@@ -200,8 +200,21 @@ export type AppsSection = {
* TAB STRIP BECAME THIS RAIL — not re-derived, not simplified, not "cleaned up". A
* layout change is exactly the occasion on which a predicate gets rewritten for tidiness
* and quietly widens, which is the #3899/#4668 class arriving a third time.
* `__tests__/appsSectionsPredicates.test.ts` pins each one against the full cohort
* matrix and fails if any is edited.
* WHAT PINS THEM NOW, AND WHAT DOES NOT. The migration guard that carried a frozen
* transcription of `SUB_NAV_LINKS` was deleted with #4821 — `AppsSubNav.tsx` no longer
* exists, so it had no source left to be a transcription of. In its place:
* • `__tests__/appsSectionRowsMatchPageGates.test.ts` (node tier, blocking) extracts
* each row's `visible` expression from THIS file and evaluates it — Marketplace
* against `resolveAppsPageAccess`, Build against `resolveBuildPageAccess`, Activity
* against `resolveActivityPageAccess` / `canAccessAppsActivity` — and fails if ANY
* row in the table becomes unconditionally visible.
* • `__tests__/appsBuildAccess.test.ts` and `__tests__/appsBuildGateCallSites.test.ts`
* cover the Build row behaviourally and structurally.
* ⚠️ Invites, Revenue and Review are in NEITHER node-tier set. Their only per-row
* behavioural pin is `AppsRailNav.browser.test.tsx`, which runs in the report-only
* browser tier — so editing one of those three predicates is not blocked by anything.
* If one gains a flag term, add it to `appsSectionRowsMatchPageGates.test.ts` rather
* than relying on review.
*
* "Build" is the strongest form of the fix available: its predicate is not restated
* here at all, it is the SHARED `canAccessAppsBuild`, which the page's