[LKPR-140] Dashboard V2 real E2E (FE+BE) suite + 5 regression fixes - #341
Conversation
Two latent Dashboard V2 bugs exposed by wiring the E2E suite to a real backend (they were masked by LKPR-137's skip guards + preview-mode fallbacks): 1. AppShell.svelte was catastrophically overwritten in a839f1f [LKPR-128] with a verbatim copy of NavRail — dropping {@render children()}, TopBar, Toast, and CommandPalette. Every page has rendered only the nav rail since Jul 12. Restored the correct shell composition (from 6168b6f, incl. palette). 2. memories/+page.svelte called replaceState(params.toString(), page.url) — wrong signature: url without '?' prefix + a URL object as state (not structured-cloneable), throwing could-not-be-cloned and crashing the table render. Fixed to match sibling pages: replaceState with query string + {}.
sessions/review/links pages call syncUrl() before load() in their mount $effect. replaceState throws if the SvelteKit router isn't initialized yet (initial mount), which aborted the effect before load() ran — leaving the page stuck on its 'Loading…' state with no data ever fetched. URL sync is cosmetic, so wrap replaceState in try/catch: if the router isn't ready this pass, skip the sync and let load() proceed. Verified in browser — sessions now renders 3 seeded sessions instead of hanging.
The backend serializes memory rows with an 'id' field (serialize_memory),
but the memories page's MemoryRow type and handlers reference 'lore_id'.
tableRows mapped { ...r, id: r.lore_id } — since lore_id was always
undefined, both the DataTable row key and the row-click id were undefined,
so clicking a row fetched /api/memories/undefined (404) and the detail
drawer never opened.
Bridge the contract: derive the real id from r.lore_id ?? r.id and populate
both lore_id and id. Verified in browser — row click now opens the drawer.
seed.py defaulted to a persistent LORE_DATA_DIR (/tmp/lk-e2e). On any re-run the near-identical fixture text tripped the dedup threshold (0.6·semantic + 0.4·keyword >= 0.85), so insert returned 0 inserted, the < 10 guard raised SystemExit, and the Playwright webServer never booted — failing the entire suite. Wipe the data dir up front so every local and CI run starts from a clean, deterministic state.
- Result rows are <li role="option" class="result-row">, not .result-item; assert on role=option inside the listbox so the count is correct. - Wait for the async query outcome (rows or empty state) to render rather than the results container, which mounts before results arrive. - Enter-key test: wait for the Run button to enable (proves bind:value committed queryText) before pressing Enter, else the keydown handler reads a stale empty value and runQuery() early-returns.
The tooltip only renders for cells with cell.total > 0 (empty cells show nothing by design). The old test hovered the grid's first cell, which is an empty (0-call, tabindex=-1) cell → no tooltip. Target the first interactive cell (tabindex="0") instead, which is guaranteed to hold the seeded tool-call metrics.
The tab buttons are server-rendered; a click landing before Svelte attaches the onclick handler is a no-op, leaving aria-selected=false. Wait for networkidle (post-hydration signal) before clicking, matching the pattern already used by the bulk-select test in this file.
…tests During the client-side data fetch the table renders aria-hidden skeleton rows; a click on `tbody tr` first-match could land on a skeleton or a not-yet-hydrated row and no-op, so the detail drawer never opened. Only real data rows carry the `.clickable` class, so target `tbody tr.clickable` — Playwright auto-waits for it, guaranteeing the click hits a row with an attached onclick handler.
SessionDrawer declared role="complementary" despite being a modal drawer (overlay scrim, focus trap, Escape-to-close, aria-label). Screen readers announced it as a sidebar landmark rather than a modal dialog, and it was unreachable via the dialog role. Align with MemoryDetailDrawer, which correctly uses role="dialog" + aria-modal="true". Removed the now-unused svelte-ignore (the element is interactive once it has the dialog role). Surfaced by the LKPR-140 real E2E suite.
The app selects its command-palette modifier from the browser's reported platform (navigator.userAgentData.platform === 'macOS' → metaKey, else ctrlKey). Playwright's ControlOrMeta alias resolves from the HOST OS instead, so a macOS host driving a headless Chromium that reports 'Windows'/'Linux' sent Meta+K while the app listened for Ctrl+K — the hotkey silently never fired. Detect the browser platform in-page and press the exact modifier, with an expect.toPass retry to absorb the onMount hotkey-listener hydration window. Correct on every host and CI.
The first search after seed lazy-loads the embedding model and builds the BM25 index; under full-suite CPU contention this can exceed the previous 10s inner wait (the test intermittently failed in a full run but passed in isolation). Raise the result-visible timeout to 25s (test-level cap is 30s) to absorb the one-time cold start deterministically.
The links and sessions drawer tests previously bailed via test.skip() when no rows were present — false confidence, since with a real seeded backend rows are guaranteed. Assert the row link is visible (auto-waiting) and let the test fail loudly if data is missing. This is the core intent of LKPR-140: real FE+BE coverage, no silently-skipped cases.
playwright.config.ts: dual webServer[] — [0] seeds fixtures then starts the FastAPI backend, [1] the Vite dev server; the browser hits the FE origin and Vite proxies /api/* to the backend. vite.config.ts: dev-server /api proxy targeting the backend port (E2E_BACKEND_PORT override). Default ports offset to 7787/7788 (was 7777/7778). 7777 is the conventional live dev-dashboard port; with reuseExistingServer a plain `npx playwright test` would silently hijack that server and run against the wrong data. Offset defaults make local runs collision-free out of the box. Verified: full non-visual suite (48 tests) green with no env overrides. CI is unaffected — it spawns fresh servers (reuseExistingServer=false) regardless of port.
Playwright suffixes screenshot baselines with the OS (-linux.png etc.), so macOS-generated baselines can't be diffed on the Linux CI runner. Until CI-environment baselines are generated and committed, skip the 8 @visual tests unless RUN_VISUAL=1, keeping the suite green on every host and in CI. Refresh baselines in a Linux container with: RUN_VISUAL=1 npx playwright test --grep @visual --update-snapshots Follow-up ticket will commit the Linux baselines and flip CI to RUN_VISUAL=1.
Runs the real end-to-end suite on ubuntu-latest: sets up Python/uv, pre-warms the HuggingFace model, installs Node + Dashboard V2 deps + Chromium, then runs `npx playwright test` which seeds the backend and starts FE+BE via the config's dual webServer. Uploads report + traces/screenshots on failure. Per-run LORE_DATA_DIR isolates seed state. @visual tests skip (RUN_VISUAL unset) until Linux baselines land. .gitignore: stop ignoring the visual-baseline dir so committed baselines can serve as the CI diff target once generated.
svelte-check flagged a11y_no_noninteractive_element_to_interactive_role: <aside> is a landmark (complementary) element and can't carry the interactive 'dialog' role. Switch the panel to <div> — the .drawer class holds all styling, so this is purely the element tag. Matches MemoryDetailDrawer, which is a <div role="dialog">. Clears the svelte-check warning introduced when the role was corrected in cbaef5b.
The test tsconfig scope omits @types/node, so a bare process.env reference
failed svelte-check ('Cannot find name process'). Read the flag off
globalThis with a narrow inline cast instead — no new dependency, and the
visual gate behaves identically.
Coverage report
The report is also available in the GitHub Actions job summary. |
…oded nav badge
- MemoryDetailDrawer save/delete/forget were silent no-ops (onDrawerSave stub
returned false). Added updateMemory/deleteMemory API helpers and wired the
handlers to PATCH/DELETE /api/memories/{id}, mirroring the working Sessions page.
- Removed hardcoded Review nav badge (=8) that never reflected real pending count.
…CH/DELETE target real id
- memories.spec.ts: full edit→save→reload round-trip asserting the PATCH targets a real id and the change survives reload (guards the no-op regression). - shell.spec.ts: Review badge must match /api/health pending_suggestions (guards the hardcoded-8 regression). - test_backend_coverage.py: TestMemoryFieldPatch covers title + source_type PATCH persistence and 404 on unknown id.
|
@coderabbitai help check for major medium issue |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR metadata ==='
gh pr view 341 --json number,title,baseRefName,headRefName,changedFiles,additions,deletions,url
echo '=== Changed files ==='
gh pr diff 341 --name-only
echo '=== Diff stat ==='
gh pr diff 341 --stat
echo '=== Repository status and merge-base diff summary ==='
git status --short
base="$(git merge-base HEAD origin/$(gh pr view 341 --json baseRefName --jq .baseRefName))"
git diff --summary "$base"...HEAD
echo '=== Outlines: changed source/config files ==='
git diff --name-only "$base"...HEAD |
while IFS= read -r f; do
case "$f" in
*.ts|*.svelte|*.py|*.yml|*.yaml|*.json)
[ -f "$f" ] || continue
echo "--- $f ---"
ast-grep outline "$f" --items all 2>/dev/null | head -n 160 || true
;;
esac
doneLength of output: 3093 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/main)"
echo '=== Local diff stat ==='
git diff --stat "$base"...HEAD
echo '=== Runtime product fixes ==='
git diff --no-ext-diff --unified=80 "$base"...HEAD -- \
src/dashboard_v2/src/lib/api/memories.ts \
src/dashboard_v2/src/lib/components/overlays/SessionDrawer.svelte \
src/dashboard_v2/src/lib/components/shell/AppShell.svelte \
src/dashboard_v2/src/lib/components/shell/NavRail.svelte \
src/dashboard_v2/src/lib/constants/routes.ts \
src/dashboard_v2/src/routes/links/+page.svelte \
src/dashboard_v2/src/routes/memories/+page.svelte \
src/dashboard_v2/src/routes/review/+page.svelte \
src/dashboard_v2/src/routes/sessions/+page.svelte \
src/lorekeeper/dashboard/routes/memories.py \
src/lorekeeper/domains/memory/repository.py
echo '=== E2E/CI configuration ==='
git diff --no-ext-diff --unified=100 "$base"...HEAD -- \
.github/workflows/ci.yml \
src/dashboard_v2/playwright.config.ts \
src/dashboard_v2/vite.config.ts \
src/dashboard_v2/tests/seed.py \
src/dashboard_v2/.gitignore \
tests/test_backend_coverage.py
echo '=== Test changes ==='
git diff --no-ext-diff --unified=35 "$base"...HEAD -- \
src/dashboard_v2/tests/links.spec.ts \
src/dashboard_v2/tests/memories.spec.ts \
src/dashboard_v2/tests/metrics.spec.ts \
src/dashboard_v2/tests/query.spec.ts \
src/dashboard_v2/tests/review.spec.ts \
src/dashboard_v2/tests/sessions.spec.ts \
src/dashboard_v2/tests/shell.spec.ts \
src/dashboard_v2/tests/visual.spec.tsLength of output: 118728 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Candidate file locations ==='
fd -t f . src tests | rg '(^|/)(server|__main__|metrics|heatmap|handler|app|home|seed)\.(py|svelte|ts)$|dashboard'
echo '=== Structural outlines ==='
for f in \
src/dashboard_v2/tests/seed.py \
src/dashboard_v2/src/routes/metrics/+page.svelte \
src/dashboard_v2/src/routes/memories/+page.svelte \
src/lorekeeper/dashboard/handler.py \
src/lorekeeper/dashboard/app.py \
src/lorekeeper/server.py; do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" --items all 2>/dev/null | head -n 220 || true
fi
done
echo '=== Metrics/heatmap implementation and writes ==='
rg -n -i -C 5 \
'hm-cell|heatmap|tool.?call|record.*metric|MetricsStore|metrics_store|tabindex' \
src/dashboard_v2/src src/lorekeeper tests \
-g '*.{ts,svelte,py}'
echo '=== Dashboard CLI and port behavior ==='
rg -n -C 5 \
'ArgumentParser|add_argument\(.+port|uvicorn|def main\(|__main__|LORE_DASH_RELOAD' \
src/lorekeeper -g '*.py'
echo '=== Memory list/detail serialization and ID fields ==='
rg -n -C 6 \
'serialize_memory|list_memories_paginated|def get_memory|lore_id|["'\'']id["'\'']' \
src/lorekeeper/dashboard src/lorekeeper -g '*.py'Length of output: 193570 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Dashboard memory update path ==='
rg -n -A30 -B8 'def update_memory|update_memory_fields|def update_memory' \
src/lorekeeper/dashboard/handler.py \
src/lorekeeper/processors/memory.py \
src/lorekeeper/domains/memory \
src/lorekeeper -g '*.py'
echo '=== Drawer edit fields and save payload ==='
rg -n -A18 -B12 \
'onSave|edited|content|description|drawer-title|save' \
src/dashboard_v2/src/lib/components/overlays/MemoryDetailDrawer.svelte \
src/dashboard_v2/src/lib/components/overlays/types.ts \
src/dashboard_v2/src/lib/api/memories.ts
echo '=== Metrics page and interactive heatmap ==='
fd -t f . src/dashboard_v2/src | rg '(metrics|heatmap|Activity).*\.(svelte|ts)$' | while read -r f; do
echo "--- $f ---"
rg -n -A12 -B8 'hm-cell|role=.button|tabindex|tooltip|tool-calls|fetch' "$f" || true
done
echo '=== Metrics written by seeded operations ==='
rg -n -A8 -B8 'increment_metric|increment_metric_safe' \
src/lorekeeper/processors/reflection.py \
src/lorekeeper/processors/memory.py \
src/lorekeeper/domains/reflection/service.py \
src/lorekeeper/domains/suggestion/sweep.py \
src/lorekeeper/platform/metrics/repository.py
echo '=== Exact changed-line locations ==='
nl -ba src/dashboard_v2/src/routes/memories/+page.svelte | sed -n '95,175p'
nl -ba src/lorekeeper/dashboard/handler.py | sed -n '124,150p'
nl -ba src/lorekeeper/domains/memory/repository.py | sed -n '195,230p'Length of output: 126454
|
There was a problem hiding this comment.
Pull request overview
This PR replaces the previous "false-confidence" Dashboard V2 Playwright setup (preview-mode + test.skip-on-empty guards) with a real seeded backend + live frontend E2E suite: the browser hits the Vite dev origin, which proxies /api/* to a FastAPI backend seeded with deterministic fixtures. Wiring up a genuinely functional stack surfaced (and this PR fixes) five latent product regressions in the dashboard shell, memory write path, and a11y — the changes therefore touch core product code, not just test infra.
Changes:
- Test infrastructure: new
seed.pyfixture seeder, dualwebServerinplaywright.config.ts, Vite/apiproxy, offset ports (7787/7788), hydration/cold-start hardening, skip-guards → hard assertions, and a newplaywright-dashboardCI job. - Product regression fixes: restored the overwritten
AppShell(children/TopBar/Toast/CommandPalette), fixedreplaceStatecrashes (URL-object clone + pre-router guards), wired the memory drawer save/delete round-trip withid/lore_idbridging, and correctedSessionDrawerfromrole="complementary"torole="dialog" aria-modal="true". - Backend: added
source_typeto theMemoryUpdatemodel and the repository allowed-field set, with pytest coverage for the PATCH round-trip.
Review notes: The frontend fixes are consistent with existing patterns (SessionDrawer now matches MemoryDetailDrawer; the AppShell imports all resolve; fetchHealth().pending_suggestions feeds the live NavRail badge). CI action versions (@v7) match the rest of ci.yml. One moderate bug remains: the seed's explicit fixture links use invalid relation_type values, so they are silently dropped.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/dashboard_v2/tests/seed.py |
New in-process fixture seeder; explicit links use invalid relation types (related/supports) — silently dropped. |
src/dashboard_v2/playwright.config.ts |
Dual webServer (backend seed+serve, Vite dev proxy), offset ports. |
src/dashboard_v2/vite.config.ts |
/api dev proxy to the FastAPI backend, port overridable via env. |
.github/workflows/ci.yml |
New playwright-dashboard job (Python+Node+HF setup, npm ci, Playwright run, artifact upload). |
src/dashboard_v2/src/lib/components/shell/AppShell.svelte |
Restored full shell (children/TopBar/Toast/CommandPalette, hotkey, health badge). |
src/dashboard_v2/src/lib/components/shell/NavRail.svelte |
Adds pendingReview prop; overlays live Review badge count. |
src/dashboard_v2/src/lib/constants/routes.ts |
Removes hardcoded badge: 8 on the Review route. |
src/dashboard_v2/src/routes/memories/+page.svelte |
Wires drawer save/delete, bridges id/lore_id, fixes replaceState. |
src/dashboard_v2/src/lib/api/memories.ts |
Adds updateMemory/deleteMemory helpers. |
src/dashboard_v2/src/routes/{links,sessions,review}/+page.svelte |
Guards replaceState against pre-router crash. |
src/dashboard_v2/src/lib/components/overlays/SessionDrawer.svelte |
Modal drawer now role="dialog" aria-modal="true". |
src/lorekeeper/dashboard/routes/memories.py |
Adds source_type to MemoryUpdate. |
src/lorekeeper/domains/memory/repository.py |
Adds source_type to allowed update fields. |
tests/test_backend_coverage.py |
New PATCH round-trip tests (title, source_type, 404). |
src/dashboard_v2/tests/*.spec.ts |
Skip-guards → hard assertions, palette modifier, query/hydration timing. |
src/dashboard_v2/tests/visual.spec.ts + .gitignore |
Gate @visual behind RUN_VISUAL=1; un-ignore baseline dir. |
docs/plans/...md, backlogs/ready/LKPR-140...md |
Planning + backlog docs (describe original 7777/7778 + globalSetup approach). |
| explicit_links = [ | ||
| { | ||
| "source_memory_id": ids[0], | ||
| "target_memory_id": ids[1], | ||
| "relation_type": "related", | ||
| "reason": "fixture: related pair", | ||
| }, | ||
| { | ||
| "source_memory_id": ids[2], | ||
| "target_memory_id": ids[3], | ||
| "relation_type": "supports", | ||
| "reason": "fixture: supporting pair", | ||
| }, | ||
| ] |
LKPR-140 — Dashboard V2 real end-to-end (FE + BE) test suite
Replaces the previous false-confidence E2E setup (preview-mode +
test.skip-on-empty guards) with a real seeded backend + live frontend Playwright suite. The browser hits the Vite dev origin, which proxies/api/*to a genuine FastAPI backend seeded with deterministic fixtures. Graph feature (LKPR-138) excluded per scope.Result
48/48 functional tests green. 8
@visualtests gated off pending Linux baselines (see caveat). Full suite verified repeatedly on fresh seeds.The old preview-mode + skip guards were masking a non-functional dashboard. Fixed inside this ticket (option A — ACs were unachievable otherwise):
a839f1f, LKPR-128) — every page rendered nav-only since Jul 12;{@render children()}, TopBar, Toast, CommandPalette all dropped. Restored.replaceStatecrash — URL-object clone threw on the memories page.syncUrl()pre-router crash — sessions/review/links hung on "Loading…" (replaceState threw in mount effect before the router initialized).id/lore_idcontract mismatch — memory detail drawer fetched/api/memories/undefined→ 404, drawer never opened.role="complementary"— a modal drawer mis-declared as a sidebar landmark; unreachable by the dialog role, mis-announced to screen readers. Nowrole="dialog" aria-modal="true"on a<div>(matches MemoryDetailDrawer).Test-infrastructure fixes
seed.pywipes its data dir up front; without it, re-runs tripped the dedup threshold → 0 inserts → whole webServer boot aborted.reuseExistingServera plainnpx playwright testwould silently hijack it and run against the wrong data. CI unaffected (spawns fresh servers).tbody tr.clickable(skip aria-hidden skeleton rows); wait for hydration before tab clicks;expect.toPass()retry for the onMount-attached command-palette hotkey.navigator.userAgentData.platform), not Playwright's host-derivedControlOrMeta.bind:valueflush before Enter; 25s result timeout for the one-time embedding/BM25 cold start.playwright-dashboardjob on ubuntu-latest (seeds + runs full suite via dual webServer).Playwright suffixes screenshot baselines with the OS (
-linux.png). CI-environment (Linux) baselines can't be generated here (Docker daemon unavailable), so the 8@visualtests are gated behindRUN_VISUAL=1to keep the suite green everywhere. Follow-up: generate Linux baselines in a container (mcr.microsoft.com/playwright), commit them, flip CI toRUN_VISUAL=1.Notes
svelte-checkerrors remain in untouched files (DataTable_children/_selectedRows, RelationshipDrawer.test vitest globals, query page type) — out of scope, not introduced here.