feat(ui): <DetailPageShell> primitive — third primitive after DataTable + EntitySelect

docs/01_architecture/ui-architecture.md

@@ -261,7 +261,30 @@ The same SSE wire format (`event: <type>\ndata: <json>\n\n`) is used; only the t
 
 **Source-of-truth discipline (forms).** The vitest lint guard at [`ui/src/__tests__/components/common/form-select-discipline.test.tsx`](../../ui/src/__tests__/components/common/form-select-discipline.test.tsx) scans every form `*.tsx` under `ui/src/components/` (excluding `__tests__/`, `common/`, and `*.column-config.{ts,tsx}`) and fails when a file imports `SelectItem` from `'@/components/ui/select'` AND inlines `<SelectItem value="<literal>">` where `<literal>` matches any backend enum wire value defined in [`enums.ts`](../../ui/src/lib/enums.ts). Escape hatch: a top-of-file `// no-enum-import: <non-empty reason>` comment. This is the form-level complement to the DataTable column-discipline guard — together they cover every UI surface where the frontend ships a wire value back to the backend.
 
-**Modal-level testing.** The shadcn `<Select>` family crashes inside jsdom + Dialog focus traps (Radix `patchedFocus` infinite recursion). Every modal test that exercises an `<EntitySelect>` ships a `vi.mock('@/components/ui/select', ...)` block that replaces the Radix primitives with a native `<select>` shim — the canonical pattern is at the top of [`ui/src/__tests__/components/studies/create-study-modal.test.tsx`](../../ui/src/__tests__/components/studies/create-study-modal.test.tsx) (the form-side mock additionally forwards `data-testid`). The unit-level `<EntitySelect>` tests at [`ui/src/__tests__/components/common/entity-select.test.tsx`](../../ui/src/__tests__/components/common/entity-select.test.tsx) run against the real Radix primitives (no Dialog wrapper, no patchedFocus recursion).
+**Modal-level testing.** The shadcn `<Select>` family crashes inside jsdom + Dialog focus traps (Radix `patchedFocus` infinite recursion). Every modal test that exercises an `<EntitySelect>` ships a `vi.mock('@/components/ui/select', ...)` block that replaces the Radix primitives with a native `<select>` shim — the canonical helper lives at [`ui/src/__tests__/helpers/shadcn-select-mock.tsx`](../../ui/src/__tests__/helpers/shadcn-select-mock.tsx) (`chore_extract_shadcn_select_test_mock`, 2026-05-19) and the per-test usage pattern is a 3-line dynamic-`import()` inside `vi.mock` to sidestep vitest's hoisting rule. The unit-level `<EntitySelect>` tests at [`ui/src/__tests__/components/common/entity-select.test.tsx`](../../ui/src/__tests__/components/common/entity-select.test.tsx) run against the real Radix primitives (no Dialog wrapper, no patchedFocus recursion).
+
+## Detail page shell primitive
+
+`chore_detail_page_shell_primitive` (2026-05-19) consolidates the `isPending → isError → data` scaffolding shared by six `/{entity}/[id]` detail routes into one primitive at [`ui/src/components/common/detail-page-shell.tsx`](../../ui/src/components/common/detail-page-shell.tsx). Pre-migration, each of `clusters/[id]`, `studies/[id]`, `proposals/[id]`, `query-sets/[id]`, `templates/[id]`, and `judgments/[id]` hand-rolled the same ternary with identical className strings and slightly inconsistent copy ("deleted" vs "removed") — and only `proposals/[id]` bothered to discriminate 404 from network error. The primitive flattens that into one place.
+
+**Shape.** `<DetailPageShell<T>>` is generic over the resource type. Required props: `query: UseQueryResult<T, ApiError>` (the consumer's TanStack hook return), `entityLabel: string` (singular, e.g. `"study"`), `notFoundErrorCode: string` (e.g. `"STUDY_NOT_FOUND"` — matches the backend's `error_code` value, not HTTP status, per `api-errors.ts`). Optional props: `entityTitle?: string` (override default title-casing of `entityLabel`), `notFoundMessage?: string` and `unreachableMessage?: string` (override default copy). Children-as-function — consumer's render fires with the loaded data:
+
+```tsx
+<DetailPageShell query={studyQ} entityLabel="study" notFoundErrorCode="STUDY_NOT_FOUND">
+  {(study) => (
+    <>
+      <StudyHeader study={study} />
+      <TrialsTable studyId={study.id} />
+    </>
+  )}
+</DetailPageShell>
+```
+
+**Behavior.** `isPending` → `<Card><CardContent><p>Loading…</p></CardContent></Card>`. `isError && error.errorCode === notFoundErrorCode` → `<EmptyState title="{Entity} not found" message="The {entity} may have been deleted." />`. `isError && error.errorCode !== notFoundErrorCode` (network / 5xx) → `<EmptyState title="Backend unreachable" message="Refresh after re-launching the API." />`. Existing `<EmptyState>` primitive consumed unchanged.
+
+**Out of scope.** The `chat/[id]` detail route is structurally different (stream-rendered conversation, not a card-based scaffold) and not migrated. The shared back-link header (`<Link>← All {entities}</Link>`) is also not absorbed by this primitive — left as a per-page concern; revisit if a 7th detail route adds the same shape (Q2's deferred follow-up).
+
+**Source-of-truth discipline (detail pages).** The vitest lint guard at [`ui/src/__tests__/components/common/detail-page-shell-discipline.test.tsx`](../../ui/src/__tests__/components/common/detail-page-shell-discipline.test.tsx) scans every `src/app/<entity>/[id]/page.tsx` (excluding `chat/[id]`) and fails when a file uses both `isPending ?` and `isError ?` ternaries without importing `<DetailPageShell>`. Escape hatch: a `// detail-page-shell-allow: <non-empty reason>` comment. Companion to the DataTable column-discipline and form-select-discipline guards; together they pin the three primitive extractions against regression-by-inlining.
 
 ## Auth surface (MVP1)
 

ui/src/__tests__/components/common/detail-page-shell-discipline.test.tsx

@@ -0,0 +1,209 @@
+/**
+ * Discipline lint guard for detail page scaffolding
+ * (chore_detail_page_shell_primitive Q3 — regex-based per recommended default).
+ *
+ * Sibling to `data-table-column-discipline.test.tsx` and
+ * `form-select-discipline.test.tsx`. Those scan column configs / form
+ * components for source-of-truth violations; this one scans the six
+ * `/{entity}/[id]/page.tsx` detail routes for hand-rolled
+ * `isPending → isError → data` scaffolds that should use the shared
+ * `<DetailPageShell>` primitive instead.
+ *
+ * Failure mode it catches:
+ *   {query.isPending ? (
+ *     <Card><CardContent>Loading…</CardContent></Card>
+ *   ) : query.isError ? (
+ *     <EmptyState title="X not found" message="..." />
+ *   ) : query.data ? (
+ *     <>{actual content}</>
+ *   ) : null}
+ *
+ * Approved replacement:
+ *   <DetailPageShell query={query} entityLabel="..." notFoundErrorCode="..._NOT_FOUND">
+ *     {(data) => (
+ *       <>{actual content}</>
+ *     )}
+ *   </DetailPageShell>
+ *
+ * Escape hatch (per Q3 spec): inline comment with a non-empty reason.
+ *   // detail-page-shell-allow: chat surface uses stream-rendered conversation, not a card shell
+ *
+ * Scope:
+ *   - Scans `src/app/<entity>/[id]/page.tsx` files.
+ *   - Skips `chat/[id]/page.tsx` (intentionally different scaffold).
+ *   - Files that already import `DetailPageShell` are exempt from the
+ *     ternary check (any residual `isPending`/`isError` ternaries in
+ *     the file refer to subordinate queries — e.g. trials list inside
+ *     the migrated study-detail shell — which are correct as-is).
+ */
+
+import { readFileSync, readdirSync, statSync } from 'node:fs';
+import { basename, join } from 'node:path';
+
+import { describe, expect, it } from 'vitest';
+
+const APP_ROOT = join(process.cwd(), 'src', 'app');
+
+const DETAIL_SHELL_IMPORT_RE =
+  /import\s+(?:type\s+)?\{[^}]*\bDetailPageShell\b[^}]*\}\s+from\s+['"]@\/components\/common\/detail-page-shell['"]/;
+const IS_PENDING_TERNARY_RE = /\bisPending\s*\?/;
+const IS_ERROR_TERNARY_RE = /\bisError\s*\?/;
+const ESCAPE_HATCH_RE = /\/\/\s*detail-page-shell-allow:(.*)$/m;
+
+const EXCLUDED_FOLDERS = new Set(['chat']);
+
+interface ValidationError {
+  file: string;
+  message: string;
+}
+
+/**
+ * Walk `src/app/` and return every `[id]/page.tsx` file. Detail routes
+ * are identified by their parent folder name (`[id]`) rather than a
+ * glob — keeps the lint resilient to deeper nesting.
+ */
+function walkDetailPages(dir: string, ancestors: readonly string[] = []): string[] {
+  const out: string[] = [];
+  let entries: string[];
+  try {
+    entries = readdirSync(dir);
+  } catch {
+    return out;
+  }
+  for (const name of entries) {
+    const full = join(dir, name);
+    const stats = statSync(full);
+    if (stats.isDirectory()) {
+      if (name === '[id]') {
+        // The entity folder is the LAST element of `ancestors`. If it's in
+        // the excluded set, skip this whole `[id]` subtree.
+        const entityFolder = ancestors[ancestors.length - 1];
+        if (entityFolder && EXCLUDED_FOLDERS.has(entityFolder)) continue;
+        const pagePath = join(full, 'page.tsx');
+        try {
+          const ps = statSync(pagePath);
+          if (ps.isFile()) out.push(pagePath);
+        } catch {
+          /* no page.tsx in this [id] folder */
+        }
+      } else {
+        out.push(...walkDetailPages(full, [...ancestors, name]));
+      }
+    }
+  }
+  return out;
+}
+
+/**
+ * Returns null when the file is clean (uses DetailPageShell, or doesn't
+ * have the offending ternary pattern, or has the escape-hatch comment).
+ * Returns a string describing the violation otherwise.
+ */
+function checkDetailPage(path: string, content: string): string | null {
+  if (DETAIL_SHELL_IMPORT_RE.test(content)) return null;
+
+  const hasPendingTernary = IS_PENDING_TERNARY_RE.test(content);
+  const hasErrorTernary = IS_ERROR_TERNARY_RE.test(content);
+  if (!hasPendingTernary || !hasErrorTernary) return null;
+
+  const hatch = content.match(ESCAPE_HATCH_RE);
+  if (hatch) {
+    const reason = hatch[1]?.trim();
+    if (!reason) {
+      return (
+        'detail-page-shell-allow escape-hatch comment must include a non-empty reason ' +
+        '(e.g. `// detail-page-shell-allow: chat surface uses stream-rendered conversation`)'
+      );
+    }
+    return null;
+  }
+
+  return (
+    `hand-rolled isPending/isError ternary pattern detected without importing ` +
+    `<DetailPageShell> from '@/components/common/detail-page-shell'. ` +
+    `Use the shared primitive instead, or add an escape-hatch comment ` +
+    `\`// detail-page-shell-allow: <reason>\` if this page is intentionally different.`
+  );
+}
+
+describe('detail-page-shell discipline lint guard', () => {
+  it('every /<entity>/[id]/page.tsx uses <DetailPageShell> (or has an escape hatch)', () => {
+    const files = walkDetailPages(APP_ROOT);
+    expect(files.length).toBeGreaterThan(0); // sanity — make sure we're actually finding pages
+
+    const errors: ValidationError[] = [];
+    for (const file of files) {
+      const content = readFileSync(file, 'utf-8');
+      const violation = checkDetailPage(file, content);
+      if (violation) errors.push({ file, message: violation });
+    }
+
+    if (errors.length > 0) {
+      const lines = errors.map((e) => `  ${e.file}\n    ${e.message}`);
+      throw new Error(
+        `${errors.length} detail page(s) failed the discipline check:\n${lines.join('\n')}`,
+      );
+    }
+  });
+
+  it('the chat detail route is intentionally excluded from scanning', () => {
+    const chatPage = join(APP_ROOT, 'chat', '[id]', 'page.tsx');
+    try {
+      statSync(chatPage);
+    } catch {
+      // If chat/[id]/page.tsx doesn't exist, the exclusion is moot — no test
+      // failure. The exclusion list is intent-driven, not file-existence-driven.
+      return;
+    }
+    const files = walkDetailPages(APP_ROOT);
+    expect(files.map((p) => basename(p))).toContain('page.tsx');
+    expect(files).not.toContain(chatPage);
+  });
+
+  it('detects a hand-rolled ternary in a synthetic page', () => {
+    const synthetic = `'use client';
+import { useFoo } from '@/lib/api/foo';
+export default function FooDetailPage() {
+  const query = useFoo();
+  return query.isPending ? <div>Loading…</div> : query.isError ? <div>Not found</div> : <div>{query.data?.name}</div>;
+}
+`;
+    expect(checkDetailPage('synthetic.tsx', synthetic)).toMatch(/hand-rolled isPending\/isError/);
+  });
+
+  it('accepts a page that imports DetailPageShell', () => {
+    const synthetic = `'use client';
+import { DetailPageShell } from '@/components/common/detail-page-shell';
+import { useFoo } from '@/lib/api/foo';
+export default function FooDetailPage() {
+  const query = useFoo();
+  return <DetailPageShell query={query} entityLabel="foo" notFoundErrorCode="FOO_NOT_FOUND">{(foo) => <div>{foo.name}</div>}</DetailPageShell>;
+}
+`;
+    expect(checkDetailPage('synthetic.tsx', synthetic)).toBeNull();
+  });
+
+  it('accepts a page with a valid escape-hatch comment', () => {
+    const synthetic = `'use client';
+import { useFoo } from '@/lib/api/foo';
+// detail-page-shell-allow: chat surface uses stream-rendered conversation, not a card shell
+export default function FooDetailPage() {
+  const query = useFoo();
+  return query.isPending ? <div>Loading…</div> : query.isError ? <div>x</div> : <div>{query.data?.name}</div>;
+}
+`;
+    expect(checkDetailPage('synthetic.tsx', synthetic)).toBeNull();
+  });
+
+  it('rejects an escape-hatch comment with empty reason', () => {
+    const synthetic = `'use client';
+import { useFoo } from '@/lib/api/foo';
+// detail-page-shell-allow:
+export default function FooDetailPage() {
+  const query = useFoo();
+  return query.isPending ? <div>Loading…</div> : query.isError ? <div>x</div> : <div>{query.data?.name}</div>;
+}
+`;
+    expect(checkDetailPage('synthetic.tsx', synthetic)).toMatch(/non-empty reason/);
+  });
+});