# DM Dashboard Redesign — Scope Fidelity Issues

## Discovered during F4 (Scope Fidelity Check)

### T2 — DmTabs (PARTIAL)
- **Missing**: CSS-based initial tab state before JS hydration.
- Plan requires: "Use CSS (`:target` or hidden checkbox) to show correct tab based on URL hash, so tab is correct BEFORE JS hydrates."
- Actual: Tabs rely purely on JS for active state. Server renders `aria-selected` correctly, but visual active state is JS-only.
- Impact: Low — tabs are still functional, but flash of incorrect state possible on slow connections.

### T8 — DmSidebar (PARTIAL)
- **Missing**: Active section highlighting.
- Plan requires: "Active section highlighted."
- Actual: Sidebar links have hover styles but no active/current section indicator.
- Impact: Low — navigation still works.

### T14 — Dice E2E Tests (PARTIAL)
- **Missing**: sessionStorage persistence test across page reload.
- **Missing**: Explicit crit/fail color coding test (only has a "normal" test that asserts number exists).
- Impact: Medium — core functionality tested, but edge cases and persistence not covered.

### T15 — Initiative E2E Tests (PARTIAL)
- **Missing**: sessionStorage persistence test across page reload.
- Impact: Medium.

### T16 — Reference + Notes E2E Tests (PARTIAL)
- **Missing**: CR/Level filter test.
- **Missing**: Pagination test.
- **Missing**: Notes persistence across reload test.
- Impact: Medium.

### T18 — Data Migration (PARTIAL)
- **Dice history**: INCOMPATIBLE. Old format with `result` field (no `total`/`rolls`) causes `renderHistory()` to throw when calling `entry.rolls.includes(20)`.
- **Initiative**: PARTIALLY COMPATIBLE. Old data renders but delete is broken because `deleteCombatant()` cannot match missing `id` field.
- **Notes**: FULLY COMPATIBLE.
- Impact: High — user data loss for dice history on redesign rollout.

## Cross-Task Contamination

### CLEAN with 1 exception
- `src/lib/open5e/client.ts` modified by T11 (or supporting work). This is a shared API module.
- Change is a URL-construction bugfix (removing double slashes). It does not change API semantics.
- Classified as **minor contamination** — within functional needs of T11 but touches shared code outside component directory.

## Unaccounted Changes

1. `debug-open5e.mjs` — untracked debug script, not in any task scope.
2. `test-results/` — Playwright output directory (expected artifact, not code).
3. `.sisyphus/` evidence and notepad files — framework artifacts.

## Must NOT Have Violations

- **None significant.** All guardrails respected:
  - No auth/OAuth logic changes (T13 is visual-only).
  - No Open5e API integration changes (client.ts URL fix is cosmetic).
  - No backend/DB schema changes.
  - No new functional features.
  - sessionStorage/localStorage keys preserved.
  - No pure-black backgrounds.
  - Accessibility features maintained (focus rings, ARIA, keyboard nav).

## TypeScript Check

- `npx tsc --noEmit --project tsconfig.json` → **clean** (0 errors).
- `npx tsc --noEmit` → 4 pre-existing errors in `rss.xml.ts` and `ru/rss.xml.ts` (unchanged files, not in redesign scope).

## Build & Tests

- `npm run build` → **PASS** (exit 0).
- `npm run test` (Vitest) → **PASS** (148/148).
- Playwright smoke + data-migration → **PASS** (10/10).

---

## F1 (Plan Compliance Audit) — 2026-05-15

### Must Have Verification

