feat(532): datasets admin UI - upload/schema/delete

[sneumannb5]

What

Admin page web-ui/app/admin/datasets/ for KG datasets (Closes #532) - deferred Phase-14 follow-up to #430. Upload CSV, browse schema + row preview, delete.

Why

#430 shipped the backend but deferred the admin UI its triage criteria require ("deletable via admin UI"). Pure web-ui - REST surface already exists + tested.

Test plan

• typecheck + lint + i18n:check green
• vitest run app/admin/datasets - 5/5
• manual click-through vs live middleware (list confirmed via screenshot)

Risk / blast radius

Low. Additive only - new page + appended api.ts fns + one index card + new i18n keys. No schema/API/CI/env changes.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[ConnysCode]

Review: Request changes

Four things to change before merge, then a batch of minor items inline. Everything the test plan claims is true — verified at head e5105af: tsc --noEmit clean, eslint clean on the changed files, i18n-validate OK (3304 keys, en/de parity, no adminDatasets warnings), vitest run app/admin/datasets 5/5.

Must change before merge

🟠 1. The list is capped at 50 — older datasets are invisible and undeletable through this UI. Both graph backends default the cap to 50 (middleware/packages/harness-knowledge-graph-neon/src/neonKnowledgeGraph.ts:706, .../harness-knowledge-graph-inmemory/src/inMemoryKnowledgeGraph.ts:2927 — Math.max(1, Math.min(opts.limit ?? 50, 200))), and GET /api/v1/datasets passes no limit and parses no query params at all (middleware/src/routes/datasets.ts:138; contrast GET /:id/rows, which has RowsQuerySchema at :27-30). The new page renders items.map(...) with no pagination, no total, and no "showing N of M" hint.

With 60 datasets the operator sees the 50 newest with no indication that 10 are missing. Those 10 have no row, therefore no Delete button, and openDetail is only reachable from a list row (web-ui/app/admin/datasets/page.tsx:341) — they cannot be viewed or deleted here at all. Sixty is not a stretch: the chat-attachment path creates one dataset per CSV attachment (middleware/packages/harness-orchestrator/src/orchestrator.ts:5547-5556). That is exactly the #430 triage criterion ("deletable via admin UI") this PR closes.

Note this is not fixable in web-ui alone — the route ignores query params, so even ?limit=200 would do nothing. Please add limit/offset to GET /api/v1/datasets (a z.coerce schema mirroring RowsQuerySchema), pass them through listDatasets, and paginate the list. If backend work is out of scope for this PR, please at minimum return the total and render an explicit "showing 50 of N — older datasets are not listed" warning so the operator isn't silently misled, and file the follow-up.

🟠 2. Rows from one dataset can render under another dataset's schema. Missing request guard in loadRowsPage/openDetail — details and repro inline at page.tsx:172.

🟠 3. A failed delete is reported as a failed load, the stale row stays listed, and the confirm is disarmed. Inline at page.tsx:194.

🟠 4. The page copy promises instance-wide scope on a page that only ever lists your own datasets. Inline at messages/en.json:3808. Copy-only fix; this is not a new gating hole and I say so in the comment.

PR description — factcheck

Claim	Reality
"typecheck + lint + i18n:check green"	✅ Reproduced at head: tsc --noEmit exit 0, eslint app/admin/datasets app/_lib/api.ts app/admin/page.tsx exit 0, i18n-validate: OK — 3304 keys, locales: en, de.
"vitest run app/admin/datasets — 5/5"	✅ Reproduced: 1 file, 5 tests, all passing.
"mirrors uploadPackage" (api.ts)	✅ Accurate — same FormData / no manual Content-Type / credentials: 'include' / cookie-forward shape as api.ts:642-650, minus the XHR progress path. It also adds maybeNavigateToLogin on failure, which handleUploadResponse doesn't.
"Additive only — new page + appended api.ts fns + one index card + new i18n keys"	⚠️ Omits the two docs files it also changes, and docs/middleware-agent-handoff.md:1646-1663 is a rewrite, not an append (present→past tense throughout, the "Folge-Issue ist offen zu erfassen" paragraph replaced). Making those edits is correct; the description just doesn't list them. Code-wise the claim holds — api.ts:4527-4621 is a pure append, admin/page.tsx a 2-line GROUPS insertion.
"No schema/API/CI/env changes"	✅ Accurate.
"Low" risk	✅ Accurate — everything new is reachable only from the new route.

Minimum to merge

1. Pagination (or an explicit truncation warning + total) on the dataset list — item 1 above.
2. Request guard in openDetail/loadRowsPage, and disable the row trigger while a detail load is in flight — page.tsx:172.
3. Separate delete-error state; reload() on the failure path too — page.tsx:194.
4. Say in the copy that the list is owner-scoped — messages/en.json:3808 and the index card at :214.
5. Two tests that would have caught #2: a totalMatched: 60 fixture exercising prev/next, and a truncatedCellCount > 0 upload — __tests__/page.test.tsx:74.

Nits (non-blocking)

• page.tsx:38-40 — the comment says "no per-row hover fill", but globals.css:778-780 applies background: var(--accent-subtle) to table … tbody tr:hover globally, including all three tables here. The comment describes what the utility classes omit, but reads as a claim about the rendered result.
• page.tsx:408-410 + messages/en.json:3837 — detail.heading is "{name}", a catalog key that can never be translated, rendered into the 10px uppercase tracking-wider eyebrow class. Q3 Sales EMEA comes out as Q 3 S A L E S E M E A in muted grey, and the panel gains no label saying what it is. Consider "Dataset · {name}" with normal case for the name.
• page.tsx:494-516 — Prev/Next are text-xs with no padding, so the hit target is ~16px (WCAG 2.5.8 wants ≥24×24). px-2 py-1 -mx-2 fixes it. No sibling pagination exists in app/admin, so this sets the precedent.
• page.tsx:359-388 — the confirm swaps a <button> for a <span> subtree, so React unmounts the focused element and focus falls to <body>; a keyboard user has to Tab from the top of the document to reach "Confirm delete". Also, every row's control is named just "Delete"/"Confirm delete" — a screen-reader user hears "Delete, Delete, Delete" with no way to tell which dataset. An aria-label={t('list.deleteAria', { name: ds.name })} and focusing the confirm button when confirmDelete === ds.id covers both. (_components/ConfirmDialog.tsx:35-43 does the focus part; admin/registries/page.tsx:124 and admin/mcp/page.tsx:396 both name the target.)
• page.tsx:226-242 — the only visible native <input type="file"> in the app; the other three (SkillImportModal.tsx:80-93, store/UploadDropzone.tsx:201-207, store/builder/_components/ImportBundleButton.tsx:196-202) are className="hidden" behind a styled trigger. Side effect: globals.css:701,711 explicitly excludes [type='file'] from the input focus recipe, so it's the one focusable control here without the house focus ring. Sighted users also get no visible caption, while the name field right below it does.
• page.tsx:257-263 / :361-388 — Button already ships a danger variant and busy/busyLabel (which emit aria-busy); the page reimplements both as a manual label swap, so assistive tech gets no in-flight signal. admin/registries/page.tsx:234-240 uses the component form for the same shape.
• api.ts:4564-4568 — DatasetRowsResult.rows is required where the shared DatasetQueryResult.rows is optional (plugin-api/src/knowledgeGraph.ts:2158-2171). Unreachable today (the route never forwards an aggregate, and both shipped backends always populate rows), and the comment says why — but KnowledgeGraph is a plugin interface, and rows?: + rows.rows?.length would cost nothing.
• docs/middleware-agent-handoff.md:1649 — Phase 14 is marked — ✅ ERLEDIGT (#532) but left inside ## 13. Offene Roadmap. It's the only ✅ ERLEDIGT marker in the file; every other completed phase was removed from the roadmap rather than annotated. Consider deleting the block and letting the CHANGELOG carry the record.

Checked and deliberately not raised

Verified against the repo and found to be house convention or correct, so they're not findings: the local TABLE_WRAP/TH_CLS/TD_CLS constants (every other admin table inlines these — this is more DRY than its neighbours, not less); relative ../../_lib/api next to aliased @/app/_components/ui/Button (77 relative vs 25 aliased repo-wide, same pairing in 11 admin pages); the react-hooks/set-state-in-effect disable (~48 sites, rule is warn); redeclaring DatasetSummary/DatasetColumnSchema in api.ts (web-ui has no dependency on @omadia/plugin-api); all nine CSS tokens exist and the /8 opacity idiom appears 93× ; key={i} on preview rows; missing role="alert" / <caption> / scope (zero uses in app/admin); String(row[col.name] ?? '') correctly uses ?? so 0 and false render; the 960px shell matches 11 sibling pages character for character; and the CHANGELOG entry matches CONTRIBUTING.md:131-133 and the adjacent #439 entry.

I also traced the useEffect(() => { void reload(); }, [reload]) identity chain through use-intl's memoization — t is stable, the effect runs once, no loop.

(Reviewed against head e5105af.)

[ConnysCode]

Rows from one dataset can render under another dataset's schema. loadRowsPage checks selected at call time (:165) but writes setRows/setRowsOffset unconditionally after the await (:172-173), and the dataset-name buttons that call openDetail are never disabled (:339-345). Both interleavings are broken:

• Click Next → on dataset A, then click dataset B before A's page lands. If B resolves first, the final state is selected = B but rows = A's page 2. The <thead> is built from selected.columns (:462-466) while the cells come from rows.rows (:470-479) — so any column the two CSVs share (id, name, date, very common in exports) renders A's values as B's rows, with no visual cue that anything is wrong.
• If A's late page lands first instead, rowsOffset sticks at 25 while B's page 1 is on screen: the footer reads Showing 26–50 of <B total> (:487-491) and Next jumps to offset 50, silently skipping B's rows 26–50.

For a page whose stated purpose is letting an operator see what actually landed in the graph, showing one dataset's rows under another's headers is the wrong failure mode.

Please add a generation guard — capture const my = ++seqRef.current before each fetch and if (my !== seqRef.current) return; after every await in both openDetail and loadRowsPage — and disable the row trigger while a detail load is in flight (which also stops N parallel getDataset/getDatasetRows pairs from one impatient operator). The repo already uses the simpler let cancelled variant of this in app/admin/domains/page.tsx:56-77.

[ConnysCode]

A failed delete is reported as a failed load, the stale row stays listed, and the confirm is disarmed. This catch writes into the list-load slot, which renders as t('loadError', …) — "Loading failed: {message}" / "Laden fehlgeschlagen: {message}" (web-ui/messages/en.json:3810, rendered at :309-313).

Concretely: another session already deleted the dataset, so DELETE /v1/datasets/:id returns 404 { code: 'dataset.not_found' } (middleware/src/routes/datasets.ts:200-202). The operator gets "Loading failed: Request failed (dataset.not_found)." sitting above a list that loaded perfectly well — and on a long list the banner is above the table, so someone who clicked Delete on a bottom row sees nothing at all.

Two more problems in the same handler:

• reload() only runs on the success path (:192), so the row that failed to delete is still listed and the operator can't tell whether it's gone.
• finally clears confirmDelete (:197), so retrying means re-arming the two-click guard from scratch.

Please give delete its own error state with its own key (e.g. list.deleteFailed) rendered next to the list, call reload() in the catch as well so the table reflects reality, and leave confirmDelete armed on failure. app/admin/registries/page.tsx:35,288 already keeps a dedicated actionError for exactly this.

[ConnysCode]

This promises instance-wide scope on a page that only ever lists your own datasets. Every dataset handler is scoped to the session user, not to an admin role (middleware/src/routes/datasets.ts:32-39, :138; SQL filter owner_omadia_user_id = $2 at neonKnowledgeGraph.ts:710). Cross-user datasets genuinely exist — the chat-attachment path ingests with the chatting user as owner (harness-orchestrator/src/orchestrator.ts:5547-5554). Neither this intro nor the /admin index card at :214 says any of that.

Failure mode: an operator on a page titled "Knowledge · Datasets", sitting in the /admin grid under Knowledge & Memory, sold as PII governance ("Every upload is privacy-scanned for PII before it lands in the graph"), deletes every row here and reasonably concludes the instance has no dataset-borne PII left. In fact every CSV any other user dropped into chat is still stored, still queryable via query_dataset, and invisible here.

To be explicit about what this is not: it isn't a new gating hole. /admin/* isn't role-gated anywhere (web-ui/proxy.ts:25-49 only checks for a live session), no sibling admin API checks a role either, and owner-only is the documented #430 ACL (plugin-api/src/knowledgeGraph.ts:713-716). The defect is the copy over-promising, on a page placed in the admin area.

Please make the copy honest — state in intro and in the index-card description that the list covers the datasets you own, e.g. "Lists the datasets you own; CSVs other users imported from chat attachments are not shown." Mirror it in de.json. An admin-scoped list variant is the real long-term answer, but the copy fix removes the misleading claim today.

[ConnysCode]

Clicking a dataset name produces no feedback, and the result lands off-screen. openDetail sets rowsLoading = true but leaves selected === null until both fetches resolve (:147-152), while the only loading indicator lives inside {selected !== null && …} at :405/:522. So from click to resolve, nothing on the page changes — no spinner, no aria-busy, no disabled trigger.

Then the panel mounts below the header, the upload section and the entire list table. At ~20 datasets it starts well below the fold, with no scrollIntoView and no focus move. The user clicks a name and, as far as they can tell, nothing happened.

This isn't the house pattern: app/admin/mcp/page.tsx:442,512 expands an inline <tr data-detail-row> directly beneath the clicked row, and admin/users / admin/duplicates / admin/inconsistencies navigate to a dedicated [id] route. This is the only admin list→detail that renders the detail in a distant sibling section.

Please render the loading state where the click happened (or mount a placeholder panel while rowsLoading), set aria-busy/disabled on the trigger while in flight, and scrollIntoView({ behavior: 'smooth', block: 'start' }) on open — already used in app/conductor/page.tsx:131 and app/_components/store/AdminUiPanel.tsx:87. Expanding an inline row like admin/mcp would be better still.

[ConnysCode]

detailError is never cleared, so the red banner outlives both a successful retry and this Close button. setDetailError(null) appears exactly once, in openDetail (:142). loadRowsPage's success path (:172-173) doesn't clear it, this Close handler clears selected and rows but not detailError, and the banner renders at :399-403 — outside the selected !== null guard at :405.

Repro: open a dataset, click Next →, the request 500s → banner appears. Click ← Prev → succeeds, rows update, banner still there. Click Close → the whole detail panel disappears and the red banner stays on the page with nothing left to explain it, undismissable short of opening another dataset or reloading.

Please clear detailError at the top of loadRowsPage and in this Close handler.

[ConnysCode]

Changing the selected file keeps the previous file's derived name, so the dataset is stored under the wrong one. The guard only auto-fills when name is empty, and it can't tell "user typed this" from "we derived this from the file they just replaced".

Repro: pick customers.csv → name becomes customers. Notice it's the wrong file, pick orders.csv → name stays customers. Upload → uploadDataset(file, 'customers') (api.ts:4603 appends the non-empty name) and the dataset containing orders.csv is persisted as "customers" (middleware/src/routes/datasets.ts:95-98 only falls back to file.originalname when the field is absent or blank). Silent, no warning.

Please track a nameTouched flag set in the text input's onChange (:252) and re-derive on every file change while it's false.

[ConnysCode]

Two problems in this mapper: the raw exception string is rendered as primary UI text, and the parsed message is thrown away.

(a) This fallback is rendered bare at :267-271 ({uploadError}) and :399-403 ({detailError}) — no catalog wrapper. web-ui/CLAUDE.md:33-35 is explicit: "Error messages are user-facing strings too. Never render a raw ApiError/exception message as the primary UI text; give it a catalog key and put the technical detail behind it." With the middleware down, fetch rejects with a TypeError — not an ApiError — so a German operator's entire error box reads "Failed to fetch". If an upstream proxy answers 502 with HTML, JSON.parse throws and the box reads "POST /v1/datasets failed: 502". loadError gets this right at :311, so the same file is internally inconsistent.

(b) datasetErrorCode parses { code } and discards the sibling message the middleware always populates, and errorByCode is one generic string — "Request failed ({code})." — not the catalog the docblock at :52-56 promises. So a header-only CSV shows "Request failed (dataset.import_failed).", dropping the one sentence that says what to fix; a 40 MB upload shows "Request failed (dataset.limit_file_size)." with no mention of the 25 MB cap (datasets.ts:25,69-74) and no client-side size check.

Preferring the code over message is defensible — the middleware messages are hardcoded German and would leak into the English UI — so this is about finishing the job the comment describes.

Please (a) wrap both renders in catalog keys mirroring loadError (uploadError: "Upload failed: {message}", detailError: "Could not open the dataset: {message}"), and (b) add per-code keys for the codes an operator will actually hit — limit_file_size (with the limit in the text), unsupported_type, import_failed, not_found — falling back to errorByCode for the rest.

[ConnysCode]

This logs an IntlError to the console for every row rendered. No timeZone is configured anywhere — i18n/request.ts's loadConfig returns only { locale, messages }, and app/layout.tsx passes only locale/messages to NextIntlClientProvider — so use-intl fires onError(IntlError ENVIRONMENT_FALLBACK) on every format.dateTime call, and the default onError is console.error (in production builds too).

Verified at head: npx vitest run app/admin/datasets --reporter=verbose emits 32 ENVIRONMENT_FALLBACK errors — from 5 tests rendering a one-row table. A deployment with 50 datasets logs 50 console errors per render pass.

This is new surface, not pre-existing: format.dateTime appears nowhere else in web-ui (grep across app/ returns only this line). The other useFormatter() callers use format.number only, and every other admin date goes through toLocaleString with an explicit zone. Secondary effect: these dates render in the viewer's local zone while the rest of the app doesn't.

Please add timeZone to the object returned by loadConfig in i18n/request.ts (fixes it app-wide), or pass it at this call site.

[ConnysCode]

Bare {arg} placeholders skip locale number formatting, so the same page formats the same quantity two different ways. In ICU, only {arg, number} (and # inside a plural) goes through Intl.NumberFormat — a bare {total} is plain string substitution. Rendered through this repo's own catalogs and translator:

en rowsShowing => Showing 1–25 of 4213      (want 4,213)
de rowsShowing => Zeige 1–25 von 4213       (want 4.213)
en privacyScan => Privacy scan: 1200 of 45000 cells masked.
en success     => Imported 4,213 rows.      ← correct, plural `#`

Meanwhile the list's Rows column does format correctly via format.number(ds.rowCount) (page.tsx:348) — so a 4 213-row dataset reads "4.213" in the table and "von 4213" in the preview footer two sections below. web-ui/CLAUDE.md:36-39 requires locale-aware number formatting.

Please switch to the ICU number type here and in upload.privacyScan (:3821), in both locales: "Showing {from, number}–{to, number} of {total, number}" and "Privacy scan: {masked, number} of {scanned, number} cells masked." Placeholder names don't change, so i18n:check parity still passes.

[ConnysCode]

The fixtures make the two most bug-prone branches unreachable. totalMatched: 2 against a page size of 25 means the pagination controls can never render, so loadRowsPage, Math.max(0, rowsOffset - ROWS_PAGE_SIZE) and the next-disabled predicate rowsOffset + rows.rows.length >= rows.totalMatched (page.tsx:163-181, :494-517) — the only arithmetic in the file — have zero coverage. That's exactly where the request-guard bug I flagged at page.tsx:172 lives. Likewise truncatedCellCount: 0 at :119 means the truncation branch (page.tsx:286-294) never renders, and truncation + privacy stats are what the page docstring and the CHANGELOG entry both call out as the point of the change.

Also uncovered: the delete-error path (page.tsx:193-198), the load-error path, the empty state, cancel-confirm, and detail-close. And mockUploadDataset is only asserted with toHaveBeenCalledOnce() (:131, :156), never toHaveBeenCalledWith(file, name), so the name-derivation logic has no coverage either.

For context, this suite is roughly at the house bar, not below it — embedding-provider/__tests__/page.test.tsx is 7 tests, ProvidersPanel.test.tsx 6, and those are the only two sibling admin-page tests in the repo. And the parts that are here are sound: renderWithIntl loads the real en.json, so the ICU assertions exercise production strings, and the mocked ApiError signature genuinely matches api.ts:182-191 so instanceof works across the vi.mock boundary.

Please raise totalMatched to e.g. 60 with mockGetDatasetRows keyed on offset and add one test clicking Next that asserts getDatasetRows was called with { limit: 25, offset: 25 }, plus one upload test with truncatedCellCount > 0 asserting the truncation line renders. Two tests, and they cover the failure this review is asking you to fix.