Files
oikos/plans/done/2026-07-11-ui-review-ia-usability.md
dtoro c8b479d565 docs: close out task-completion safety net plan; fix stale relative links
Fixes 1-3 deployed and verified live: fresh trivial Q&A sessions now reach
done immediately, and a goal-bearing session that stalled was correctly
nudged by the idle sweep. Fix 4 (backfill) was replaced with deletion after
the operator's call — verified against the DB first that zero knowledge
notes were linked to or written by any of the 53 removed sessions, so
nothing was lost. Documents the pagination gap in listSessions (hardcoded
LIMIT 50, no total count) that hid 6 of those sessions from the original
audit.

Also fixes relative links in this plan and in the UI-review plan that broke
when both moved from plans/ to plans/done/ (one directory level deeper).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-12 00:14:15 +02:00

263 lines
14 KiB
Markdown

# UI review: information architecture, usability, and best practices
Status: Done — 2026-07-11. All fix-plan items implemented and verified live
except C2 (a11y lint enforcement — no ESLint/svelte-check is configured in
`web/` at all, so there's nothing to promote from warn to error; flagged
below instead of silently adding lint infra). Verification also surfaced an
unrelated pre-existing bug (Knowledge page search results never render) —
spun off as a separate task, not fixed here.
## Scope
Systematic review of `web/src/` (Svelte 5 + shadcn-svelte + Tailwind v4
control-room UI): all 13 pages, the 11 shared components, the sidebar/routing
shell (`App.svelte`), and cross-cutting patterns (filtering, loading/empty
states, live-event wiring, accessibility). Read in full, not sampled.
Grounded in what's actually in the code — no speculative "best practice"
items without a concrete file:line instance.
Not implementation. Findings and a proposed fix plan only, mirroring
[`2026-07-11-nomos-agent-code-review.md`](2026-07-11-nomos-agent-code-review.md)'s
structure — implement on a later "proceed."
## Findings
### A. Information architecture
**A1. Entity detail has two competing UI patterns for the same content.**
[`Entities.svelte:16-19,155`](../../web/src/pages/Entities.svelte) opens entity
detail as an in-page `EntitySheet` slide-over (no URL change, no sidebar
state change). [`Knowledge.svelte:57-59`](../../web/src/pages/Knowledge.svelte)
and [`Graph.svelte:464`](../../web/src/pages/Graph.svelte) instead navigate via
`location.hash = '#/entity/' + slug`, which `App.svelte`'s router resolves to
a full-page `EntityDetail` route — but `'entity'` isn't in `navItems`
([`App.svelte:68-79`](../../web/src/App.svelte)), so landing there leaves the
sidebar with nothing highlighted and the header showing the raw slug instead
of a section name. Same underlying view
(`EntityDetailContent.svelte`), three different entry points, two
different navigation models, one of which produces an orphaned page state.
A user who reaches an entity via Knowledge or Graph has no way back to
"where they were" via the sidebar — only browser back.
**A2. Two chat entry points with no visual link between them.**
The sidebar's "Tasks" section (board → `Chat.svelte` detail,
`isActive={page === 'tasks' || page === 'chat'}`,
[`App.svelte:127`](../../web/src/App.svelte)) and the footer's "Chat drawer"
button ([`App.svelte:160-163`](../../web/src/App.svelte), opens a `Sheet`
wrapping the same `Chat` component) are both valid, intentional ways to
reach chat — but nothing in the UI explains they're different modes (drawer
= overlay on current page, keeps your place; Tasks = full navigation). A
first-time user has no way to know which one preserves their current page.
Low-severity, but worth a tooltip/label distinction.
**A3. Overview's KPI cards don't drill down.**
[`Overview.svelte`](../../web/src/pages/Overview.svelte) shows "Pending
approvals," "Open signals," and fleet-health counts as static cards. The
header badges for the same data (`approvalsPending`, `openSignals`,
[`App.svelte:185-194`](../../web/src/App.svelte)) ARE clickable and navigate to
Ops/Signals — so the pattern exists in the app, just not on the page whose
entire purpose is summarizing this data. A dashboard card showing a count
that doesn't lead anywhere is a standard drill-down gap.
### B. Usability / interaction consistency
**B1. Table-row click targets lack keyboard/screen-reader support in one
place but not others.**
[`Entities.svelte:117-120`](../../web/src/pages/Entities.svelte) makes an
entire `Table.Row` clickable via a bare `onclick`, with no `role`,
`tabindex`, or `onkeydown` — unreachable and inoperable via keyboard, and
screen readers get no indication the row is interactive. This is a
regression against the codebase's own established pattern: `Tasks.svelte`
wraps its cards in real `<button>` elements
([`Tasks.svelte:172`](../../web/src/pages/Tasks.svelte)), `Events.svelte`'s
correlation-group headers are real `<button>`s
([`Events.svelte:110-114`](../../web/src/pages/Events.svelte)), and
`Graph.svelte`'s SVG nodes explicitly add `role="button"`, `tabindex="0"`,
and `onkeydown` ([`Graph.svelte:416-421`](../../web/src/pages/Graph.svelte)).
Entities is the outlier.
**B2. Filter inputs are inconsistently "live" vs. "apply-on-blur," with no
visual cue either way.**
`Entities.svelte`'s slug/name filter and `Graph.svelte`'s search box filter
as-you-type (bound to a `$derived`). But `Ops.svelte` (implicitly, no text
filters), `Audit.svelte`'s action/entity inputs
([`Audit.svelte:71-72`](../../web/src/pages/Audit.svelte)),
`Agent.svelte`'s agent_id input
([`Agent.svelte:59`](../../web/src/pages/Agent.svelte)), and `Events.svelte`'s
type/severity inputs ([`Events.svelte:92-93`](../../web/src/pages/Events.svelte))
all use `onchange`, which only fires on blur — a user typing a filter value
and watching the table sees nothing happen until they click or tab away, and
nothing in the UI (placeholder text, a debounce spinner, an "Enter to
apply" hint) tells them why. Three different pages share the same
`onchange`-only pattern, so it's a systemic choice, not an oversight — but
it reads as broken on first use.
**B3. Entity filter is case-sensitive; nothing else in the app is.**
[`Entities.svelte:50`](../../web/src/pages/Entities.svelte) matches with raw
`.includes()`, no `.toLowerCase()`. `Graph.svelte`'s equivalent search
normalizes both sides
([`Graph.svelte:175-176`](../../web/src/pages/Graph.svelte):
`n.slug.toLowerCase().includes(q)`). Slugs are lowercase by convention today,
which is why this hasn't bitten anyone yet, but entity *names* are
free text and can be mixed-case — a name filter that silently returns zero
results for a correctly-spelled but wrong-case query is a real trap, and the
one-line fix already has a working reference implementation three files
away.
**B4. `{@html}` on server-provided search snippets.**
[`Knowledge.svelte:120-121`](../../web/src/pages/Knowledge.svelte) renders
`hit.snippet` with `{@html}`, justified by a comment claiming the backend's
`ts_headline` output is pre-sanitized. That's true for Postgres
`ts_headline` today (it only wraps matched terms in `<b>` from a
parameterized query), but there's no client-side enforcement of that
invariant — if the search query or snippet source ever changes upstream,
this becomes a stored-XSS vector with no guard at the point of use. Not an
active vulnerability, but a fragile trust boundary worth tightening
defensively (e.g. a tiny allow-list sanitizer) rather than relying on a
comment to hold forever.
### C. Accessibility
**C1. `SessionRail.svelte`'s delete control is a `<span>`, not a button.**
[`SessionRail.svelte:54-64`](../../web/src/lib/components/SessionRail.svelte)
attaches `onclick` to a `<span>` for the per-session delete affordance, with
no `role`, `tabindex`, or keyboard handler — same defect class as B1, on a
destructive action this time (delete a chat session), which makes it a
notch more important: a keyboard-only user cannot delete a session from
this rail at all.
**C2. Same defect, lower stakes, elsewhere.**
Scan for the same "clickable non-interactive element" shape found in B1/C1
should be swept across `web/src/` once — these two are the ones a full read
surfaced, but the pattern (a `<div>`/`<span>` with `onclick` and no
keyboard path) is exactly the kind of thing that creeps back in per-PR
without a lint rule catching it. Worth checking whether
`eslint-plugin-svelte`'s `a11y_click_events_have_key_events` /
`a11y_no_static_element_interactions` rules are enabled and enforced in CI
(the prior summary noted these exist as warnings, not build failures — that
should be confirmed and possibly promoted to errors as part of implementing
C1/B1).
### D. Visual / component consistency
**D1. One page bypasses the shared `Button` component.**
`Agent.svelte`'s "Refresh" control is a bare
`<button class="rounded-md border px-3 py-1.5 text-xs">`
([`Agent.svelte:73`](../../web/src/pages/Agent.svelte)) instead of
`Button` (`variant="outline"`), which every other page's refresh/action
buttons use (`Ops.svelte`, `Signals.svelte`, `Audit.svelte`, `Events.svelte`
all use `<Button variant="outline">`). Cosmetically near-identical today
(both render as a bordered pill) but it'll drift the moment the design
tokens on `Button` change, since this one doesn't inherit them.
**D2. `formatEventLabel` is a needless indirection.**
[`Overview.svelte`](../../web/src/pages/Overview.svelte)'s
`formatEventLabel(ev)` returns `ev.type` verbatim — a one-line wrapper with
no formatting logic. Trivial, but noted since it reads as if formatting
were intended and never finished.
### E. Loading / empty states
No real findings — this is a strength worth naming rather than "fixing."
Every page reviewed (Overview, Entities, Ops, Signals, Events, Agent, Audit,
Knowledge, Learning, Graph, Tasks) has both a loading state (skeletons or an
implicit empty table) and an explicit, page-appropriate empty-state message
(not a generic "no data"). That consistency is worth preserving as new pages
get added — call it out in the PR template or a short frontend README note
rather than leaving it as tribal knowledge.
## Fix plan
Priority order, grounded in user impact:
1. **C1 (SessionRail delete button)** — highest priority: it's a destructive
action that's currently unreachable by keyboard at all. Swap the `<span>`
for a real `<button>` with `aria-label="Delete session"`, matching the
pattern `Tasks.svelte` already uses for its own delete affordance
([`Tasks.svelte:195-207`](../../web/src/pages/Tasks.svelte) — same feature,
done correctly, in the same codebase).
2. **B1 (Entities row click)** — wrap row content in a `<button>` (or add
`role="button" tabindex="0" onkeydown`) matching `Tasks.svelte` /
`Events.svelte`'s existing pattern.
3. **B3 (case-sensitive filter)** — one-line `.toLowerCase()` fix on both
sides of the `.includes()` calls in `Entities.svelte:50`.
4. **A1 (dual entity-detail navigation)** — pick one pattern. Recommend
standardizing on the `EntitySheet` (in-page, no navigation loss) and
changing `Knowledge.svelte`/`Graph.svelte`'s "View entity detail" actions
to open the sheet directly instead of hash-navigating to the orphaned
`#/entity/:slug` route. If the full-page route is kept for deep-linking
(a legitimate reason to keep it), then at minimum highlight the
originating section in the sidebar and give the header a real label
instead of the bare slug.
5. **A3 (Overview KPI cards not clickable)** — wrap the approvals/signals
cards in the same click-to-navigate pattern already used by the header
badges.
6. **D1 (Agent.svelte bare button)** — swap for `<Button variant="outline">`.
7. **B2 (inconsistent live-vs-blur filtering)** — standardize on
`oninput`-driven, debounced (~300ms) filtering across Audit/Agent/Events,
matching the already-live feel of Entities/Graph. Lower priority than the
above since it's a rough edge, not a defect.
8. **B4 (`{@html}` trust boundary)** — add a minimal sanitize step (strip
everything but the `<b>` tags `ts_headline` emits) at the point of
render, so the safety property doesn't depend on the backend never
changing.
9. **A2 (chat drawer vs. Tasks unlabeled)** and **D2 (`formatEventLabel`)**
cosmetic, do opportunistically or skip.
10. **C2 (a11y lint enforcement)** — checked: `web/` has no ESLint config and
no `lint`/`check` npm script at all (confirmed via `package.json` and
directory listing). The "a11y warnings" referenced in earlier session
notes were editor/IDE diagnostics, not a CI gate. There's nothing to
promote from warn to error because no lint infrastructure exists —
setting one up is a separate, larger decision (which rules, whether to
also add `svelte-check` for types) that wasn't part of this review's
scope. Not done; flagging for a separate decision rather than silently
bootstrapping tooling.
## Implementation notes (2026-07-11)
- C1, B1, B3, A1, A3, D1, D2, B2, B4, A2 all implemented and verified live
in the browser preview against the running stack (see Verification below).
- A1: standardized on `EntitySheet` per the plan's recommendation —
`Knowledge.svelte` and `Graph.svelte`'s "View entity detail" now open the
sheet instead of hash-navigating to the orphaned `#/entity/:slug` route.
The full-page `EntityDetail` route/component was left in place (not
deleted) as a harmless deep-link fallback — nothing internal navigates to
it anymore, but a bookmarked/shared URL still resolves.
- B4: used the `dompurify` package, already a `dependencies` entry in
`web/package.json` (unused until now) — no new dependency added.
- B2: added a small `debounce()` helper to `web/src/lib/utils.ts` and
switched Audit/Agent/Events' filter inputs from `onchange` (blur-only) to
debounced `oninput`.
- **Found during verification, not in the original fix list:** the
Knowledge page's search never actually renders results (the "Clear"
button appears, confirming `searched` flips to `true`, but the content
area stays on the "Recently learned" branch) despite the backend request
succeeding with real data. Confirmed via `git diff` this isn't caused by
anything touched here. Spun off as a separate follow-up rather than fixed
in this pass, since it's unrelated to any finding in this review.
## Verification
- After each interaction fix (C1, B1, A3): manual keyboard-only pass (Tab +
Enter/Space, no mouse) through the affected page in the browser preview.
- After B3: type a filter query in Entities with mixed case against a
known-mixed-case entity name; confirm it now matches.
- After A1: confirm both entry paths (Entities row click, Knowledge search
hit's linked entity, Graph node's "View entity detail") land on the same
UI pattern; confirm sidebar/header state is coherent from whichever page
the user started on.
- `cd web && npm run lint && npm run check` clean after all fixes.
- Visual: `npm run build` + spot-check each changed page in the browser
preview (light pass, not full regression).
## Open questions
- **A1's resolution direction** (sheet vs. full-page route) is a genuine
product call, not just a bug fix — needs a decision before implementing,
not just "proceed." Recommendation given above (standardize on the
sheet), but flagging it explicitly since it changes user-visible behavior
for Knowledge and Graph, not just Entities.
- Whether to promote a11y lint rules from warn to error (C2) is a policy
call for the repo, worth a one-line "yes/no" rather than silently doing
it.