Fix five defects found by running Motion rather than reading it
The markdown parser was in the eager entry chunk. `manualChunks` in its object form does not leave an unlisted vendor package to Vite's async splitting, so react-markdown was hoisted into the entry even though its only importers are lazy routes — 327.70 kB gzip against a 314 kB baseline, on the one download every route pays for. Naming it as its own chunk puts it back behind the Motion pages and takes the entry to 282.21 kB, below where it was before Motion existed. The starter library could never be improved. Seeding was insert-only, so a deployment seeded in August was frozen on August's wording for ever with no upgrade path short of editing production rows by hand — for a feature whose entire premise is that the library gets better. A second run now refreshes a starter row, but only while it is still ours: `is_system`, `usage_count = 0` and no owner. That is the same condition §7a already enforces on the API, so a template an engagement was cut from is left alone and reported by name rather than silently overwritten. The refresh was not idempotent, and the seed lied about it. `jsonb` does not preserve key order — Postgres sorts keys by length then bytewise — so comparing `JSON.stringify(stored)` against `JSON.stringify(authored)` marked every template as changed on every run, and the seed rewrote nine rows each time while reporting itself clean. Comparison is now canonical. Found by running the seed three times and reading the counts. `motion-overflow-check.mjs` measured less than it claimed. It seeded `pig.sidebar` and `pig.piggy.dock`, neither of which anything reads (the keys are `pig.sidebarOpen` and `pig.piggyDockOpen`), so the layout it pinned was whatever the last run left. Its dark pass set `colorScheme` only, and the appearance preference is stored server-side and adopted after hydration, so the dark pass measured the light palette a beat after first paint. It now rewrites the profile response as `screenshots.mjs` does, asserts the rendered `data-theme`, and fails a page that renders almost no text — a page that throws inside its own body otherwise measures zero overflow and passes. The stage rail rendered "1 templates", in the visible label and in every aria-label. Singular and plural are now both passed. Also normalised `artifact` to `artefact` in the seeded prose, which had drifted American in the playbook. The `artifacts` field key is untouched: FieldsView reads it, and already labels it in British.
This commit is contained in:
@@ -18,12 +18,17 @@
|
|||||||
import { DEMAND_OPEN_STAGES, DEMAND_STAGE_LABELS, type DemandStage } from '@pig/core';
|
import { DEMAND_OPEN_STAGES, DEMAND_STAGE_LABELS, type DemandStage } from '@pig/core';
|
||||||
import { cn } from '@/components/ui';
|
import { cn } from '@/components/ui';
|
||||||
|
|
||||||
|
/** Zero takes the plural, as it does in English: "no templates", "0 templates". */
|
||||||
|
function noun(count: number, [one, many]: readonly [string, string]): string {
|
||||||
|
return count === 1 ? one : many;
|
||||||
|
}
|
||||||
|
|
||||||
export function StageRail({
|
export function StageRail({
|
||||||
counts,
|
counts,
|
||||||
coverage,
|
coverage,
|
||||||
stages = DEMAND_OPEN_STAGES,
|
stages = DEMAND_OPEN_STAGES,
|
||||||
countLabel = 'engagements',
|
countNoun = ['engagement', 'engagements'],
|
||||||
coverageLabel = 'templates',
|
coverageNoun = ['template', 'templates'],
|
||||||
activeStage,
|
activeStage,
|
||||||
onSelect,
|
onSelect,
|
||||||
className,
|
className,
|
||||||
@@ -33,8 +38,9 @@ export function StageRail({
|
|||||||
/** The second figure, if the caller has one — library cover, typically. */
|
/** The second figure, if the caller has one — library cover, typically. */
|
||||||
coverage?: Partial<Record<DemandStage, number>>;
|
coverage?: Partial<Record<DemandStage, number>>;
|
||||||
stages?: readonly DemandStage[];
|
stages?: readonly DemandStage[];
|
||||||
countLabel?: string;
|
/** Singular and plural, because "1 engagements" is read aloud by a screen reader. */
|
||||||
coverageLabel?: string;
|
countNoun?: readonly [one: string, many: string];
|
||||||
|
coverageNoun?: readonly [one: string, many: string];
|
||||||
activeStage?: DemandStage | null;
|
activeStage?: DemandStage | null;
|
||||||
/** Omit to render a read-only rail: a non-interactive button is a trap. */
|
/** Omit to render a read-only rail: a non-interactive button is a trap. */
|
||||||
onSelect?: (stage: DemandStage) => void;
|
onSelect?: (stage: DemandStage) => void;
|
||||||
@@ -61,7 +67,7 @@ export function StageRail({
|
|||||||
// surface — it is where the motion stops repeating — so it is
|
// surface — it is where the motion stops repeating — so it is
|
||||||
// called out rather than shown as another grey zero.
|
// called out rather than shown as another grey zero.
|
||||||
<span className={cn('nums truncate text-xs', covered === 0 ? 'text-warning' : 'text-muted')}>
|
<span className={cn('nums truncate text-xs', covered === 0 ? 'text-warning' : 'text-muted')}>
|
||||||
{covered === 0 ? `No ${coverageLabel}` : `${covered} ${coverageLabel}`}
|
{covered === 0 ? `No ${coverageNoun[1]}` : `${covered} ${noun(covered, coverageNoun)}`}
|
||||||
</span>
|
</span>
|
||||||
)}
|
)}
|
||||||
</>
|
</>
|
||||||
@@ -79,13 +85,13 @@ export function StageRail({
|
|||||||
type="button"
|
type="button"
|
||||||
onClick={() => onSelect(stage)}
|
onClick={() => onSelect(stage)}
|
||||||
aria-pressed={active}
|
aria-pressed={active}
|
||||||
aria-label={`${DEMAND_STAGE_LABELS[stage]}: ${count} ${countLabel}`}
|
aria-label={`${DEMAND_STAGE_LABELS[stage]}: ${count} ${noun(count, countNoun)}`}
|
||||||
className={cn(shape, 'transition-colors hover:bg-surface-2')}
|
className={cn(shape, 'transition-colors hover:bg-surface-2')}
|
||||||
>
|
>
|
||||||
{body}
|
{body}
|
||||||
</button>
|
</button>
|
||||||
) : (
|
) : (
|
||||||
<div className={shape} aria-label={`${DEMAND_STAGE_LABELS[stage]}: ${count} ${countLabel}`}>
|
<div className={shape} aria-label={`${DEMAND_STAGE_LABELS[stage]}: ${count} ${noun(count, countNoun)}`}>
|
||||||
{body}
|
{body}
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -314,7 +314,7 @@ export function Engagement() {
|
|||||||
<StageRail
|
<StageRail
|
||||||
counts={counts}
|
counts={counts}
|
||||||
stages={railStages}
|
stages={railStages}
|
||||||
countLabel="artefacts"
|
countNoun={['artefact', 'artefacts']}
|
||||||
activeStage={stageFilter ?? engagement.stage}
|
activeStage={stageFilter ?? engagement.stage}
|
||||||
onSelect={(stage) => setStageFilter((current) => (current === stage ? null : stage))}
|
onSelect={(stage) => setStageFilter((current) => (current === stage ? null : stage))}
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -180,7 +180,7 @@ export function MotionLibrary() {
|
|||||||
|
|
||||||
<StageRail
|
<StageRail
|
||||||
counts={railCounts}
|
counts={railCounts}
|
||||||
countLabel="templates"
|
countNoun={['template', 'templates']}
|
||||||
activeStage={filters.stage}
|
activeStage={filters.stage}
|
||||||
onSelect={(stage) =>
|
onSelect={(stage) =>
|
||||||
setParam(setParams, STAGE_PARAM, stage === filters.stage ? '' : stage)
|
setParam(setParams, STAGE_PARAM, stage === filters.stage ? '' : stage)
|
||||||
|
|||||||
@@ -44,6 +44,14 @@ export default defineConfig({
|
|||||||
react: ['react', 'react-dom', 'react-router-dom'],
|
react: ['react', 'react-dom', 'react-router-dom'],
|
||||||
supabase: ['@supabase/supabase-js'],
|
supabase: ['@supabase/supabase-js'],
|
||||||
query: ['@tanstack/react-query'],
|
query: ['@tanstack/react-query'],
|
||||||
|
/*
|
||||||
|
* Named explicitly, or the object form of `manualChunks` hoists the
|
||||||
|
* markdown parser into the entry chunk even though the only importers
|
||||||
|
* are lazy routes — measured at 327.70 kB gzip entry with it hoisted
|
||||||
|
* against 314 kB without. As its own chunk it is fetched when a
|
||||||
|
* Motion page is opened and never on any other route.
|
||||||
|
*/
|
||||||
|
markdown: ['react-markdown', 'remark-gfm'],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -350,9 +350,16 @@ async function seed() {
|
|||||||
// ------------------------------------------------ the Motion starter library
|
// ------------------------------------------------ the Motion starter library
|
||||||
const motion = await seedMotionLibrary(db);
|
const motion = await seedMotionLibrary(db);
|
||||||
console.log(
|
console.log(
|
||||||
` ${motion.total} Motion starter template(s) (${motion.added} new) — authored content, ` +
|
` ${motion.total} Motion starter template(s) (${motion.added} new, ${motion.refreshed} ` +
|
||||||
'shared and system-owned, version 1 of their lineages.',
|
'refreshed) — authored content, shared and system-owned, version 1 of their lineages.',
|
||||||
);
|
);
|
||||||
|
if (motion.held.length) {
|
||||||
|
console.log(
|
||||||
|
` ${motion.held.length} starter template(s) have newer wording here and were LEFT ALONE, ` +
|
||||||
|
`because they are in use or have been adopted: ${motion.held.join(', ')}. ` +
|
||||||
|
'Publish a new version rather than editing one an engagement was cut from.',
|
||||||
|
);
|
||||||
|
}
|
||||||
for (const bad of motion.rejected) {
|
for (const bad of motion.rejected) {
|
||||||
console.error(` SKIPPED a starter template with a value outside the ontology: ${bad}`);
|
console.error(` SKIPPED a starter template with a value outside the ontology: ${bad}`);
|
||||||
}
|
}
|
||||||
|
|||||||
File diff suppressed because one or more lines are too long
@@ -19,12 +19,22 @@
|
|||||||
* against this to confirm the attribute syntax is understood by each, since
|
* against this to confirm the attribute syntax is understood by each, since
|
||||||
* the server runs the TypeScript directly.
|
* the server runs the TypeScript directly.
|
||||||
*
|
*
|
||||||
* Idempotency is `onConflictDoNothing({ target: [slug, version] })`, which
|
* Idempotency is keyed on `(slug, version)`, which is enforceable **only**
|
||||||
* works **only** because of `motion_templates_slug_version_key`. AGENTS.md §5
|
* because of `motion_templates_slug_version_key`. AGENTS.md §5 is blunt about
|
||||||
* is blunt about what that clause does without a constraint to fire on — it
|
* what `onConflictDoNothing` does without a constraint to fire on — it silently
|
||||||
* silently duplicated seed data here twice — so `assertNoDuplicates` below
|
* duplicated seed data here twice — so `assertNoDuplicates` below checks the
|
||||||
* checks the outcome rather than trusting the schema, and CI counts this table
|
* outcome rather than trusting the schema, and CI counts this table in its
|
||||||
* in its idempotency gate alongside `contacts`.
|
* idempotency gate alongside `contacts`.
|
||||||
|
*
|
||||||
|
* A second run REFRESHES a starter row rather than skipping it, but only while
|
||||||
|
* that row is still ours: `is_system`, nobody has forked or instantiated it
|
||||||
|
* (`usage_count = 0`) and it has no owner. Insert-only was the first version and
|
||||||
|
* is wrong for the one thing this library is for — the content improves, and a
|
||||||
|
* deployment seeded in August would otherwise be frozen on August's wording for
|
||||||
|
* ever, with no upgrade path short of hand-editing production rows. The
|
||||||
|
* pristine test is what keeps that from becoming a write over somebody's work:
|
||||||
|
* the moment a template has been used, §7a says it is never edited in place,
|
||||||
|
* and this respects that with the same condition the API enforces.
|
||||||
*/
|
*/
|
||||||
import { DEMAND_STAGES, isMotionKind, type DemandStage, type MotionKind } from '@pig/core';
|
import { DEMAND_STAGES, isMotionKind, type DemandStage, type MotionKind } from '@pig/core';
|
||||||
import { and, eq, inArray } from 'drizzle-orm';
|
import { and, eq, inArray } from 'drizzle-orm';
|
||||||
@@ -89,6 +99,29 @@ const isDemandStage = (value: string): value is DemandStage =>
|
|||||||
* inside `fields` means promotion carries it forward with the rest of the
|
* inside `fields` means promotion carries it forward with the rest of the
|
||||||
* structured payload without any special handling in the promote path.
|
* structured payload without any special handling in the promote path.
|
||||||
*/
|
*/
|
||||||
|
/*
|
||||||
|
* Key order is not preserved by `jsonb`: Postgres stores an object with its
|
||||||
|
* keys sorted by length and then bytewise, so the value that comes back is
|
||||||
|
* rarely the value that went in. A plain `JSON.stringify` comparison therefore
|
||||||
|
* reported every starter template as changed on every run, and the seed
|
||||||
|
* rewrote nine rows each time while claiming to be idempotent. Found by
|
||||||
|
* running it twice and reading the count, not by reading the code.
|
||||||
|
*/
|
||||||
|
function canonical(value: unknown): string {
|
||||||
|
const order = (node: unknown): unknown => {
|
||||||
|
if (Array.isArray(node)) return node.map(order);
|
||||||
|
if (node && typeof node === 'object') {
|
||||||
|
return Object.fromEntries(
|
||||||
|
Object.keys(node as Record<string, unknown>)
|
||||||
|
.sort()
|
||||||
|
.map((key) => [key, order((node as Record<string, unknown>)[key])]),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return node;
|
||||||
|
};
|
||||||
|
return JSON.stringify(order(value));
|
||||||
|
}
|
||||||
|
|
||||||
function toRow(
|
function toRow(
|
||||||
template: StarterTemplate,
|
template: StarterTemplate,
|
||||||
kind: MotionKind,
|
kind: MotionKind,
|
||||||
@@ -144,9 +177,17 @@ async function assertNoDuplicates(db: Database, slugs: readonly string[]): Promi
|
|||||||
|
|
||||||
export async function seedMotionLibrary(
|
export async function seedMotionLibrary(
|
||||||
db: Database,
|
db: Database,
|
||||||
): Promise<{ total: number; added: number; rejected: readonly string[] }> {
|
): Promise<{
|
||||||
|
total: number;
|
||||||
|
added: number;
|
||||||
|
refreshed: number;
|
||||||
|
held: readonly string[];
|
||||||
|
rejected: readonly string[];
|
||||||
|
}> {
|
||||||
const rejected: string[] = [];
|
const rejected: string[] = [];
|
||||||
|
const held: string[] = [];
|
||||||
let added = 0;
|
let added = 0;
|
||||||
|
let refreshed = 0;
|
||||||
|
|
||||||
for (const template of STARTER_LIBRARY) {
|
for (const template of STARTER_LIBRARY) {
|
||||||
/*
|
/*
|
||||||
@@ -161,12 +202,64 @@ export async function seedMotionLibrary(
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const row = toRow(template, template.kind, template.stage);
|
||||||
|
|
||||||
|
const [existing] = await db
|
||||||
|
.select({
|
||||||
|
id: motionTemplates.id,
|
||||||
|
isSystem: motionTemplates.isSystem,
|
||||||
|
usageCount: motionTemplates.usageCount,
|
||||||
|
ownerUserId: motionTemplates.ownerUserId,
|
||||||
|
title: motionTemplates.title,
|
||||||
|
summary: motionTemplates.summary,
|
||||||
|
body: motionTemplates.body,
|
||||||
|
fields: motionTemplates.fields,
|
||||||
|
kind: motionTemplates.kind,
|
||||||
|
stage: motionTemplates.stage,
|
||||||
|
})
|
||||||
|
.from(motionTemplates)
|
||||||
|
.where(and(eq(motionTemplates.slug, template.slug), eq(motionTemplates.version, 1)))
|
||||||
|
.limit(1);
|
||||||
|
|
||||||
|
if (!existing) {
|
||||||
const [created] = await db
|
const [created] = await db
|
||||||
.insert(motionTemplates)
|
.insert(motionTemplates)
|
||||||
.values(toRow(template, template.kind, template.stage))
|
.values(row)
|
||||||
.onConflictDoNothing({ target: [motionTemplates.slug, motionTemplates.version] })
|
.onConflictDoNothing({ target: [motionTemplates.slug, motionTemplates.version] })
|
||||||
.returning({ id: motionTemplates.id });
|
.returning({ id: motionTemplates.id });
|
||||||
if (created) added += 1;
|
if (created) added += 1;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
const pristine =
|
||||||
|
existing.isSystem && existing.usageCount === 0 && existing.ownerUserId === null;
|
||||||
|
const changed =
|
||||||
|
existing.title !== row.title ||
|
||||||
|
existing.summary !== row.summary ||
|
||||||
|
existing.body !== row.body ||
|
||||||
|
existing.kind !== row.kind ||
|
||||||
|
existing.stage !== row.stage ||
|
||||||
|
canonical(existing.fields) !== canonical(row.fields);
|
||||||
|
|
||||||
|
if (pristine && changed) {
|
||||||
|
await db
|
||||||
|
.update(motionTemplates)
|
||||||
|
.set({
|
||||||
|
title: row.title,
|
||||||
|
summary: row.summary,
|
||||||
|
body: row.body,
|
||||||
|
fields: row.fields,
|
||||||
|
kind: row.kind,
|
||||||
|
stage: row.stage,
|
||||||
|
updatedAt: new Date(),
|
||||||
|
})
|
||||||
|
.where(eq(motionTemplates.id, existing.id));
|
||||||
|
refreshed += 1;
|
||||||
|
} else if (changed) {
|
||||||
|
// Reported rather than forced: the row has been used or adopted, so
|
||||||
|
// overwriting it would edit a template a live engagement was cut from.
|
||||||
|
held.push(template.slug);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const total = await assertNoDuplicates(
|
const total = await assertNoDuplicates(
|
||||||
@@ -174,5 +267,5 @@ export async function seedMotionLibrary(
|
|||||||
STARTER_LIBRARY.map((template) => template.slug),
|
STARTER_LIBRARY.map((template) => template.slug),
|
||||||
);
|
);
|
||||||
|
|
||||||
return { total, added, rejected };
|
return { total, added, refreshed, held, rejected };
|
||||||
}
|
}
|
||||||
|
|||||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -49,12 +49,31 @@ for (const theme of ['light', 'dark']) {
|
|||||||
isMobile: vp.name === 'mobile',
|
isMobile: vp.name === 'mobile',
|
||||||
hasTouch: vp.name === 'mobile',
|
hasTouch: vp.name === 'mobile',
|
||||||
});
|
});
|
||||||
// The sidebar and the dock are per-device states in localStorage; left to
|
/*
|
||||||
// whatever a human last set, the measurement is not reproducible.
|
* The sidebar and the dock are per-device states in localStorage; left to
|
||||||
|
* whatever a human last set, the measurement is not reproducible. The key
|
||||||
|
* names are `pig.sidebarOpen` and `pig.piggyDockOpen` — an earlier version
|
||||||
|
* of this file invented `pig.sidebar` and `pig.piggy.dock`, which set two
|
||||||
|
* keys nothing reads and left the real state untouched, so the run looked
|
||||||
|
* pinned and was not.
|
||||||
|
*/
|
||||||
await ctx.addInitScript(`
|
await ctx.addInitScript(`
|
||||||
localStorage.setItem('pig.sidebar', 'expanded');
|
localStorage.setItem('pig.sidebarOpen', 'true');
|
||||||
localStorage.setItem('pig.piggy.dock', 'closed');
|
localStorage.setItem('pig.piggyDockOpen', 'false');
|
||||||
|
localStorage.setItem('pig.themeMode', '${theme}');
|
||||||
`);
|
`);
|
||||||
|
/*
|
||||||
|
* colorScheme alone only sets prefers-color-scheme, and the appearance
|
||||||
|
* preference is stored SERVER-side and adopted after hydration — so the
|
||||||
|
* dark pass rendered light a beat after first paint and measured the wrong
|
||||||
|
* theme. Rewriting the profile response is what screenshots.mjs does, and
|
||||||
|
* for the same reason.
|
||||||
|
*/
|
||||||
|
await ctx.route('**/api/me/profile', async (route) => {
|
||||||
|
const response = await route.fetch();
|
||||||
|
const body = await response.json().catch(() => ({}));
|
||||||
|
await route.fulfill({ json: { ...body, themeMode: theme } });
|
||||||
|
});
|
||||||
|
|
||||||
for (const route of ROUTES) {
|
for (const route of ROUTES) {
|
||||||
const page = await ctx.newPage();
|
const page = await ctx.newPage();
|
||||||
@@ -90,11 +109,27 @@ for (const theme of ['light', 'dark']) {
|
|||||||
.split(' ')
|
.split(' ')
|
||||||
.slice(0, 4)
|
.slice(0, 4)
|
||||||
.join('.')}`);
|
.join('.')}`);
|
||||||
return { overflow, wide, text: document.body.innerText.slice(0, 120) };
|
return {
|
||||||
|
overflow,
|
||||||
|
wide,
|
||||||
|
theme: doc.getAttribute('data-theme'),
|
||||||
|
chars: document.body.innerText.trim().length,
|
||||||
|
text: document.body.innerText.slice(0, 120),
|
||||||
|
};
|
||||||
});
|
});
|
||||||
|
|
||||||
const label = `${route.slug} ${vp.name} ${theme}`;
|
const label = `${route.slug} ${vp.name} ${theme}`;
|
||||||
if (measured.overflow !== 0) {
|
if (measured.theme !== theme) {
|
||||||
|
// Without this the dark pass silently measured the light palette, and a
|
||||||
|
// dark-only overflow — a wider border, a different font fallback —
|
||||||
|
// would have gone unseen while the run reported ok.
|
||||||
|
failures++;
|
||||||
|
console.log(`FAIL ${label}: rendered data-theme=${measured.theme}, not ${theme}`);
|
||||||
|
} else if (measured.chars < 120) {
|
||||||
|
// A page that throws inside its own body still measures zero overflow.
|
||||||
|
failures++;
|
||||||
|
console.log(`FAIL ${label}: rendered only ${measured.chars} characters — the page is blank`);
|
||||||
|
} else if (measured.overflow !== 0) {
|
||||||
failures++;
|
failures++;
|
||||||
console.log(`FAIL ${label}: overflows by ${measured.overflow}px — ${measured.wide.join(', ')}`);
|
console.log(`FAIL ${label}: overflows by ${measured.overflow}px — ${measured.wide.join(', ')}`);
|
||||||
} else if (problems.length) {
|
} else if (problems.length) {
|
||||||
|
|||||||
Reference in New Issue
Block a user