diff --git a/.planning/phases/02-ui-performance/02-FINDINGS.md b/.planning/phases/02-ui-performance/02-FINDINGS.md new file mode 100644 index 00000000..2c043238 --- /dev/null +++ b/.planning/phases/02-ui-performance/02-FINDINGS.md @@ -0,0 +1,190 @@ +# Phase 02 UI Performance — Findings (D-10) + +**Status:** Gate doc — committed before any `neode-ui/src` change lands in this phase (D-10). +Every claim below cites a field in `02-PERF-BASELINE.json`; nothing here is inferred from +code reading alone. + +## Method + +**Target:** archi-dev-box (the on-device verification target named by D-11), resolved to +`http://archi-dev-box` from the executing workstation. This is a real Archipelago node, not +the local mock backend or the `:8100` dev preview — `02-PERF-BASELINE.json`'s run header +records `baseUrl: "http://archi-dev-box"` and `commit: "a75b6709"` (the harness commit the +run was taken against). + +**Harness:** `neode-ui/e2e/perf/{surfaces,measure,surface-perf.spec}.ts` (built and committed +in this plan's Task 1). Run command: + +```bash +cd neode-ui +ARCHY_BASE_URL=http://archi-dev-box \ +ARCHY_PASSWORD= \ +ARCHY_PERF_OUT=../.planning/phases/02-ui-performance/02-PERF-BASELINE.json \ +npx playwright test e2e/perf/surface-perf.spec.ts --project=chromium --reporter=line +``` + +**Sample count:** 3 runs per surface (`runs: 3` in the artifact header), each run +measuring a fresh first-visit (navigate from dashboard Home to the surface) and a revisit +(navigate to the neutral Settings tab — or close the surface's own trigger, for the +wallet-send modal — then return via the same UI-click path) in the same authenticated +session. `02-PERF-BASELINE.json`'s `firstVisitMs`/`revisitMs`/`revisitRpcCount` top-level +fields are the **median** across successful runs; `samples` holds every individual run +(including any per-run errors) so no data is discarded. + +**Field meanings:** +- `firstVisitMs` / `revisitMs` — wall-clock ms from the click that starts navigation to + the surface's `contentSelector` becoming visible. +- `revisitRpcCount` — number of POSTs to `/rpc/v1` observed during the revisit window + (method name + timing only, never bodies — see the harness's threat mitigation). +- `maxConcurrentRpc` — peak number of RPC calls in flight simultaneously during the + revisit, derived by sweep-line over each call's `[start, start+duration]` interval. A + waterfall shows `maxConcurrentRpc: 1` with 2+ calls; a parallel fan-out shows + `maxConcurrentRpc` close to the total call count. +- `remounted` — read back from a dataset-stamp on the view's root element after the + away/back round-trip. `true` means the component instance was destroyed and recreated + (expected everywhere right now — `grep -r "KeepAlive" neode-ui/src` returns 0 hits, + confirmed again this session). + +**All 15 SURFACES rows were run against archi-dev-box.** 13 of 15 produced clean +first-visit/revisit/RPC/remount numbers. 2 are recorded as `unmeasured` with the specific +reason (never as "already fast" — see the Mesh and Chat rows below). + +**A methodological caveat that materially affects three rows' RPC evidence:** several +`navSteps` chains reach their target surface by first clicking through another main tab +(Marketplace and its own AppDetails go through Home's "Browse Store" link; OpenWrtGateway +goes through Server's "OpenWrt Gateway" link). Home.vue and Server.vue both fire a burst of +RPCs from their own `onMounted` the instant they're briefly mounted mid-chain +(`system.stats`, `fips.status`, `vpn.status`/`vpn.list-peers`, `wallet.*`, +`network.list-interfaces`, `tor.list-services`, etc.). Those in-flight requests are still +resolving when the chain lands on the destination surface, and the harness's RPC tracker — +which starts recording at the top of the revisit click-chain, not at the destination +surface's own mount — captures them as if they belonged to the destination. This was +confirmed by comparing captured method names directly: OpenWrtGateway's captured set +(`vpn.list-peers`, `network.list-interfaces`, `tor.list-services`, `fips.status`) is an +exact subset of Server.vue's own 8-call set measured cleanly on the `server` row (whose +`navSteps` do **not** transit another tab). Where this confound applies, the affected row's +Primary Cause is classified conservatively (`remount storm`, the reading the elevated +`revisitMs` alone supports) rather than `uncached fetch`, and the confound is called out +explicitly in that row's Intended Fix column so 02-02/02-03 re-verify with DevTools before +assuming the RPC count is real. Rows whose `navSteps` reach their target directly (Apps, +Cloud, Mesh, Server, Web5, Chat, AppDetails, CloudFolder) have no such confound — their RPC +counts are trustworthy as measured. + +## Per-Surface Findings + +| Surface | First visit (ms, median) | Revisit (ms, median) | Revisit RPC count | Max concurrent RPC | Remounted | Primary cause | Intended fix | Owning plan | +|---|---|---|---|---|---|---|---|---| +| Home (wallet figures) | 354 | 390 | 1 | 1 | true | already fast | Left alone (D-02) — still gets wrapped in `` per D-01 for scroll/animation-state persistence, no data-cache conversion needed | 02-04 (KeepAlive wrapping only) | +| Apps (My Apps) | 359 | 267 | 1 | 1 | true | already fast | Left alone (D-02) — KeepAlive wrapping only | 02-04 (KeepAlive wrapping only) | +| Marketplace (App Store) | 1228 | 2033 | 13 | 12 | true | remount storm | KeepAlive + convert to `useCachedResource`; **RPC count is confounded by the Home-transit artifact above — do not treat 13 calls as Marketplace's own fetch pattern without re-verifying via DevTools** | 02-02 (tracer/app store) | +| Discover (App Store tab) | 1117 | 1083 | 1 | 1 | true | remount storm | Convert to `useCachedResource` alongside Marketplace (same app-store surface family, D-09) | 02-02 (tracer/app store) | +| Cloud / Files | 463 | 247 | 0 | 0 | true | already fast | Left alone (D-02) — already on `useCachedResource` per code read; still gets KeepAlive wrapping | 02-04 (KeepAlive wrapping only) | +| Mesh | — | — | — | — | — | **unmeasured** | `.mesh-status-grid` never left the hidden/loading branch within the 20s wait window across all 3 runs — the device did not report `connected` on this node in the window measured. Re-run 02-05's own on-node pass once implementing; do not assume already-fast | 02-05 (Mesh) | +| Server (Network) | 969 | 738 | 8 | 7 | true | uncached fetch | Convert Server's 7–8 independent fire-and-forget `onMounted` calls to `useCachedResource` (clean measurement — `navSteps` reach Server directly, no transit confound) | 02-06 (Server and Home) | +| Web5 | 307 | 566 | 0 | 0 | true | remount storm | KeepAlive wrapping — 0 RPC on revisit confirms the sub-cards' existing `useCachedResource` usage already prevents refetch; remaining cost is render/mount of the multi-card tree | 02-04 (KeepAlive wrapping only) | +| Fleet | 376 | 330 | 0 | 0 | true | already fast | Left alone (D-02) — still gets KeepAlive wrapping | 02-04 (KeepAlive wrapping only) | +| Chat (AIUI) | — | — | — | — | — | **unmeasured** | AIUI's own "connecting" overlay stayed up long enough on this node that the harness's close-button click never resolved within its retry budget (`.chat-close-btn` timeout, 20s) in run 0; runs 1–2 timed out even earlier at 6.6s once the harness's stuck-state recovery kicked in. This is itself a data point worth carrying into 02-07: AIUI's connection/handshake latency on real hardware is materially longer than on the dev workstation | 02-07 (Chat/AIUI) | +| AppDetails (secondary) | 1040 | 1204 | 2 | 1 | true | uncached fetch | Convert `loadBitcoinSync()`/`loadCredentials()` to keyed `useCachedResource` (clean measurement — `navSteps` transit Apps, not Home, no confound) | 02-03 (secondary screens) | +| MarketplaceAppDetails (secondary) | 1978 | 1793 | 14 | 12 | true | remount storm | Convert to keyed `useCachedResource`; **13 of the 14 captured calls match the same Home-transit confound as Marketplace — only `package.versions` (the 14th call, firing later at +1459ms, well after the Home-mount burst settles) is trustworthy as this view's own fetch** | 02-03 (secondary screens) | +| CloudFolder (secondary) | 5116 | 1212 | 0 | 0 | true | remount storm | First visit is consistently ~5s (lazy route-chunk cold load, confirmed across runs); revisit drops to ~1.2s once the chunk is warm client-side but stays remount-bound (0 RPC either way) — convert to keyed `useCachedResource` for the folder listing regardless, per D-04's secondary-screen pattern | 02-03 (secondary screens) | +| OpenWrtGateway (secondary) | 3213 | 663.5 | 4 | 4 | true | remount storm | **RPC count is confounded by the Server-transit artifact above — the captured methods are an exact subset of Server.vue's own call set, not evidence of OpenWrtGateway's own fetch pattern.** Re-verify with DevTools before deciding whether this page needs its own `useCachedResource` conversion | 02-03 (secondary screens) | +| Wallet / send flow (Home wallet card → SendBitcoinModal) | 735 | 2607 | 0 | 0 | true | remount storm | **Anomaly worth flagging, not hiding:** revisit (2607ms median) is consistently *slower* than first visit (735ms median) across all 3 runs (2562/2619/2607 vs 857/735/607) — the modal always fully remounts (`BaseModal`'s `v-if`) with zero RPC either time, so the extra ~1.9s on reopen is pure client-side cost (fee/balance recompute, store re-subscription, or similar) that should be profiled directly with DevTools during 02-03, not assumed away | 02-03 (secondary screens) | + +## Ranked Fix Order + +Worst-revisit-first, main tabs and secondary screens together (medians from the table +above; `—` rows excluded, listed separately below): + +1. **Wallet-send — 2607ms** (secondary/modal; anomalous revisit > first-visit, see note above) +2. **Marketplace — 2033ms** (main tab; RPC evidence confounded, see caveat) +3. **MarketplaceAppDetails — 1793ms** (secondary; RPC evidence confounded, see caveat) +4. **CloudFolder — 1212ms** (secondary; first-visit-dominant, ~5s cold chunk load) +5. **AppDetails — 1204ms** (secondary; clean uncached-fetch evidence) +6. **Discover — 1083ms** (secondary; remount storm) +7. **Server — 738ms** (main tab; clean uncached-fetch evidence) +8. **OpenWrtGateway — 663.5ms** (secondary; RPC evidence confounded) +9. **Web5 — 566ms** (main tab; remount storm, 0 RPC) +10. **Home — 390ms** (main tab; already fast) +11. **Fleet — 330ms** (main tab; already fast) +12. **Apps — 267ms** (main tab; already fast) +13. **Cloud — 247ms** (main tab; already fast) + +Unmeasured: **Mesh**, **Chat** — see their rows above for why, and do not slot them into +this ranking as if they were fast. + +**Tracer tab for plan 02-02:** among *main tabs* specifically (02-02's actual scope — +wallet-send and the secondary screens are not tabs), **Marketplace is the worst-performing +surface (2033ms revisit)** and is also the surface the user named first ("often app +store" — D-09). It is the tracer pick for 02-02, consistent with the plan set's existing +"02-02 tracer/app store" naming. + +## Surfaces Left Alone (D-02) + +These measured `already fast` on real hardware — no `useCachedResource` conversion is +planned for their data layer. Per D-01 they still get wrapped in `` as part of +02-04's shared Dashboard.vue restructure (scroll/animation-state persistence is free and +harmless even for an already-fast view), but that is template plumbing, not a per-view +data-caching change, so they are listed here rather than under any per-surface fix plan. + +| Surface | First visit (ms) | Revisit (ms) | Revisit RPC | Why left alone | +|---|---|---|---|---| +| Home | 354 | 390 | 1 | Sub-400ms revisit with a single trivial `system.get-hostname` ping (not view-specific wallet data); wallet figures render from already-reactive store state | +| Apps | 359 | 267 | 1 | Sub-300ms revisit, single trivial ping; confirms RESEARCH.md's prediction that Apps.vue's data is WebSocket-pushed, not per-mount-fetched | +| Cloud | 463 | 247 | 0 | Sub-250ms revisit, zero RPC — confirms Cloud.vue's existing `useCachedResource` usage (`peersResource`/`countsResource`) is already preventing refetch | +| Fleet | 376 | 330 | 0 | Sub-350ms revisit, zero RPC | + +## Corrections to Prior Research + +**`ContainerAppDetails.vue` dead-code claim — CONFIRMED, stronger than RESEARCH.md stated.** +RESEARCH.md's Assumptions & Flagged Items (FA-B) reported the planner's grep found "only a +self-referential comment inside the file itself." Re-running the identical grep this +session found **zero matches at all**, including no self-reference: + +``` +$ grep -rn "ContainerAppDetails" neode-ui/src +(no output) +``` + +The file exists at `neode-ui/src/views/ContainerAppDetails.vue` (351 lines) but has no +importer anywhere in `neode-ui/src` and no route entry in `neode-ui/src/router/index.ts` +(verified directly against the full route table read for this plan's Task 1). It is +unreachable dead code. + +**Impact:** RESEARCH.md named `ContainerAppDetails.vue:169-172`'s +`await loadContainer(); await loadLogs(); await loadHealthStatus()` as the phase's one +*confirmed* serial-RPC-waterfall fix target (Pattern 3). Since the file is unreachable, **no +plan in this phase should spend effort parallelizing it** — doing so would touch dead code +for zero user-visible benefit. More importantly: **no other surface in the measured D-09 set +shows a serial-waterfall signature.** Every surface in this findings doc with 2+ RPC calls on +revisit (Marketplace, Server, AppDetails, MarketplaceAppDetails, OpenWrtGateway) shows +`maxConcurrentRpc` at or near its total call count (7 of 8, 12 of 13, 12 of 14, 1 of 1-2, 4 of +4) — i.e. already parallel, not serial. **`serial RPC waterfall` is not used as a Primary +Cause anywhere in the Per-Surface Findings table above** because the measured evidence does +not support it for any reachable surface. This is the single most consequential correction +this profiling pass makes to RESEARCH.md: D-13's `Promise.all`/`Promise.allSettled` +parallelization pattern has no live target to apply to in this phase's D-09 surface set. + +## Owning Plans + +| Surface | Owning plan | +|---|---| +| Home | 02-04 (KeepAlive wrapping only — see Surfaces Left Alone) / 02-06 names Home in its title but the profiling shows no data-layer work is needed there | +| Apps | 02-04 (KeepAlive wrapping only) | +| Marketplace | 02-02 (tracer/app store) | +| Discover | 02-02 (tracer/app store) | +| Cloud | 02-04 (KeepAlive wrapping only) | +| Mesh | 02-05 (Mesh) | +| Server | 02-06 (Server and Home) | +| Web5 | 02-04 (KeepAlive wrapping only) | +| Fleet | 02-04 (KeepAlive wrapping only) | +| Chat | 02-07 (Chat/AIUI) | +| AppDetails | 02-03 (secondary screens) | +| MarketplaceAppDetails | 02-03 (secondary screens) | +| CloudFolder | 02-03 (secondary screens) | +| OpenWrtGateway | 02-03 (secondary screens) | +| Wallet / send flow | 02-03 (secondary screens) | + +The shared foundation both 02-02 and every other per-surface plan depend on — the +Dashboard.vue nested-`` KeepAlive restructure and the `useCachedResource` +`onActivated` extension — is owned by **02-04**, per RESEARCH.md's own recommendation to +land that shared work before converting individual views.