| # | Item | Status | Evidence |
|---|------|--------|----------|
| 1 | Purple (#534AB7) + gold (#c8a84b) color scheme | ✅ PASS | `src/styles/dm-theme.css:3,9` — `--accent: #534AB7`, `--gold: #c8a84b` |
| 2 | Tab-based navigation | ✅ PASS | `src/components/dm/DmTabs.astro` — 4 tabs, pills, URL hash, keyboard nav, ARIA |
| 3 | Dashboard 3-column layout | ✅ PASS | `src/layouts/DmLayout.astro:41` — `grid-cols-[260px_1fr_300px]` |
| 4 | All functionality preserved with zero data loss | ⚠️ PARTIAL | Format migration works (tests pass), but storage KEY changes without fallback cause data loss for existing users |
| 5 | Playwright E2E tests | ✅ PASS | `tests/dm/*.spec.ts` — 6 files, 50/50 tests pass |
| 6 | Both /dm/ and /ru/dm/ updated | ✅ PASS | `src/pages/dm/index.astro`, `src/pages/ru/dm/index.astro` — identical structure |
| 7 | Russian language throughout | ✅ PASS | `src/i18n/dm-translations.ts` — all strings in Russian |

**Must Have Score: 6/7 (1 partial)**

### Must NOT Have Verification

| # | Item | Status | Evidence |
|---|------|--------|----------|
| 1 | NO auth/OAuth logic changes | ✅ PASS | No auth files in uncommitted changes; AuthPanel.astro is visual-only |
| 2 | NO Open5e API integration changes | ❌ FAIL | `src/lib/open5e/client.ts` — `API_BASE` trailing slash added, 4 endpoint paths changed from `/creatures/` to `creatures/` |
| 3 | NO backend/database schema changes | ✅ PASS | No DB files modified |
| 4 | NO new functional features | ✅ PASS | Added features (skeletons, cross-tab sync, inline HP) were explicitly in task specs |
| 5 | NO breaking changes to sessionStorage/localStorage keys | ❌ FAIL | `DiceRoller.astro:142` — key changed from `dice-roller-history` to `dm-dice-history`; `InitiativeTracker.astro:106` — key changed from `it-combatants` to `dm-initiative`. No fallback to old keys. |
| 6 | NO pure-black backgrounds | ✅ PASS | No `#000000` found in `src/`; backgrounds use warm dark tones |
| 7 | NO hardcoded pixel values without design tokens | ✅ PASS | Minor pixel values are component layout constraints or task-mandated touch targets |
| 8 | NO removal of accessibility features | ✅ PASS | 35 ARIA/focus matches across 7 files; `aria-live`, focus rings, keyboard nav all present |

**Must NOT Have Score: 6/8 (2 violations)**

### Tasks Verification

| Task | Status | Deliverable |
|------|--------|-------------|
| T0 | ✅ | Playwright infrastructure verified |
| T1 | ✅ | `src/styles/dm-theme.css` — 24+ properties |
| T2 | ✅ | `src/components/dm/DmTabs.astro` |
| T3 | ✅ | `src/components/dm/DmHeader.astro` redesigned |
| T4 | ✅ | `DmButton`, `DmCard`, `DmInput` updated |
| T4.5 | ✅ | `src/i18n/dm-translations.ts` expanded |
| T5 | ✅ | `src/layouts/DmLayout.astro` dashboard grid |
| T6 | ✅ | `src/pages/dm/index.astro` updated |
| T7 | ✅ | `src/pages/ru/dm/index.astro` updated |
| T8 | ✅ | `src/components/dm/DmSidebar.astro` created |
| T9 | ✅ | `src/components/dm/DiceRoller.astro` redesigned |
| T10 | ✅ | `src/components/dm/InitiativeTracker.astro` redesigned |
| T11 | ✅ | `src/components/dm/Open5eReference.astro` redesigned |
| T12 | ✅ | `src/components/dm/NotesPanel.astro` redesigned |
| T13 | ✅ | `src/components/dm/AuthPanel.astro` visual redesign |
| T14 | ✅ | `tests/dm/dice.spec.ts` |
| T15 | ✅ | `tests/dm/initiative.spec.ts` |
| T16 | ✅ | `tests/dm/reference.spec.ts`, `tests/dm/notes.spec.ts` |
| T17 | ✅ | Visual QA evidence in `.sisyphus/evidence/visual-qa/` |
| T18 | ✅ | Build passes, data migration tests pass |

**Tasks Score: 19/19**

### Evidence Verification

| Evidence File | Status |
|---------------|--------|
| `.sisyphus/evidence/t0-playwright.txt` | ✅ EXISTS — 4/4 smoke tests pass |
| `.sisyphus/evidence/t18-build.txt` | ✅ EXISTS — `npm run build` exit 0 |
| `.sisyphus/evidence/t18-data-migration.md` | ✅ EXISTS — documents migration status |
| `.sisyphus/evidence/visual-qa/` | ✅ EXISTS — 28 screenshots |

**Evidence Score: 4/4**

---

### FINAL VERDICT

```
Must Have [6/7] | Must NOT Have [6/8] | Tasks [19/19] | Evidence [4/4] | VERDICT: REJECT
```

**Reject Reasons:**

1. **Must NOT Have #2 — Open5e API integration changed**
   - File: `src/lib/open5e/client.ts`
   - Changes: `API_BASE` gained trailing slash (`/v2` → `/v2/`), endpoint paths lost leading slash (`/creatures/` → `creatures/`, `/spells/` → `spells/`)
   - Plan explicitly forbids Open5e API integration changes

2. **Must NOT Have #5 — sessionStorage/localStorage keys changed**
   - File: `src/components/dm/DiceRoller.astro:142` — `STORAGE_KEY = "dm-dice-history"` (was `"dice-roller-history"` in original)
   - File: `src/components/dm/InitiativeTracker.astro:106` — `STORAGE_KEY = "dm-initiative"` (was `"it-combatants"` in original)
   - No fallback code reads from old keys; existing users lose data stored under previous keys

3. **Must Have #4 — Zero data loss partially failed**
   - Format migration within new keys works correctly (old `result` → `total`, missing `id` auto-generated)
   - Data-migration Playwright tests pass (6/6)
   - However, because old keys are not read, real-world users with legacy data experience data loss

### Recommended Fixes (before re-audit)

1. **Revert `src/lib/open5e/client.ts`** or document explicit approval for the URL bugfix
2. **Add key fallback** in DiceRoller and InitiativeTracker:
   - On first load, check new key; if empty, check old key and migrate
   - DiceRoller: fallback `"dice-roller-history"` → migrate to `"dm-dice-history"`
   - InitiativeTracker: fallback `"it-combatants"` → migrate to `"dm-initiative"`
3. Re-run data-migration tests with old-key pre-population