From ca34f276952a54a5d58964269e4604654bcbb7a1 Mon Sep 17 00:00:00 2001 From: Harry Randall Date: Sun, 20 Sep 2026 13:59:28 +1000 Subject: [PATCH 01/25] fix: make catalogue admin actions and tabs take effect Four defects that all present as the interface ignoring the reader. revalidatePath received the record page's own URL, which carries the academic year as a query string, but it matches a route path. Every revalidation missed, so accepting a change or publishing a draft only appeared after a manual reload. The path is now trimmed at the query. The record page is a server component whose tab lives in the query string, yet it mounted an uncontrolled Tabs. The editor's redirect to the preview after a save re-rendered the same instance and the tab never moved. A small client wrapper controls the value from the URL, which also keeps a reloaded or shared link on the expected tab. The directory passed a controlled state to FilterBar. That branch calls onQueryChange immediately, skipping the 250ms debounce the URL-bound branch uses, so every keystroke pushed a server navigation into a dynamic page and the pending state was lost. The directory now lets FilterBar bind to the URL, as the other list pages do. The remaining controlled callers filter client side, where immediate updates are correct, so the shared component is unchanged. The ten admin error boundaries removed with the old interface are restored. CatalogueError survived the rebuild but nothing imported it, so any failure under /admin/ took down the whole page. The open-review indicator is a counted badge rather than a bare amber dot, and the record container drops a mx-auto that had no width to centre within. --- apps/web/app/admin/courses/error.tsx | 2 ++ apps/web/app/admin/courses/imports/error.tsx | 2 ++ apps/web/app/admin/majors/error.tsx | 2 ++ apps/web/app/admin/majors/imports/error.tsx | 2 ++ apps/web/app/admin/minors/error.tsx | 2 ++ apps/web/app/admin/minors/imports/error.tsx | 2 ++ apps/web/app/admin/programmes/error.tsx | 2 ++ .../app/admin/programmes/imports/error.tsx | 2 ++ apps/web/app/admin/specialisations/error.tsx | 2 ++ .../admin/specialisations/imports/error.tsx | 2 ++ .../lib/coursemap/admin-catalogue-actions.ts | 26 ++++++++++----- .../admin/catalogue/catalogue-directory.tsx | 6 ---- apps/web/ui/admin/catalogue/record-page.tsx | 32 +++++++++++------- apps/web/ui/admin/catalogue/record-tabs.tsx | 33 +++++++++++++++++++ 14 files changed, 92 insertions(+), 25 deletions(-) create mode 100644 apps/web/app/admin/courses/error.tsx create mode 100644 apps/web/app/admin/courses/imports/error.tsx create mode 100644 apps/web/app/admin/majors/error.tsx create mode 100644 apps/web/app/admin/majors/imports/error.tsx create mode 100644 apps/web/app/admin/minors/error.tsx create mode 100644 apps/web/app/admin/minors/imports/error.tsx create mode 100644 apps/web/app/admin/programmes/error.tsx create mode 100644 apps/web/app/admin/programmes/imports/error.tsx create mode 100644 apps/web/app/admin/specialisations/error.tsx create mode 100644 apps/web/app/admin/specialisations/imports/error.tsx create mode 100644 apps/web/ui/admin/catalogue/record-tabs.tsx diff --git a/apps/web/app/admin/courses/error.tsx b/apps/web/app/admin/courses/error.tsx new file mode 100644 index 00000000..22a72703 --- /dev/null +++ b/apps/web/app/admin/courses/error.tsx @@ -0,0 +1,2 @@ +"use client"; +export { CatalogueError as default } from "@/ui/admin/catalogue-table/catalogue-error"; diff --git a/apps/web/app/admin/courses/imports/error.tsx b/apps/web/app/admin/courses/imports/error.tsx new file mode 100644 index 00000000..22a72703 --- /dev/null +++ b/apps/web/app/admin/courses/imports/error.tsx @@ -0,0 +1,2 @@ +"use client"; +export { CatalogueError as default } from "@/ui/admin/catalogue-table/catalogue-error"; diff --git a/apps/web/app/admin/majors/error.tsx b/apps/web/app/admin/majors/error.tsx new file mode 100644 index 00000000..22a72703 --- /dev/null +++ b/apps/web/app/admin/majors/error.tsx @@ -0,0 +1,2 @@ +"use client"; +export { CatalogueError as default } from "@/ui/admin/catalogue-table/catalogue-error"; diff --git a/apps/web/app/admin/majors/imports/error.tsx b/apps/web/app/admin/majors/imports/error.tsx new file mode 100644 index 00000000..22a72703 --- /dev/null +++ b/apps/web/app/admin/majors/imports/error.tsx @@ -0,0 +1,2 @@ +"use client"; +export { CatalogueError as default } from "@/ui/admin/catalogue-table/catalogue-error"; diff --git a/apps/web/app/admin/minors/error.tsx b/apps/web/app/admin/minors/error.tsx new file mode 100644 index 00000000..22a72703 --- /dev/null +++ b/apps/web/app/admin/minors/error.tsx @@ -0,0 +1,2 @@ +"use client"; +export { CatalogueError as default } from "@/ui/admin/catalogue-table/catalogue-error"; diff --git a/apps/web/app/admin/minors/imports/error.tsx b/apps/web/app/admin/minors/imports/error.tsx new file mode 100644 index 00000000..22a72703 --- /dev/null +++ b/apps/web/app/admin/minors/imports/error.tsx @@ -0,0 +1,2 @@ +"use client"; +export { CatalogueError as default } from "@/ui/admin/catalogue-table/catalogue-error"; diff --git a/apps/web/app/admin/programmes/error.tsx b/apps/web/app/admin/programmes/error.tsx new file mode 100644 index 00000000..22a72703 --- /dev/null +++ b/apps/web/app/admin/programmes/error.tsx @@ -0,0 +1,2 @@ +"use client"; +export { CatalogueError as default } from "@/ui/admin/catalogue-table/catalogue-error"; diff --git a/apps/web/app/admin/programmes/imports/error.tsx b/apps/web/app/admin/programmes/imports/error.tsx new file mode 100644 index 00000000..22a72703 --- /dev/null +++ b/apps/web/app/admin/programmes/imports/error.tsx @@ -0,0 +1,2 @@ +"use client"; +export { CatalogueError as default } from "@/ui/admin/catalogue-table/catalogue-error"; diff --git a/apps/web/app/admin/specialisations/error.tsx b/apps/web/app/admin/specialisations/error.tsx new file mode 100644 index 00000000..22a72703 --- /dev/null +++ b/apps/web/app/admin/specialisations/error.tsx @@ -0,0 +1,2 @@ +"use client"; +export { CatalogueError as default } from "@/ui/admin/catalogue-table/catalogue-error"; diff --git a/apps/web/app/admin/specialisations/imports/error.tsx b/apps/web/app/admin/specialisations/imports/error.tsx new file mode 100644 index 00000000..22a72703 --- /dev/null +++ b/apps/web/app/admin/specialisations/imports/error.tsx @@ -0,0 +1,2 @@ +"use client"; +export { CatalogueError as default } from "@/ui/admin/catalogue-table/catalogue-error"; diff --git a/apps/web/lib/coursemap/admin-catalogue-actions.ts b/apps/web/lib/coursemap/admin-catalogue-actions.ts index 5f9fb699..f3857aa5 100644 --- a/apps/web/lib/coursemap/admin-catalogue-actions.ts +++ b/apps/web/lib/coursemap/admin-catalogue-actions.ts @@ -28,6 +28,16 @@ function failure(error: unknown, fallback: string): ActionResult { }; } +/** + * The record page identifies itself with a URL carrying the academic year, but + * revalidatePath matches a route path. Passing the query string made every + * revalidation silently miss, so an accepted change only appeared after a + * manual reload. + */ +function revalidateRecord(path: string) { + revalidatePath(path.split("?")[0] ?? path); +} + export async function resolveReviewEntryAction({ entryId, status, @@ -48,7 +58,7 @@ export async function resolveReviewEntryAction({ p_note: note ?? undefined, }); if (error) return { ok: false, error: error.message }; - revalidatePath(path); + revalidateRecord(path); return { ok: true }; } @@ -81,7 +91,7 @@ export async function resolveAllChangesAction({ ); if (resolveError) return { ok: false, error: resolveError.message }; } - revalidatePath(path); + revalidateRecord(path); return { ok: true, message: `${entries?.length ?? 0} change${entries?.length === 1 ? "" : "s"} ${status}.`, @@ -101,7 +111,7 @@ export async function applyReviewAction({ if (!viewer) return { ok: false, error: "Authentication is required." }; try { const result = await applyImportReview({ targetId, userId: viewer.id }); - revalidatePath(path); + revalidateRecord(path); return { ok: true, message: result.reusedCandidate @@ -129,7 +139,7 @@ export async function publishDraftAction({ p_item_year_id: itemYearId, }); if (error) return { ok: false, error: error.message }; - revalidatePath(path); + revalidateRecord(path); return { ok: true, message: "Published. Students now see this version." }; } @@ -147,7 +157,7 @@ export async function unpublishAction({ p_item_year_id: itemYearId, }); if (error) return { ok: false, error: error.message }; - revalidatePath(path); + revalidateRecord(path); return { ok: true, message: "Unpublished. Students no longer see this record for the year.", @@ -176,7 +186,7 @@ export async function saveManualSnapshotAction({ write, userId: viewer.id, }); - revalidatePath(path); + revalidateRecord(path); return { ok: true, snapshotId: result.snapshotId, @@ -210,7 +220,7 @@ export async function restoreSnapshotAction({ snapshotId, userId: viewer.id, }); - revalidatePath(path); + revalidateRecord(path); return { ok: true, message: result.unchanged @@ -238,7 +248,7 @@ export async function discardDraftAction({ p_item_year_id: itemYearId, }); if (error) return { ok: false, error: error.message }; - revalidatePath(path); + revalidateRecord(path); return { ok: true, message: "Draft discarded. The snapshot stays in history.", diff --git a/apps/web/ui/admin/catalogue/catalogue-directory.tsx b/apps/web/ui/admin/catalogue/catalogue-directory.tsx index 447ad4ff..6075fd09 100644 --- a/apps/web/ui/admin/catalogue/catalogue-directory.tsx +++ b/apps/web/ui/admin/catalogue/catalogue-directory.tsx @@ -286,12 +286,6 @@ export function CatalogueDirectory({ })), }, ]} - state={{ - query, - values: { status: filter === "all" ? "" : filter }, - onQueryChange: (value) => navigate({ q: value }), - onFilterChange: (key, value) => navigate({ [key]: value }), - }} /> {selected.length > 0 ? ( diff --git a/apps/web/ui/admin/catalogue/record-page.tsx b/apps/web/ui/admin/catalogue/record-page.tsx index 14af505b..bd851f0b 100644 --- a/apps/web/ui/admin/catalogue/record-page.tsx +++ b/apps/web/ui/admin/catalogue/record-page.tsx @@ -1,9 +1,10 @@ import { - Tabs, TabsContent, TabsList, TabsTrigger, } from "@coursemap/ui/primitives/tabs"; +import { Badge } from "@coursemap/ui/components/badge"; +import { badgeVariantForTone } from "@/lib/ui"; import Link from "next/link"; import { canManageCourseImports } from "@/lib/auth/viewer"; import { @@ -20,6 +21,7 @@ import { AccessDeniedError } from "@/ui/errors/access-denied-error"; import { AppShell } from "@/ui/shell"; import { CatalogueEmpty } from "@/ui/admin/catalogue-table/catalogue-empty"; import { RecordHeader } from "./record-header"; +import { RecordTabs } from "./record-tabs"; import { RecordHistory } from "./record-history"; import { ReviewPanel } from "./review-panel"; import { SnapshotEditor } from "./snapshot-editor"; @@ -68,6 +70,14 @@ export async function CatalogueRecordPage({ ]) : [null, null]; + const openReviewCount = + record?.reviews.reduce( + (total, review) => + total + + review.entries.filter((entry) => entry.status === "open").length, + 0, + ) ?? 0; + return ( ) : ( -
+
- + Review - {record.reviews.some((review) => - review.entries.some((entry) => entry.status === "open"), - ) ? ( - + {openReviewCount > 0 ? ( + + {openReviewCount} + ) : null} @@ -160,7 +170,7 @@ export async function CatalogueRecordPage({ - +
)} diff --git a/apps/web/ui/admin/catalogue/record-tabs.tsx b/apps/web/ui/admin/catalogue/record-tabs.tsx new file mode 100644 index 00000000..c6bb0948 --- /dev/null +++ b/apps/web/ui/admin/catalogue/record-tabs.tsx @@ -0,0 +1,33 @@ +"use client"; + +import type { ReactNode } from "react"; +import { useRouter } from "next/navigation"; +import { Tabs } from "@coursemap/ui/primitives/tabs"; + +/** + * The record page is a server component, so its tab state lives in the URL. + * An uncontrolled `Tabs` kept its own state instead, which meant the editor's + * redirect to the preview after a save re-rendered the same instance and the + * tab never moved. Controlling the value from the query string also keeps a + * reloaded or shared link on the tab the reader expects. + */ +export function RecordTabs({ + value, + path, + children, +}: { + value: string; + path: string; + children: ReactNode; +}) { + const router = useRouter(); + return ( + router.replace(`${path}&tab=${next}`)} + className="block" + > + {children} + + ); +} From 97baa362832a64fac39eebeafe773ca6047917ce Mon Sep 17 00:00:00 2001 From: Harry Randall Date: Sun, 20 Sep 2026 14:04:06 +1000 Subject: [PATCH 02/25] refactor: show import runs, records and stages as tables The import page held three hand-rolled lists. Runs were a column of buttons 20rem wide whose run number, status, progress and timestamp sat in unaligned stacked spans, so nothing lined up across rows. Records were a wrapping flex row where a long error message pushed the Review button onto the next line. Stages were bordered list items printing a raw millisecond count. All three are now DataTableShell tables. The six-column data-imports grid already existed in catalogue-table.module.css, written for this page and unused since the rebuild; the record table adopts it. Runs and stages get their own grids alongside it. The run list moves out of the narrow master column and across the page, so model, cost and progress are readable. Durations render as seconds past a thousand milliseconds. Run status shows a label rather than the raw enum. The target detail placeholder is a skeleton matching the card it replaces, instead of a bare sentence that shifted the layout when the detail arrived. --- .../catalogue-table.module.css | 16 + .../admin/catalogue-table/catalogue-table.tsx | 2 +- apps/web/ui/admin/catalogue/import-runs.tsx | 452 ++++++++++++------ 3 files changed, 312 insertions(+), 158 deletions(-) diff --git a/apps/web/ui/admin/catalogue-table/catalogue-table.module.css b/apps/web/ui/admin/catalogue-table/catalogue-table.module.css index 142d7721..8ec8dfec 100644 --- a/apps/web/ui/admin/catalogue-table/catalogue-table.module.css +++ b/apps/web/ui/admin/catalogue-table/catalogue-table.module.css @@ -186,3 +186,19 @@ a.title:focus-visible { min-height: 44px; } } + +/* Import runs: run, year, status, progress, model, cost, started. */ +.shell[data-layout="runs"] tr { + min-width: 860px; + grid-template-columns: + 90px 70px 116px 132px + minmax(150px, 1fr) 88px 168px; +} + +/* Import stages: step, stage, status, duration, artefacts. */ +.shell[data-layout="stages"] tr { + min-width: 760px; + grid-template-columns: + 56px minmax(180px, 1fr) 116px 92px + minmax(200px, 1.2fr); +} diff --git a/apps/web/ui/admin/catalogue-table/catalogue-table.tsx b/apps/web/ui/admin/catalogue-table/catalogue-table.tsx index 6d99022f..034332df 100644 --- a/apps/web/ui/admin/catalogue-table/catalogue-table.tsx +++ b/apps/web/ui/admin/catalogue-table/catalogue-table.tsx @@ -38,7 +38,7 @@ export function DataTableShell({ footer?: ReactNode; selectable?: boolean; imports?: boolean; - layout?: "public-courses" | "users" | "directory"; + layout?: "public-courses" | "users" | "directory" | "runs" | "stages"; }) { return (
= { @@ -38,6 +49,21 @@ const RUN_TONE: Record = { cancelled: "neutral", }; +const RUN_STATUS_LABELS: Record = { + queued: "Queued", + running: "Running", + completed: "Completed", + failed: "Failed", + cancelled: "Cancelled", +}; + +const STAGE_TONE: Record = { + completed: "success", + failed: "danger", + running: "info", + queued: "neutral", +}; + const STAGE_LABELS: Record = { source_fetch: "Fetch page", html_capture: "Capture HTML", @@ -189,23 +215,48 @@ export function ImportRuns({ } return ( -
-
@@ -345,6 +443,33 @@ export function ImportRuns({ ); } +/** Milliseconds are unreadable past a second, which most stages are. */ +function formatDuration(milliseconds: number | null) { + if (milliseconds === null) return "\u2014"; + if (milliseconds < 1000) return `${milliseconds} ms`; + return `${(milliseconds / 1000).toFixed(1)} s`; +} + +function ArtifactLink({ + artifact, +}: { + artifact: ImportTargetDetail["artifacts"][number]; +}) { + return ( + + {ARTIFACT_LABELS[artifact.kind] ?? artifact.kind} + + {formatBytes(artifact.byteSize)} + + + ); +} + function TargetDetail({ detail }: { detail: ImportTargetDetail }) { const latestAttempt = Math.max( 1, @@ -380,73 +505,86 @@ function TargetDetail({ detail }: { detail: ImportTargetDetail }) { -
    - {stages.map((stage) => { - const duration = durationMs(stage.startedAt, stage.completedAt); - const artifacts = artifactsByStage.get(stage.id) ?? []; - return ( -
  1. -
    - {stage.status === "completed" ? ( -
    - {stage.errorSummary ? ( -

    - {stage.errorCode ? `${stage.errorCode}: ` : ""} - {stage.errorSummary} -

    - ) : null} - {artifacts.length ? ( - - ) : null} -
    2. - ); - })} -
+ + + + Pipeline stages for {detail.code}, attempt {latestAttempt}. + + + + Step + Stage + Status + Duration + Artefacts + + + + {stages.map((stage, index) => { + const duration = durationMs(stage.startedAt, stage.completedAt); + const artifacts = artifactsByStage.get(stage.id) ?? []; + return ( + + + {index + 1} + + + + {STAGE_LABELS[stage.name] ?? stage.name} + + {stage.errorSummary ? ( +

+ {stage.errorCode ? `${stage.errorCode}: ` : ""} + {stage.errorSummary} +

+ ) : null} + + + + {stage.status === "completed" ? ( + + + + {formatDuration(duration)} + + + {artifacts.length ? ( +
    + {artifacts.map((artifact) => ( +
  • + +
    • + ))} +
+ ) : ( + + None + + )} + + + ); + })} + +
+ ); From 188c231d23576eb6ecdc0d80ea25541055061134 Mon Sep 17 00:00:00 2001 From: Harry Randall Date: Sun, 20 Sep 2026 14:05:41 +1000 Subject: [PATCH 03/25] docs: record the component reuse list and the work left after A7 The rebuild in A5 to A7 was specified in data-flow terms and reused none of the existing component kit, so the plan now names the kit every remaining project draws from and the review rule that goes with it. The new document carries the interface, pipeline, schema and test work still outstanding, with the findings behind each item so they are not rediscovered: the review panel cannot show a per-field diff because changes.ts diffs whole collections, a manual edit clears the publication gate, summary fields are parsed and never stored, discarding a model extraction is silent, and a failed target blocks its own retry. --- docs/README.md | 1 + docs/catalogue-admin-rework.md | 195 +++++++++++++++++++++++++++++++++ docs/redesign-plan.md | 12 ++ 3 files changed, 208 insertions(+) create mode 100644 docs/catalogue-admin-rework.md diff --git a/docs/README.md b/docs/README.md index cb08c3f0..39a1b5ef 100644 --- a/docs/README.md +++ b/docs/README.md @@ -11,6 +11,7 @@ ## Proposals +- [Catalogue admin rework](catalogue-admin-rework.md): the component reuse list and the interface, pipeline and test work left after A7. - [Redesign plan](redesign-plan.md): decisions, target model and stacked pull request sequence for the catalogue schema and import redesign, prerequisites, student interface and campus map. Keep current operating instructions in the guides and label proposals explicitly. diff --git a/docs/catalogue-admin-rework.md b/docs/catalogue-admin-rework.md new file mode 100644 index 00000000..507aef52 --- /dev/null +++ b/docs/catalogue-admin-rework.md @@ -0,0 +1,195 @@ +# Catalogue admin rework + +Status: in progress. This document carries the work left on the catalogue +administration interface and the import pipeline after A7, and the findings +behind it. Fold each item into [architecture](architecture.md) or +[catalogue operations](catalogue-operations.md) as it lands, and delete this +document when the list is empty. + +## Why this exists + +A5 to A7 specified data flow and routes and said nothing about reusing the +existing component kit. The rebuild was executed literally: around 9,300 lines +of admin interface across 69 files were removed and replaced with roughly 3,100 +lines that hand-roll tables, selects, labels, diffs and loading states the +repository already owns. Four finished components were deleted for schema +reasons and never replaced. The B and C projects will repeat this unless the +reuse list below is part of their scope. + +## Reuse list + +Read [UI conventions](../apps/web/ui/AGENTS.md) before touching any of this. +Nothing in `apps/web/ui/admin/` or `apps/web/ui/common/` is a second primitive +library, and neither is a reason to write a third. + +| Need | Use | +| -------------------------- | --------------------------------------------------------------------- | +| Any tabular list | `ui/admin/catalogue-table/` `DataTableShell` and the Table parts | +| A row identity | `CatalogueIdentity` | +| A whole-row link | `ui/common/linked-table-row.tsx` | +| Search and filtering | `ui/common/filter-bar.tsx` above the table, never inside it | +| Sorting, paging | `ui/common/sort-menu.tsx`, `ui/common/pagination.tsx` | +| A choice or boolean field | `ui/common/option-picker.tsx`, `ui/common/select-field.tsx` | +| A labelled field | `@coursemap/ui/primitives/field` | +| Long-form or section forms | `ui/common/section-navigation.tsx` | +| A destructive bulk action | `ui/common/confirm-dialog.tsx` | +| Empty, loading, error | `ui/admin/catalogue-table/catalogue-{empty,loading,error}.tsx` | +| A status label | `ui/common/status-pill.tsx`, or a tone map plus `badgeVariantForTone` | +| JSON or source display | `ui/common/json-code.tsx` | + +Rules that the rebuild broke and that reviews should enforce: no `
    ` or +`
  • ` standing in for a table, no hand-rolled ``, evidence fields editable, rows titled "Item N", and a +boolean detector that cannot match because it tests a lowercase pattern against +an already capitalised label. Structure requirement trees render as raw JSON +until the editor covers `structure_set`, `tagged_units` and `elective_units`. + +### 4. Remaining list and table work + +- `record-history.tsx` is two bare `
      ` lists exposing raw bigint identifiers. + Snapshot history is tabular: snapshot, created, origin, run, role, actions. +- `catalogue-directory.tsx` should use `LinkedTableRow` so the whole row is a + target, and gained no actions column although the grid reserves one. +- `catalogue-loading.tsx` has no `directory` layout even though `DataTableShell` + supports one, so the directory skeleton is a six-column table in front of a + five-column one. `app/admin//imports/loading.tsx` draws a table skeleton + in front of a card layout. +- `catalogue-pages.tsx` defines a local `DirectorySkeleton` that duplicates + `CatalogueLoading`; the reader sees both in sequence. +- `import-runs.tsx` has no search, filter, sort or paging and + `loadCatalogueImportRuns` caps at twenty-five with no offset. It polls with + `router.refresh()` every four seconds, refetching every run and target; + `readImportStream` already exists for this. +- `record-header.tsx` shows publish blockers as the faintest text on the page + and repeats them in a `title` attribute. They belong in an `Alert`. +- Record pages under `/admin//[code]` still have no error boundary. + `CatalogueError`'s copy is list-specific, so this needs a record variant + rather than the same component. + +### 5. Pipeline and schema + +- A manual edit bypasses the publication gate. `catalogue_publish_blockers` + finds blockers through the draft's `import_target_id`, and + `manual-snapshot.ts` never sets one, so any trivial edit clears the gate. + Carry the originating target through, or resolve blockers through the item + year's latest target. +- `summaryFields` are parsed, hashed into the projection and never persisted. + The table was dropped in `20260918150000_requirements.sql` and nothing + replaced it. Every programme import loses them. +- Discarding a model extraction is silent: the target still lands `ready` with + a null `error_code`. Emit a blocking flag and an error code. + `kinds/structure/model-response-error.ts` explains truncation and is imported + by nothing. +- `kinds/structure/adapter.ts` accepts or rejects the whole model response. + Make it field-level as `kinds/course/merge.ts` already is, so a bad fee does + not cost the requirement tree. +- `persist-snapshot.ts` drops unresolvable options, references and related + courses with a bare `continue`. Record them. +- A failed target stays `queued` and the active-target index then blocks every + retry, with no interface path out. `cancel_catalogue_import` needs an + administrator claim, so recovery is currently a manual SQL statement. +- `catalogue_items` cannot be deleted: the immutability trigger fires on delete + although its comment says cascades pass. Combined with placeholder creation + from any regex-valid code, one hallucinated code is permanent. +- `requirement_item_references` stays empty for structures. +- Consider `json_schema` structured output in `openrouter.ts`. Deliberately not + done: the schema uses `$ref` and `$defs`, the configured model is + `google/gemini-3.1-flash-lite`, and `provider.require_parameters` would turn a + soft failure into a hard one. Revisit with a model that supports it. + +### 6. Tests the plan promised and A5 to A7 never added + +No pgTAP covers `catalogue_import_targets`, `catalogue_import_changes`, +`catalogue_directory_entries`, `catalogue_directory_statuses`, +`catalogue_import_stages`, `catalogue_import_artifacts`, `catalogue_extractions`, +`start_catalogue_import`, `cancel_catalogue_import`, +`resolve_catalogue_import_change`, `catalogue_publish_blockers`, +`publish_catalogue_snapshot`, `unpublish_catalogue_item_year`, +`discard_catalogue_draft` or `admin_snapshot_projection`. The plan asks +specifically for a sealed snapshot, a stale-baseline Apply and a blocked +Publish. + +## Working locally + +- Use `pnpm dev:local`. `apps/web/.env.local` points at the hosted project, + which still carries the pre-redesign schema, so `pnpm dev` fails with + `PGRST205` against tables this branch removed. None of the fixes above are + live there; they travel with the cutover after A8. +- `pnpm db:reset` drops the storage buckets and does not recreate them, so + imports then fail with an opaque gateway error. Recreate + `course-import-artifacts` from `supabase/config.toml`. +- `package.json` still exposes `course-import:benchmark`, whose script was + deleted. + +## Before A8 + +A8 squashes the migration history into a baseline, which states that the +schema is the intended one. Items 5 and 6 above land first, along with the +renames the plan deferred: the surviving `academic_structure_*` child tables, +`published_catalogue_item` (today `published_course_detail`, with no structure +equivalent at all), and `published_requirement_graph`, which is still keyed on +a course code. diff --git a/docs/redesign-plan.md b/docs/redesign-plan.md index 57c37bcf..b98cfccb 100644 --- a/docs/redesign-plan.md +++ b/docs/redesign-plan.md @@ -333,6 +333,18 @@ student-facing surface. Administrators cannot import or edit catalogue content locally between A3a and A7. If A5 exceeds a reviewable size, split along the course and structure kind boundary rather than across layers. +### Interface reuse + +A5 to A7 specified data flow and routes and said nothing about reusing the +existing component kit, so the rebuild hand-rolled tables, selects, labels, +diffs and loading states the repository already owns and deleted four finished +components that had no schema problem. Every remaining project in this plan +reuses the kit listed in +[catalogue admin rework](catalogue-admin-rework.md#reuse-list), and a pull +request that introduces another bespoke table, form control or status label is +not ready. That document also carries the interface, pipeline and test work +still outstanding from Project A. + ### Verification Per pull request: `pnpm verify`, `pnpm db:reset`, `pnpm db:test`, `pnpm db:lint`, From 76264ae531f5e597c7b16dd6319399fe03bbce0a Mon Sep 17 00:00:00 2001 From: Harry Randall Date: Sun, 20 Sep 2026 14:13:02 +1000 Subject: [PATCH 04/25] refactor: diff a catalogue change field by field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit changes.ts records a collection as one row, so altering a single session's delivery mode produced an entry whose old and new values were the whole session array. ValueDiff rendered both sides as JSON and left the reviewer to find the difference by eye, then offered Accept or Reject for the collection entire. That is the reason reviewing a record did not read as a review. The diff now walks into the change and lists only the fields that differ, naming each one, so the example above reads "Item 1 · Delivery mode: In person / Online". Added and removed rows are labelled as such. Values render through CatalogueValue, recovered from the interface removed in 80b95de, which shows a record as a definition list rather than braces. A change too wide to reduce still falls back to both raw values, so nothing is ever hidden. Component tests cover a scalar, a single altered field inside a collection, an added row and the fallback. Flag codes and entry statuses are named for the reader instead of printing the database vocabulary, and Reject all, which discards every imported value at once, now asks first. ConfirmDialog was already used twice in neighbouring files. --- apps/web/lib/coursemap/catalogue-kinds.ts | 16 ++ apps/web/next-env.d.ts | 4 +- apps/web/tests/value-diff.test.tsx | 69 ++++++ .../ui/admin/catalogue/catalogue-value.tsx | 44 ++++ apps/web/ui/admin/catalogue/review-panel.tsx | 59 ++++- apps/web/ui/admin/catalogue/value-diff.tsx | 208 ++++++++++++++---- 6 files changed, 349 insertions(+), 51 deletions(-) create mode 100644 apps/web/tests/value-diff.test.tsx create mode 100644 apps/web/ui/admin/catalogue/catalogue-value.tsx diff --git a/apps/web/lib/coursemap/catalogue-kinds.ts b/apps/web/lib/coursemap/catalogue-kinds.ts index d5ba2dd2..b5b4a3f7 100644 --- a/apps/web/lib/coursemap/catalogue-kinds.ts +++ b/apps/web/lib/coursemap/catalogue-kinds.ts @@ -208,3 +208,19 @@ export const FIELD_LABELS: Record = { export function fieldLabel(fieldPath: string) { return FIELD_LABELS[fieldPath] ?? fieldPath; } + +/** + * A readable name for a key the label map does not carry, such as a field + * inside a collection row. Prefer FIELD_LABELS; this is the fallback, not a + * substitute for naming a field properly. + */ +export function humaniseKey(key: string) { + const known = FIELD_LABELS[key]; + if (known) return known; + const spaced = key + .replace(/([a-z\d])([A-Z])/g, "$1 $2") + .replace(/[_-]+/g, " ") + .toLowerCase() + .trim(); + return spaced.charAt(0).toUpperCase() + spaced.slice(1); +} diff --git a/apps/web/next-env.d.ts b/apps/web/next-env.d.ts index a419cbe4..ce4e94a6 100644 --- a/apps/web/next-env.d.ts +++ b/apps/web/next-env.d.ts @@ -1,7 +1,7 @@ /// /// -import "./.next/dev/types/routes.d.ts"; -import "./.next/dev/types/root-params.d.ts"; +import "./.next/types/routes.d.ts"; +import "./.next/types/root-params.d.ts"; // NOTE: This file should not be edited // see https://nextjs.org/docs/app/api-reference/config/typescript for more information. diff --git a/apps/web/tests/value-diff.test.tsx b/apps/web/tests/value-diff.test.tsx new file mode 100644 index 00000000..c2c7d336 --- /dev/null +++ b/apps/web/tests/value-diff.test.tsx @@ -0,0 +1,69 @@ +import { expect, test } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { ValueDiff } from "@/ui/admin/catalogue/value-diff"; + +test("a scalar change reads as one pair of values", () => { + render( + , + ); + expect(screen.getByText("Algorithms")).toBeInTheDocument(); + expect(screen.getByText("Advanced Algorithms")).toBeInTheDocument(); +}); + +test("only the changed field of a collection row is shown", () => { + const before = [ + { classNumber: "1", deliveryMode: "In person", location: "Manning Clark" }, + { classNumber: "2", deliveryMode: "In person", location: "Hanna Neumann" }, + ]; + const after = [ + { classNumber: "1", deliveryMode: "Online", location: "Manning Clark" }, + { classNumber: "2", deliveryMode: "In person", location: "Hanna Neumann" }, + ]; + render( + , + ); + + // The one altered field is named, with its own before and after. + expect(screen.getByText("Item 1 · Delivery mode")).toBeInTheDocument(); + expect(screen.getByText("In person")).toBeInTheDocument(); + expect(screen.getByText("Online")).toBeInTheDocument(); + + // The untouched row never appears, and neither does a JSON dump of it. + expect(screen.queryByText(/Item 2/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Hanna Neumann/)).not.toBeInTheDocument(); +}); + +test("an added row is labelled as added", () => { + render( + , + ); + expect(screen.getByText("Item 2 (added)")).toBeInTheDocument(); + expect(screen.getByText("International")).toBeInTheDocument(); +}); + +test("a value the walk cannot reduce falls back to the raw values", () => { + const wide = Array.from({ length: 80 }, (_, index) => ({ + key: `value-${index}`, + })); + render( + , + ); + // Nothing was dropped: the reviewer still sees both sides in full. + expect(screen.getByLabelText("Current course.sessions")).toBeInTheDocument(); + expect(screen.getByLabelText("Imported course.sessions")).toBeInTheDocument(); +}); diff --git a/apps/web/ui/admin/catalogue/catalogue-value.tsx b/apps/web/ui/admin/catalogue/catalogue-value.tsx new file mode 100644 index 00000000..fbdb1894 --- /dev/null +++ b/apps/web/ui/admin/catalogue/catalogue-value.tsx @@ -0,0 +1,44 @@ +import { humaniseKey } from "@/lib/coursemap/catalogue-kinds"; + +/** + * A readable view of an extracted value. Reviewers compare content, not JSON, + * so objects become definition lists and arrays become stacks. Anything the + * pipeline cannot name still renders as text rather than being hidden. + */ +export function CatalogueValue({ value }: { value: unknown }) { + if (value === null || value === undefined || value === "") + return Not set; + if (typeof value === "boolean") return <>{value ? "Yes" : "No"}; + if (Array.isArray(value)) + return value.length ? ( +
        + {value.map((entry, index) => ( +
      • + +
        • + ))} +
      + ) : ( + None + ); + if (typeof value === "object") + return ( +
      + {Object.entries(value) + .filter(([, child]) => child !== null && child !== "") + .map(([key, child]) => ( +
      +
      + {humaniseKey(key)} +
      +
      + +
      +
      + ))} +
      + ); + return ( + {String(value)} + ); +} diff --git a/apps/web/ui/admin/catalogue/review-panel.tsx b/apps/web/ui/admin/catalogue/review-panel.tsx index 366ec22b..253dab2b 100644 --- a/apps/web/ui/admin/catalogue/review-panel.tsx +++ b/apps/web/ui/admin/catalogue/review-panel.tsx @@ -30,8 +30,9 @@ import type { ReviewEntry, ReviewTarget, } from "@/lib/coursemap/admin-catalogue-record"; -import { fieldLabel } from "@/lib/coursemap/catalogue-kinds"; +import { humaniseKey, fieldLabel } from "@/lib/coursemap/catalogue-kinds"; import { badgeVariantForTone } from "@/lib/ui"; +import { ConfirmDialog } from "@/ui/common/confirm-dialog"; import { ValueDiff } from "./value-diff"; function formatDateTime(value: string | null) { @@ -125,12 +126,12 @@ export function ReviewPanel({ > Accept all - + trigger={ + + } + /> ) : null} + } + /> + ) : snapshot.id !== record.publishedSnapshotId || + record.draftSnapshotId ? ( + + ) : null} +
) : null} - - {snapshot.id === record.publishedSnapshotId ? ( - Published - ) : null} - {snapshot.id === record.draftSnapshotId ? ( - Draft - ) : null} - {!snapshot.sealedAt ? ( - Unsealed - ) : null} - {snapshot.id === record.draftSnapshotId ? ( - - run(() => - discardDraftAction({ - itemYearId: record.itemYearId, - path, - }), - ) - } - trigger={ - - } - /> - ) : snapshot.id !== record.publishedSnapshotId || - record.draftSnapshotId ? ( - - ) : null} - - - ))} - - -
-

- Publications{" "} - - {record.publications.length} - -

- {record.publications.length === 0 ? ( -

Never published.

- ) : ( -
    - {record.publications.map((publication, index) => ( -
  1. - {formatDateTime(publication.publishedAt)} - - {publication.snapshotId - ? ` · snapshot #${publication.snapshotId}` - : " · unpublished"} - -
    2. - ))} -
- )} -
-
+ + + ); + })} + ); } From f2df671b2a61345c466b65719dcc99a74ae44089 Mon Sep 17 00:00:00 2001 From: Harry Randall Date: Sun, 20 Sep 2026 14:26:42 +1000 Subject: [PATCH 06/25] docs: correct the table guidance and record which shell to use The reuse list named one table shell where the repository has two, and the outstanding work claimed version history was tabular. Both were inferred from the data rather than read off the component being replaced, and both were wrong. The list now separates the catalogue grid, which sizes columns for a list of records, from the ordinary shell used for every other table, and names the timeline as the shape a version or activity history takes. A short note says to read the deleted component first and how to find it. --- docs/catalogue-admin-rework.md | 71 ++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 30 deletions(-) diff --git a/docs/catalogue-admin-rework.md b/docs/catalogue-admin-rework.md index 507aef52..4f9cbf72 100644 --- a/docs/catalogue-admin-rework.md +++ b/docs/catalogue-admin-rework.md @@ -22,20 +22,22 @@ Read [UI conventions](../apps/web/ui/AGENTS.md) before touching any of this. Nothing in `apps/web/ui/admin/` or `apps/web/ui/common/` is a second primitive library, and neither is a reason to write a third. -| Need | Use | -| -------------------------- | --------------------------------------------------------------------- | -| Any tabular list | `ui/admin/catalogue-table/` `DataTableShell` and the Table parts | -| A row identity | `CatalogueIdentity` | -| A whole-row link | `ui/common/linked-table-row.tsx` | -| Search and filtering | `ui/common/filter-bar.tsx` above the table, never inside it | -| Sorting, paging | `ui/common/sort-menu.tsx`, `ui/common/pagination.tsx` | -| A choice or boolean field | `ui/common/option-picker.tsx`, `ui/common/select-field.tsx` | -| A labelled field | `@coursemap/ui/primitives/field` | -| Long-form or section forms | `ui/common/section-navigation.tsx` | -| A destructive bulk action | `ui/common/confirm-dialog.tsx` | -| Empty, loading, error | `ui/admin/catalogue-table/catalogue-{empty,loading,error}.tsx` | -| A status label | `ui/common/status-pill.tsx`, or a tone map plus `badgeVariantForTone` | -| JSON or source display | `ui/common/json-code.tsx` | +| Need | Use | +| ------------------------------ | ------------------------------------------------------------------------------------------- | +| A list of catalogue records | `ui/admin/catalogue-table/` `DataTableShell`, whose CSS grid owns the column widths | +| Any other table | `ui/common/data-table.tsx` `DataTableShell` around a primitive `Table` with a minimum width | +| An activity or version history | A timeline, as `CourseImportHistory` and `CatalogueReviewHistory` were. Not a table | +| A row identity | `CatalogueIdentity` | +| A whole-row link | `ui/common/linked-table-row.tsx` | +| Search and filtering | `ui/common/filter-bar.tsx` above the table, never inside it | +| Sorting, paging | `ui/common/sort-menu.tsx`, `ui/common/pagination.tsx` | +| A choice or boolean field | `ui/common/option-picker.tsx`, `ui/common/select-field.tsx` | +| A labelled field | `@coursemap/ui/primitives/field` | +| Long-form or section forms | `ui/common/section-navigation.tsx` | +| A destructive bulk action | `ui/common/confirm-dialog.tsx` | +| Empty, loading, error | `ui/admin/catalogue-table/catalogue-{empty,loading,error}.tsx` | +| A status label | `ui/common/status-pill.tsx`, or a tone map plus `badgeVariantForTone` | +| JSON or source display | `ui/common/json-code.tsx` | Rules that the rebuild broke and that reviews should enforce: no `
    ` or `
  • ` standing in for a table, no hand-rolled `