7646f3236f
Web UI rework lifecycle: verify + sync phases. - verify-report.md: PASS verdict (23/64 vitest, 5/5 node, zero @mui in src, 71/71 tasks, all spec scenarios green). Non-blocking findings recorded: slice-5 review-budget overage (5a/5b not split); node --test tests cmd typo. - sync-report.md + openspec/specs/web-ui/spec.md: canonical domain spec distilled from the verified change (design system, thin-dashboard model, TanStack visibility-only tables, reconciled IA, removed deps, Vitest harness). - Change-side delta spec under openspec/changes/web-ui-rework/specs/.
274 lines
19 KiB
Markdown
274 lines
19 KiB
Markdown
# Verify Report — web-ui-rework
|
||
|
||
> Phase: **verify** · Change: `web-ui-rework` · Repo: `/home/user/Manage_01` (frontend at `frontend/`)
|
||
> FRESH-CONTEXT adversarial verification of the full change against `proposal.md`,
|
||
> `spec.md`, `design.md`, `tasks.md`, and `apply-progress.md`. All 8 apply slices
|
||
> are committed (`c767fc6` slice 1 … `baf412b` slice 8, with sub-splits 6a/6b/7a/7b).
|
||
> Read-only verification; no source edits. The verify report is the only file written.
|
||
|
||
**Head commit verified:** `baf412b` (`feat(frontend): slice 8 — remove MUI/@emotion deps + update REQUIREMENTS`).
|
||
|
||
---
|
||
|
||
## 0. Executive summary / verdict
|
||
|
||
**VERDICT: PASS** (proceed to `sdd-sync`).
|
||
|
||
The MUI v9 → shadcn/ui + Tailwind v4 + lucide-react migration is **complete and
|
||
verified green** at `baf412b`. Every spec scenario was checked against source and
|
||
passes: zero `@mui/*`/`@emotion/*` imports in `frontend/src` (grep-verified);
|
||
`@mui/*`, `@emotion/*`, `recharts`, `d3` absent from `package.json`; `theme.ts`
|
||
deleted; `/media` is canonical with `/applications` → `<Navigate to="/media" replace />`
|
||
mirroring `/monitoring` → `/observability`; top-level Backups nav item present;
|
||
both TanStack tables are **visibility-only** (no `getSortedRowModel`, no resizing —
|
||
the only `enableSorting` occurrence is `enableSorting: false` on the selection
|
||
display column, which *enforces* the no-sort rule); ObservabilityPage parity
|
||
preserved (no in-app charts); `docs/REQUIREMENTS.md` updated comprehensively.
|
||
|
||
All four gates are green: `npm run build` (exit 0), `npm run lint` (exit 0, 2
|
||
*pre-existing* warnings verified identical at baseline `ef5311b`), `npm test`
|
||
(Vitest **23 files / 64 tests**), `node --test` (auto-discover, **5/5**). TDD
|
||
evidence is genuine and behavioral (selection toggle, select-all, column-visibility,
|
||
row-click + stopPropagation edge cases, the exact 15-column Media set, status→Badge
|
||
variant mapping). **71/71 tasks checked**; zero unchecked implementation tasks.
|
||
|
||
Two findings, neither blocking:
|
||
|
||
- **[WARNING] Review-workload boundary deviation on slice 5.** Slice 5 (Settings +
|
||
Actions) shipped as a single commit with ~1,097 hand-written source insertions
|
||
(`Settings.tsx` +758, `Actions.tsx` +338), exceeding the 400-line budget **without**
|
||
the prescribed `5a`/`5b` sub-split. Code is correct and tested; this is a
|
||
process/budget deviation, not a defect.
|
||
- **[INFO] Structured-status artifact-format note.** The change uses a flat
|
||
`spec.md` (15 requirements, complete) with no `openspec/specs/<domain>/` delta
|
||
specs. The native status engine reports `specs: partial` / `applyState: blocked`
|
||
on this basis. The flat spec is the authoritative contract the work was built
|
||
against and is complete; the domain-spec format gap is a **sync/archive**
|
||
concern, not a verify blocker (the verify phase itself is `ready`).
|
||
|
||
---
|
||
|
||
## 1. Structured status & actionContext findings
|
||
|
||
Consumed from the native `gentle-pi.sdd-status` (authoritative):
|
||
|
||
- `changeName: web-ui-rework`, `artifactStore: openspec`, change root correct.
|
||
- Artifacts present and populated: `proposal`/`design`/`tasks`/`apply-progress` =
|
||
done; `spec.md` present (legacy-flat, no domain delta specs).
|
||
- `taskProgress`: **total 71, complete 71, remaining 0, unchecked []** —
|
||
independently re-confirmed: `grep -nE '^\s*- \[ \]' tasks.md` → **NONE**;
|
||
`grep -cE '^\s*- \[x\]'` → **71**.
|
||
- `verify: ready`; `apply: blocked`, `sync: blocked`, `archive: blocked`.
|
||
- `actionContext`: `mode: repo-local`, `workspaceRoot: /home/user/Manage_01`,
|
||
`allowedEditRoots: ["/home/user/Manage_01"]`, `warnings: []`. Implementation
|
||
ownership and target files are provably inside the authoritative workspace. ✓
|
||
- `blockedReasons`: "domain specs are missing or partial" + "Legacy flat spec
|
||
present without domain specs." → This is an artifact-**format** condition
|
||
(no `openspec/specs/web-ui/` delta). It does **not** block verify; the flat
|
||
`spec.md` is complete and was the contract for the work. It gates **sync/archive**.
|
||
|
||
## 2. Gate results (actual output, run from `frontend/` at `baf412b`)
|
||
|
||
| Gate | Command | Result | Evidence |
|
||
|------|---------|--------|----------|
|
||
| Build | `npm run build` (`tsc -b` + `vite build`) | **PASS** exit 0 | `✓ built in 789ms`; 1959 modules transformed. Non-fatal `>500 kB` chunk-size warning (pre-existing, present at baseline). |
|
||
| Lint | `npm run lint` (`eslint .`) | **PASS** exit 0 | `✖ 2 problems (0 errors, 2 warnings)`. Both warnings `react-hooks/exhaustive-deps` in `UsersPage.impl.tsx` (lines 120, 159). **Adversarially verified pre-existing** at baseline `ef5311b` (same `baseRows` + `rows`-missing-dep warnings). |
|
||
| Vitest | `npm test` (`vitest run`) | **PASS** | `Test Files 23 passed (23)` · `Tests 64 passed (64)`. Matches expected 23/64. |
|
||
| node:test | `node --test` (auto-discover) | **PASS** 5/5 | `# tests 5 / # pass 5 / # fail 0`. Legacy `users.test.mjs` + `userState.test.mjs`. |
|
||
|
||
### ⚠ Acceptance-crosscheck command correction (recorded, NOT a regression)
|
||
|
||
`tasks.md` and `apply-progress.md` reference `node --test tests` (or `npm run
|
||
test:node`). **This command is pre-existing broken** (verified identically at
|
||
baseline `ef5311b`): `node` treats the bare `tests` argument as a *module path*,
|
||
failing with `Cannot find module '/home/user/Manage_01/frontend/tests'` (real
|
||
exit code `1` at both HEAD and `ef5311b`). The **correct** command is
|
||
`node --test` (auto-discover), which passes **5/5**. This is a documentation
|
||
typo in the cross-check line, not a defect introduced by the rework.
|
||
|
||
### Runtime gates configured by parent
|
||
|
||
- `zero-mui`: `grep -rlE '@mui/(material|icons-material|x-data-grid)|@emotion/(react|styled)' src` → **ZERO** ✓
|
||
- `build`: `npm run build` → **passed** (exit 0) ✓
|
||
- `test`: `npm test` → **passed** (23/64) ✓
|
||
- `exists`: `test -s verify-report.md` → this file ✓
|
||
|
||
## 3. Spec scenario coverage (spec.md, 15 requirements)
|
||
|
||
| # | Requirement / scenario | Check | Result |
|
||
|---|------------------------|-------|--------|
|
||
| R1 | No MUI/`@emotion` imports anywhere in `frontend/src` | `grep -rlE '@mui/(material\|icons-material\|x-data-grid)\|@emotion/(react\|styled)' src` | **ZERO** ✓; deps absent from `package.json` ✓ |
|
||
| R1 | No new design system adopted | `package.json` deps | Only shadcn/Tailwind/lucide/radix; no `@mui`/`@emotion`/`recharts`/`d3` ✓ |
|
||
| R2 | `recharts`/`d3` gone from `package.json` + no imports | grep src + package.json | **ZERO** imports; absent from deps ✓ |
|
||
| R2 | No chart component introduced; metrics = numbers/Badges/Grafana links | ObservabilityPage inspection | No recharts/d3; no chart canvas ✓ |
|
||
| R2 | `theme.ts` deleted; no `theme`/`getAppTheme` imports | `test ! -e src/theme.ts` + grep | **GONE**; **NO-IMPORTS** ✓ |
|
||
| R3 | Primary stays `#4f8cff` (light + dark) | `index.css` | `--color-primary: #4f8cff` in `@theme` (L13) **and** `.dark` (L52) ✓ |
|
||
| R3 | `chart-1..5` retained + repurposed (documented) | `index.css` | All 5 present in **both** blocks with role comment (chart-1=info, 2=success, 3=warning, 4=destructive, 5=neutral) ✓ |
|
||
| R4 | Comfortable density (`p-4 md:p-6`, `gap-4`); no compact mode | `App.tsx` shell + grep | `<main className="p-4 md:p-6">`; no compact/dense toggle found ✓ |
|
||
| R5 | Status Badge semantics (success/warning/destructive cues) | `badge.tsx` + tests | `success` (chart-2) + `warning` (chart-3) variants present; mapping asserted in tests ✓ |
|
||
| R6 | Backups top-level nav item → `/backups` | `App.tsx` `navItems` | `{ path: "/backups", label: "Backups", icon: DatabaseBackup }` (after Files, before Users) ✓ |
|
||
| R7 | Media canonical `/media`; `/applications` → redirect; `/monitoring`→`/observability` intact | `App.tsx` routes | `<Route path="/media" element={<Applications />}/>` + `<Route path="/applications" element={<Navigate to="/media" replace />}/>` in **both** route blocks; `/monitoring`→`/observability` intact ✓ |
|
||
| R7 | Auth/routing model unchanged | `App.tsx` | `AuthProvider`/`react-oidc-context`/`react-router-dom` structure unchanged ✓ |
|
||
| R8/R9/R10/R11 | TanStack visibility-only (no sort, no resize) | `data-table.tsx`/`Media.tsx`/`FileBrowser.impl.tsx` | **No** `getSortedRowModel`, **no** `enableColumnResizing`/`columnResizing`/sortable header. Only `enableSorting: false` (selection column). `getCoreRowModel` + conditional `getPaginationRowModel` (manual for Media) ✓ |
|
||
| R8 | Column visibility parity (Media 15 / FileBrowser 5) | column defs + tests | Media: `title,series,season,episode,type,year,runtime_min,size,bitrate,hdr,video,resolution,date_added,library,path` = **15** ✓; FileBrowser: `type,name,ext,size,modified` = **5** ✓ (both asserted in tests) |
|
||
| R12 | Media pagination parity (server-driven) | `Media.tsx` + `data-table.tsx` | `enablePagination`+`manualPagination`+`rowCount` (from `queryResult.total`); `limit`/`offset` lifted to `usePersistentState` ✓ |
|
||
| R13 | Observability parity (alerts + target health + Grafana deep-links; no in-app chart) | `ObservabilityPage.tsx` | On shadcn (card/badge/alert/select/skeleton/collapsible); no recharts/d3; Grafana/ExternalLink refs present ✓ |
|
||
| R14 | Vitest harness introduced; migrated components have tests; legacy node suites pass | `package.json` + `src/**` | `vitest` + `@testing-library/react` present; 23 test files; `node --test` 5/5 ✓ |
|
||
| R15 | Per-slice build + lint green gate | gate runs | All green at HEAD ✓ |
|
||
| R16 | Eight-slice delivery strategy | git log | 10 commits: slices 1–8 + 6a/6b/7a/7b sub-splits ✓ |
|
||
| R17 | `docs/REQUIREMENTS.md` updated | grep | Documents shadcn/Tailwind/lucide, thin-dashboard model, TanStack tables, `/media` + `/applications` redirect, Backups nav, removed deps, decision-log entry 2026-06-17 ✓ |
|
||
| R18 | No backend API / `frontend/src/types/*` changes | spec non-goal | No router/type-contract changes introduced (scope honored) ✓ |
|
||
|
||
**Spec coverage: 100% of verifiable scenarios PASS.**
|
||
|
||
## 4. Task completion status
|
||
|
||
- **71/71 tasks checked.** Zero unchecked implementation task lines
|
||
(`grep -nE '^\s*- \[ \]' tasks.md` → none).
|
||
- No archive blockers from incomplete tasks.
|
||
|
||
## 5. TDD compliance & assertion-quality assessment
|
||
|
||
Strict-TDD was active for this change (design §6, spec R14; apply-progress carries
|
||
`TDD Cycle Evidence (standard mode; RED → GREEN)` tables at the slice-7a and
|
||
slice-7b sections, e.g. `apply-progress.md:54` and `:1066`).
|
||
|
||
**Assertion quality — assessed as GENUINELY BEHAVIORAL, not trivial.** Spot-checked
|
||
the highest-risk tests:
|
||
|
||
- `data-table.test.tsx` (9 tests): row selection toggle + state reflection + un-select;
|
||
header select-all (indeterminate → all-checked → all-unchecked); **column-visibility
|
||
dropdown removes a column and leaves others intact**; **`onRowClick` fires with
|
||
`row.original` on row-body click**; **does NOT fire when a selection checkbox is
|
||
toggled** (stopPropagation edge case); empty message; client pagination controls;
|
||
manual-pagination total (`rowCount=42` → "42 rows", "Page 1 of 21").
|
||
- `Media.test.tsx`: **"exposes exactly the 15 locked toggleable columns"**
|
||
(`toEqual([...sorted 15...])`, `toHaveLength(15)`, `not.toContain("__select__")`);
|
||
**"navigates to the file browser at the item path on row click"**
|
||
(`navigate` called once with the encoded path); **"does NOT navigate when toggling
|
||
a selection checkbox"**; server-driven pagination total.
|
||
- `FileBrowser.test.tsx`: 5 locked columns via `arrayContaining` + uniqueness;
|
||
**clicking a file row selects for ffprobe preview**; **clicking a directory row
|
||
navigates in** (clean dir-vs-file distinction).
|
||
- `BackupAlertsTable.test.tsx` + `SessionActivityPanel.test.tsx`: status→Badge
|
||
variant mapping asserted via rendered `data-variant` attribute —
|
||
critical→`destructive`, warning→`warning`, playing/healthy→`success`, paused→`warning`,
|
||
idle→`secondary`. **This directly covers spec R5 "Healthy status uses a success cue"
|
||
and "Firing alert uses a destructive cue."** No type-only, CSS-detail, or
|
||
tautological assertions found.
|
||
|
||
No tautologies, no ghost loops, no smoke-only, no implementation-detail CSS
|
||
assertions identified. Tests assert the spec-parity behaviors (visibility-only
|
||
features, row-click, selection/stopPropagation, exact column sets, status semantics).
|
||
|
||
## 6. Review-workload / PR-boundary findings
|
||
|
||
Per-slice changed-line counts (numstat, excluding `package-lock.json` churn):
|
||
|
||
| Commit | Slice | Source Δ (no-lock) | Lockfile | Sub-split? | Verdict |
|
||
|--------|-------|--------------------|----------|------------|---------|
|
||
| `c767fc6` | 1 Foundation | 2,616 (≈1,527 = planning docs; ≈984 = 15 vendored shadcn primitives; ≈60 = hand-written) | 2,135 | — | **OK** — vendored-generated + planning-doc dominated; explicit size exception per forecast |
|
||
| `b8be41f` | 2 Shared | 1,299 (10 rewrites) | 0 | not split | Minor over; per-file boundaries clean |
|
||
| `befebb6` | 3 Backups + nav | 1,043 | 0 | — | OK (forecast "likely OK"; close to budget) |
|
||
| `b6c3b76` | 4 Dashboard + Applications | 988 | 0 | not split | Minor over |
|
||
| **`cd95f25`** | **5 Settings + Actions** | **2,837 (Settings +758 / Actions +338 + 240 test)** | 0 | **NOT split** | **⚠ WARNING** — exceeded 400 by ~2.7× without prescribed 5a/5b |
|
||
| `3f7b249` | 6a Users dir | 1,620 | 0 | **6a done** ✓ | OK (mandatory split honored) |
|
||
| `5601575` | 6b Users compose | 251 | 0 | **6b done** ✓ | OK |
|
||
| `df2a4de` | 7a DataTable + FileBrowser | 1,819 | 0 | **7a done** ✓ | OK (mandatory split honored) |
|
||
| `58f41c6` | 7b Media | 1,177 | 0 | **7b done** ✓ | OK |
|
||
| `baf412b` | 8 Cleanup + docs | 318 | 626 | — | OK |
|
||
|
||
**Mandatory sub-splits honored:** slices 6 and 7 (the forecast-confirmed over-400
|
||
slices) were both sub-split as prescribed (6a/6b, 7a/7b). ✓
|
||
|
||
**Deviation — slice 5 [WARNING]:** The forecast (`tasks.md` Review Workload
|
||
Forecast) rates slice 5 "Medium-High" with policy *"If over, split 5a (Actions)
|
||
→ 5b (Settings)."* Slice 5 was delivered as a **single commit** with **~1,097
|
||
hand-written source insertions** (`Settings.tsx` +758, `Actions.tsx` +338), far
|
||
over the 400-line budget, **without** the `5a`/`5b` sub-split. `apply-progress.md:739`
|
||
characterizes this as "under the 400-line added budget," which is **incorrect**
|
||
under the standard insertions+deletions review metric (it appears the author used
|
||
net delta). This is a **review-workload process deviation**, not a correctness
|
||
defect: both pages are fully migrated, MUI-free, covered by 240 lines of
|
||
component tests, and the gates are green. The per-file boundary (Settings vs
|
||
Actions) is clean. Recommend noting this in the archive summary so the 5a/5b
|
||
forecast-vs-actual delta is on record; **does not block sync/archive**.
|
||
|
||
## 7. Residual risks
|
||
|
||
1. **No visual / browser smoke was performed** (none in scope; component tests
|
||
assert DOM structure and behavior, not pixel fidelity). Layout regressions
|
||
(spacing, table density, Sheet/Drawer transitions, responsive grids) are only
|
||
covered structurally. A manual browser smoke of Media paging + row-click,
|
||
FileBrowser row-click preview, the Users compose dialog, and Backups tabs is
|
||
advisable before release — not a verify gate.
|
||
2. **`lucide-react@^1.14.0` pin re-confirmed.** Residual risk (unusual major)
|
||
verified resolved: all 14 names the rework depends on — `Pencil, X, Paperclip,
|
||
Bold, Italic, Link, List, Mail, Send, Trash2, DatabaseBackup, ExternalLink,
|
||
HardDrive, Archive` — **export at the installed version** (`node -e` import
|
||
probe → `ALL_EXPORT_OK`). The `DatabaseBackup` fallback (`HardDrive`/`Archive`)
|
||
was not needed.
|
||
3. **`ResizeObserver` polyfill is test-only.** Defined in `src/test/setup.ts`
|
||
(jsdom no-op stub for Radix primitives). Not a runtime concern; flagged for
|
||
completeness.
|
||
4. **Two `react-hooks/exhaustive-deps` lint warnings** in `UsersPage.impl.tsx`
|
||
(`baseRows` logical expr; `rows` missing memo dep) — **adversarially verified
|
||
pre-existing** at baseline `ef5311b` (identical substance, pre-dating slice 1).
|
||
They are warnings (lint passes at exit 0), but if the project ever tightens
|
||
eslint to `--max-warnings 0`, these would surface. Worth a follow-up cleanup
|
||
task outside this change.
|
||
5. **`node --test tests` cross-check command is broken** (pre-existing typo;
|
||
correct form is `node --test`). Recorded; recommend correcting the
|
||
`npm run test:node` script + tasks.md/apply-progress cross-check lines in a
|
||
follow-up docs commit. Not introduced by this change.
|
||
6. **Chunk-size build warning** (`index-*.js` ~701 kB > 500 kB) is non-fatal and
|
||
pre-existing; orthogonal to this migration but worth future code-splitting.
|
||
7. **Slice-5 review-budget overage** (see §6) — single-commit delivery above the
|
||
400-line forecast without the prescribed sub-split. Process note for the
|
||
archive record.
|
||
|
||
## 8. Exact blockers
|
||
|
||
**None (CRITICAL).** No blockers to `sdd-sync`. The two findings (slice-5
|
||
review-budget deviation; flat-spec vs domain-spec format) are WARNING/INFO and
|
||
do not gate sync. All spec scenarios pass, all gates green, all tasks complete,
|
||
TDD evidence genuine.
|
||
|
||
## 9. Recommended next phase
|
||
|
||
→ **`sdd-sync`** (PASS). Before archive: reconcile the flat `spec.md` into
|
||
`openspec/specs/web-ui/` delta specs (the native status blocks archive on the
|
||
missing domain specs), and optionally correct the `node --test tests` typo +
|
||
record the slice-5 forecast-vs-actual in the archive summary.
|
||
|
||
---
|
||
|
||
### Appendix A — Verification commands run (authoritative, at `baf412b`)
|
||
|
||
```
|
||
cd /home/user/Manage_01/frontend
|
||
npm run build → exit 0 (✓ built in 789ms; >500kB chunk warning pre-existing)
|
||
npm run lint → exit 0 (0 errors, 2 pre-existing warnings)
|
||
npm test → 23 files / 64 tests passed
|
||
node --test → 5/5 pass (auto-discover) [correct command]
|
||
node --test tests → FAIL exit 1 "Cannot find module .../tests" [PRE-EXISTING broken; identical at ef5311b]
|
||
grep -rlE '@mui/(@mui/(material|icons-material|x-data-grid)|@emotion/(react|styled)' src → ZERO
|
||
test ! -e src/theme.ts → GONE
|
||
lucide import probe (14 names) → ALL_EXPORT_OK
|
||
```
|
||
|
||
### Appendix B — Files substantively changed by the change (representative)
|
||
|
||
- `frontend/src/App.tsx` — `/media` canonical, `/applications` redirect, Backups nav, `/monitoring`→`/observability` intact.
|
||
- `frontend/src/components/ui/data-table.tsx` — new TanStack wrapper (visibility-only).
|
||
- `frontend/src/pages/Media.tsx`, `frontend/src/pages/FileBrowser.impl.tsx` — off `@mui/x-data-grid`.
|
||
- `frontend/src/components/ui/{tabs,table,dialog,input,label,checkbox,switch,progress,separator,avatar,textarea,dropdown-menu,scroll-area}.tsx` — added primitives.
|
||
- `frontend/src/components/ui/badge.tsx` — `success`/`warning` variants (chart-2/chart-3).
|
||
- `frontend/src/index.css` — `chart-1..5` role comments (both blocks); primary `#4f8cff` unchanged.
|
||
- `frontend/vitest.config.ts`, `frontend/src/test/setup.ts` — harness.
|
||
- `frontend/package.json` — `@tanstack/react-table` + vitest/testing-library added; `@mui/*`,`@emotion/*`,`recharts`,`d3` removed; `theme.ts` deleted.
|
||
- `frontend/src/**/__tests__/*.test.tsx` (23 files) — component tests.
|
||
- `docs/REQUIREMENTS.md` — rework documented.
|
||
- `frontend/src/components/{SectionCard,SelectionRailCard,TabbedCard,MetricCard,DiskSpaceCard,HoverEditButton,DialogFooter,ConfirmDialog,LibraryOverview,NowPlaying,SessionActivityPanel,Backup*,ObservabilityPage}.tsx`,
|
||
`frontend/src/pages/{Dashboard,Applications,Settings,Actions,UsersPage.impl}.tsx` — migrated to shadcn/Tailwind/lucide.
|