diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 1fdffbdd..87099f25 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -22,16 +22,16 @@ ### Watchlist View -- [ ] **WLVW-01**: User can search the watchlist by artist name -- [ ] **WLVW-02**: User can sort by name (A–Z/Z–A) or date added (newest/oldest), with a stable tie-break -- [ ] **WLVW-03**: User can sort by latest release (the artist's newest own `new_release` event date that is not upcoming); artists with none sort last in both directions -- [ ] **WLVW-04**: User can filter the watchlist by one or more tags -- [ ] **WLVW-05**: User can filter to artists with muted event types or non-default release-type filters -- [ ] **WLVW-06**: Search, sort, and filters combine; the view shows "N of M artists" and an empty state with a clear-filters action +- [x] **WLVW-01**: User can search the watchlist by artist name +- [x] **WLVW-02**: User can sort by name (A–Z/Z–A) or date added (newest/oldest), with a stable tie-break +- [x] **WLVW-03**: User can sort by latest release (the artist's newest own `new_release` event date that is not upcoming); artists with none sort last in both directions +- [x] **WLVW-04**: User can filter the watchlist by one or more tags +- [x] **WLVW-05**: User can filter to artists with muted event types or non-default release-type filters +- [x] **WLVW-06**: Search, sort, and filters combine; the view shows "N of M artists" and an empty state with a clear-filters action ### History -- [ ] **HIST-02**: User can filter the History feed by tag, combined with existing filters and respecting event retention +- [x] **HIST-02**: User can filter the History feed by tag, combined with existing filters and respecting event retention ### Bulk Edit @@ -89,13 +89,13 @@ Which phases cover which requirements. Updated during roadmap creation. | TAG-06 | Phase 24 | Complete | | TAG-07 | Phase 24 | Complete | | NOTE-01 | Phase 24 | Complete | -| WLVW-01 | Phase 25 | Pending | -| WLVW-02 | Phase 25 | Pending | -| WLVW-03 | Phase 25 | Pending | -| WLVW-04 | Phase 25 | Pending | -| WLVW-05 | Phase 25 | Pending | -| WLVW-06 | Phase 25 | Pending | -| HIST-02 | Phase 25 | Pending | +| WLVW-01 | Phase 25 | Complete | +| WLVW-02 | Phase 25 | Complete | +| WLVW-03 | Phase 25 | Complete | +| WLVW-04 | Phase 25 | Complete | +| WLVW-05 | Phase 25 | Complete | +| WLVW-06 | Phase 25 | Complete | +| HIST-02 | Phase 25 | Complete | | BULK-01 | Phase 26 | Pending | | BULK-02 | Phase 26 | Pending | | BULK-03 | Phase 26 | Pending | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 4b4f1c35..f28dc47f 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -88,7 +88,7 @@ Full phase-by-phase detail for every shipped milestone is archived under `.plann **Milestone Goal:** Keep a 50+ artist watchlist manageable. The user can label, annotate, find, bulk-change, and bulk-add artists, with no new background API polling. - [x] **Phase 24: Artist Tags & Notes** - Free-form, case-insensitive tags with autocomplete and a plain-text note on each Watchlist card, global tag rename/merge/delete, and tags that belong to the artist so they survive a remove and re-add (completed 2026-10-06) -- [ ] **Phase 25: Find & Filter — Watchlist and History** - Client-side name search, stable sorts (including latest release), and tag/preference filters over the fully loaded watchlist, plus a retention-aware tag filter on the History feed +- [x] **Phase 25: Find & Filter — Watchlist and History** - Client-side name search, stable sorts (including latest release), and tag/preference filters over the fully loaded watchlist, plus a retention-aware tag filter on the History feed (completed 2026-10-12) - [ ] **Phase 26: Bulk Edit & Remove** - Multi-select on the Watchlist with one-action tag add/remove, preference changes, and a count-confirmed all-or-nothing remove, backed by set-based transactional endpoints - [ ] **Phase 27: Paste-a-List Bulk Add** - Paste names one per line, watch them resolve one search at a time within the existing rate limits, review best matches by confidence, and add the confirmed set in one request - [ ] **Phase 28: Tags on Discord Notifications** - Tags on real-time embeds and digest lines, with markdown escaping added to the real-time path and the digest's chunking guarantees re-proven against longer lines @@ -193,36 +193,71 @@ Plans: 4. The user can filter History by a tag alongside the existing filters. It lists events for artists carrying that tag, including artists since removed from the watchlist, and it never shows an event older than the retention window, either on the first page or via "load older". 5. Opening the Watchlist makes one request regardless of watchlist size. Searching, sorting, and filtering then happen instantly with no further requests. -**Plans:** 10 plans +**Plans:** 21/21 plans complete Plans: **Wave 1** -- [ ] 25-01-PLAN.md — GET /watchlist gains `latest_release_date` and `next_release_date` from two lateral joins in its single query (G1–G3), with semantics and projection-parity tests -- [ ] 25-03-PLAN.md — Cards show `Latest release: {date}` / `No releases yet` and `Upcoming: {date}`, `formatReleaseDate`, and the `makeEntry` fixture builder +- [x] 25-01-PLAN.md — GET /watchlist gains `latest_release_date` and `next_release_date` from two lateral joins in its single query (G1–G3), with semantics and projection-parity tests +- [x] 25-03-PLAN.md — Cards show `Latest release: {date}` / `No releases yet` and `Upcoming: {date}`, `formatReleaseDate`, and the `makeEntry` fixture builder **Wave 2** *(blocked on Wave 1 completion)* -- [ ] 25-02-PLAN.md — GET /events `tag_id` filter inside the retention-aware ListEvents and HasOlderEvents, with retention, paging, and validation tests -- [ ] 25-04-PLAN.md — Watchlist name filter (accent and `$ø æ ß` folding), "N of M artists", URL state helper, Clear filters and filtered-to-zero state, live announcements, and sticky cards (G4) -- [ ] 25-06-PLAN.md — History Tag control (D-11), `Combobox` hardening, loading/unavailable labels, and the tag empty state with `Show all tags` +- [x] 25-02-PLAN.md — GET /events `tag_id` filter inside the retention-aware ListEvents and HasOlderEvents, with retention, paging, and validation tests +- [x] 25-04-PLAN.md — Watchlist name filter (accent and `$ø æ ß` folding), "N of M artists", URL state helper, Clear filters and filtered-to-zero state, live announcements, and sticky cards (G4) +- [x] 25-06-PLAN.md — History Tag control (D-11), `Combobox` hardening, loading/unavailable labels, and the tag empty state with `Show all tags` **Wave 3** *(blocked on Wave 2 completion)* -- [ ] 25-05-PLAN.md — Sort by name, date added, or latest release with id-stable ties and null-last ordering, URL `sort`, and the sort announcement -- [ ] 25-09-PLAN.md — History filters move to URL params (`artist`, `type`, `tag`), with stale and malformed params dropped once their source loads (G6) +- [x] 25-05-PLAN.md — Sort by name, date added, or latest release with id-stable ties and null-last ordering, URL `sort`, and the sort announcement +- [x] 25-09-PLAN.md — History filters move to URL params (`artist`, `type`, `tag`), with stale and malformed params dropped once their source loads (G6) **Wave 4** *(blocked on Wave 3 completion)* -- [ ] 25-07-PLAN.md — `Tags: any of` multi-select on the vendored base-ui Combobox, payload-derived options, and D-13 consistency (first-load validation, Manage tags delete/merge, Back) +- [x] 25-07-PLAN.md — `Tags: any of` multi-select on the vendored base-ui Combobox, payload-derived options, and D-13 consistency (first-load validation, Manage tags delete/merge, Back) **Wave 5** *(blocked on Wave 4 completion)* -- [ ] 25-08-PLAN.md — `Has muted events` / `Custom release types` toggles with full AND composition, and the card chip label as a filter toggle (D-08) +- [x] 25-08-PLAN.md — `Has muted events` / `Custom release types` toggles with full AND composition, and the card chip label as a filter toggle (D-08) **Wave 6** *(blocked on Wave 5 completion)* -- [ ] 25-10-PLAN.md — Embedded bundle rebuild, the phase-closing Definition of Done, and the blast-radius gate +- [x] 25-10-PLAN.md — Embedded bundle rebuild, the phase-closing Definition of Done, and the blast-radius gate + +**UAT gap closure — Wave 1** *(from 25-UAT.md gaps G-25-4, G-25-7, G-25-9, with the user's 2026-10-11 decisions)* + +- [x] 25-11-PLAN.md — A chip-label click anchors the clicked card. `useScrollAnchor` records the chip's on-screen position and scrolls by the drift after the filtered re-render, clamped by the browser on short pages. Includes jsdom tests with stubbed layout for both directions and the disconnected case (G-25-4) +- [x] 25-12-PLAN.md — History cards show `Detected {date}` from `created_at`, the date retention counts from (D-01 kept), plus an integration test for "detected recently, released long ago" under a tag filter (G-25-7) +- [x] 25-13-PLAN.md — The Deezer search-result button is capped at 40% of the row below lg and truncates its label, and the type line truncates, so the artist name stays readable at 375px and 640px (G-25-9) + +**UAT gap closure — Wave 2** *(blocked on UAT gap closure Wave 1)* + +- [x] 25-14-PLAN.md — Embedded bundle rebuild, the reworded UI-SPEC D-12, UAT tests 4/7/9, and verification checks 4/7, plus the Definition of Done and a blast-radius gate against `461267f` + +**Verification gap closure — Wave 1** *(from 25-VERIFICATION.md gap G-25-4 and 25-REVIEW WR-01..WR-04, with the user's 2026-10-11 gap-planning decisions)* + +- [x] 25-15-PLAN.md — Sticky cards are keyed to the filter and reset during render, so a chip click on a sticky card keeps its anchor and focus (G-25-4, WR-01). Back drops stale stickies in the same commit. The route's own Manage tags merge/delete and canonical rewrites carry the sticky set (WR-03) +- [x] 25-16-PLAN.md — The Deezer hint's `title` and 40% cap move to a wrapper span that receives pointer events, so the truncated pill explains itself on hover (WR-04) + +**Verification gap closure — Wave 2** *(blocked on 25-15)* + +- [x] 25-17-PLAN.md — Fallback scroll anchor: when the clicked card leaves the list, the next and then the previous sibling card stays in place (WR-02) + +**Verification gap closure — Wave 3** *(blocked on 25-15, 25-16, 25-17)* + +- [x] 25-18-PLAN.md — Embedded bundle rebuild with marker counts, the WR-01..WR-04 decisions recorded in three UI-SPEC lines, the Definition of Done, and a blast-radius gate against `8ccde67` + +**UAT gap closure (round 3) — Wave 1** *(from 25-UAT.md gaps G-25-11, G-25-12, G-25-13, with the user's 2026-10-12 decisions; waves are sequential because each plan edits its own 25-UI-SPEC line)* + +- [x] 25-19-PLAN.md — When the clicked card leaves, the scroll anchor falls back to the nearest surviving card in either direction (next before previous at each distance), so a run of leaving cards never collapses the page. Includes hook and route run-case tests and the widened UI-SPEC D-12 rule (G-25-11) + +**UAT gap closure (round 3) — Wave 2** *(blocked on 25-19)* + +- [x] 25-20-PLAN.md — A keyboard toggle that removes its card moves focus to the fallback card's pressed chip label, or to the `Your artists` heading when nothing survives. Uses `useEffectEvent` in `useScrollAnchor`, adds `data-chip-label`, and adds the "Toggled, card leaves" focus row (G-25-12) + +**UAT gap closure (round 3) — Wave 3** *(blocked on 25-19, 25-20)* + +- [x] 25-21-PLAN.md — The disabled Deezer button gets its hint as an accessible description (sr-only span, per-row `useId`, `aria-describedby`) (G-25-13). Then the embedded bundle is rebuilt with marker counts, and the Definition of Done and a blast-radius gate run against `9d3413f` **UI hint**: yes @@ -265,7 +300,7 @@ Plans: ### Phase 27: Paste-a-List Bulk Add **Goal**: The user can paste a list of artist names and add the right artists in one reviewed batch. They see what each line matched and can fix or skip the doubtful ones, and nothing is added until they confirm. -**Depends on**: Phase 26 (the confirm step reuses its bulk-endpoint conventions: protected route group, per-item outcomes, refresh without reload) +**Depends on**: Phase 26 (the confirm step reuses its bulk-endpoint conventions: protected route group, bounded id lists, refresh without reload. It may reuse Phase 26's per-id outcome shape, but it isn't committed to: Phase 27 confirms by external id, and its skip reasons differ) **Requirements**: IMPT-01, IMPT-02, IMPT-03, IMPT-04, IMPT-05 **Success Criteria** (what must be TRUE): @@ -327,7 +362,7 @@ Phases execute in numeric order: 24 → 25 → 26 → 27 → 28 | Phase | Plans Complete | Status | Completed | |-------|----------------|--------|-----------| | 24. Artist Tags & Notes | 14/14 | Complete | 2026-10-06 | -| 25. Find & Filter — Watchlist and History | 0/? | Not started | - | +| 25. Find & Filter — Watchlist and History | 21/21 | Complete | 2026-10-12 | | 26. Bulk Edit & Remove | 0/? | Not started | - | | 27. Paste-a-List Bulk Add | 0/? | Not started | - | | 28. Tags on Discord Notifications | 0/? | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 0e166334..c555a4c7 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -2,20 +2,20 @@ gsd_state_version: "1.0" milestone: v1.6 milestone_name: Watchlist Organization -current_phase: 25 -current_phase_name: Find & Filter — Watchlist and History +current_phase: 26 +current_phase_name: Bulk Edit & Remove status: planning -stopped_at: Phase 24 complete, ready to plan Phase 25 -last_updated: "2026-10-06T21:40:10.711Z" -last_activity: 2026-10-06 -last_activity_desc: Phase 24 complete, transitioned to Phase 25 -state_head: 43f00c104424775675eecba8545aeb3045255c0f +stopped_at: Phase 25 complete, ready to plan Phase 26 +last_updated: "2026-10-13T02:17:52.713Z" +last_activity: 2026-10-12 +last_activity_desc: Phase 25 complete, transitioned to Phase 26 +state_head: aa235f1e00be1b78be6cbea284f996b550c2fd43 progress: total_phases: 5 - completed_phases: 1 - total_plans: 24 - completed_plans: 14 - percent: 20 + completed_phases: 2 + total_plans: 35 + completed_plans: 35 + percent: 40 --- # Project State @@ -29,16 +29,16 @@ See: .planning/PROJECT.md (updated 2026-10-06) ## Current Position -Phase: 25 — Find & Filter — Watchlist and History +Phase: 26 — Bulk Edit & Remove Plan: Not started Status: Ready to plan -Last activity: 2026-10-07 - Completed quick task 261007-lc4: Phase 24 comment cleanup +Last activity: 2026-10-12 — Phase 25 complete, transitioned to Phase 26 ## Performance Metrics **Velocity:** -- Total plans completed: 113 +- Total plans completed: 134 - Average duration: - min - Total execution time: 0 hours @@ -70,6 +70,7 @@ Last activity: 2026-10-07 - Completed quick task 261007-lc4: Phase 24 comment cl | 22 | 4 | - | - | | 23 | 4 | - | - | | 24 | 14 | - | - | +| 25 | 21 | - | - | **Recent Trend:** @@ -162,6 +163,27 @@ Last activity: 2026-10-07 - Completed quick task 261007-lc4: Phase 24 comment cl | Phase 24 P12 | 6 min | 3 tasks | 4 files | | Phase 24 P13 | 7 min | 2 tasks | 4 files | | Phase 24 P14 | 20 min | 3 tasks | 5 files | +| Phase 25 P01 | 25 min | 2 tasks | 6 files | +| Phase 25 P03 | 8 min | 2 tasks | 14 files | +| Phase 25 P02 | 25min | 2 tasks | 5 files | +| Phase 25 P04 | 18 min | 3 tasks | 9 files | +| Phase 25 P06 | 14 min | 3 tasks | 8 files | +| Phase 25 P05 | 5 min | 2 tasks | 6 files | +| Phase 25 P09 | 4 min | 2 tasks | 6 files | +| Phase 25 P07 | 40 min | 3 tasks | 9 files | +| Phase 25 P08 | 25 min | 2 tasks | 8 files | +| Phase 25 P10 | 15min | 2 tasks | 1 files | +| Phase 25 P11 | 5 min | 2 tasks | 6 files | +| Phase 25 P12 | 6 min | 2 tasks | 5 files | +| Phase 25 P13 | 5 min | 2 tasks | 2 files | +| Phase 25 P14 | 4 min | 3 tasks | 17 files | +| Phase 25 P15 | 8 min | 2 tasks | 4 files | +| Phase 25 P16 | 4 min | 1 tasks | 2 files | +| Phase 25 P17 | 5 min | 2 tasks | 3 files | +| Phase 25 P18 | 5 min | 3 tasks | 3 files | +| Phase 25 P19 | 2 min | 2 tasks | 4 files | +| Phase 25 P20 | 10 min | 3 tasks | 7 files | +| Phase 25 P21 | 9 min | 3 tasks | 4 files | ## Accumulated Context @@ -389,6 +411,24 @@ Recent decisions affecting current work: - [Phase 24]: 24-12: ManageTagsDialog guards load() with a generation ref; invalidateLoad() on delete/rename/merge success bumps it and refetches only when a load was in flight (no skeleton strand) - [Phase 24]: 24-13: wrap-anywhere on ConfirmDialog title/description (not break-word) so unspaced 32-char tag names wrap in the centered header grid (G-24-5) - [Phase 24]: 24-14: overflow latch via dependency-plus-early-return in ArtistNote; Manage tags hand-off consumed only by a successful load (G-24-9..11) +- [Phase 25]: 25-01: ListWatchlist keeps zero-arg signature; latest/next release dates need no retention setting (G1) +- [Phase 25]: Release dates render via string-only formatReleaseDate; malformed values pass through as plain text — No Date construction avoids timezone shifts; server decides upcoming (G2/G3) +- [Phase 25]: 25-04: Watchlist q filters from local state per keystroke, URL write debounced 300ms with replace; input resyncs only when URL differs from last own write — Avoids history flooding and echo loops (Pitfall 6) +- [Phase 25]: 25-04: refresh() resolves with loaded rows so the 409 add path can mark the existing artist sticky; sticky set cleared only by search change or Clear filters — G4: add/Undo never succeed invisibly under a filter +- [Phase 25]: 25-06: History Tag degraded trigger labels apply only when a tag is selected — With no selection a failed GET /tags still reads All tags; matches the plan behavior block +- [Phase 25]: 25-05: Latest-release sort compares raw text (2024 before 2024-05), nulls last in both directions, artist_id ascending tie-break in every mode +- [Phase 25]: 25-09: History stale artist/tag ids are checked once per source per mount, only after a successful load; a failed fetch keeps the filter and the trigger reads Artist unavailable / Tag unavailable — A failed fetch is not proof an id is stale (UI-SPEC History Tag filter) +- [Phase 25]: 25-07: Esc on a closed tag-filter popup is cancelled (escape-key reason) so it cannot clear every chip; tags URL param serializes commas as %2C +- [Phase 25]: 25-08: Custom release types is a set comparison against the default four; chip label click reuses handleTagsChange and the x remains the only detach +- [Phase 25]: 25-11: only the chip-label path anchors scroll (useScrollAnchor); no expiry on unconsumed anchor — Toolbar toggles have no card to keep; a timer or same-key clear would race the router commit +- [Phase 25]: 25-12: History cards show a per-card Detected date (created_at); retention stays on created_at (D-01), no SQL change +- [Phase 25]: 25-15: sticky set keyed to filterKey and reset during render (not a derived key compare, which revives cards when a filter is turned off); route-own tag rewrites carry via a single-landing, replace-only carry +- [Phase 25]: 25-16: Deezer hint title and 40% cap live on a wrapper span (a disabled button gets no pointer events); Button is w-full inside it (WR-04) +- [Phase 25]: 25-17: useScrollAnchor falls back to the clicked card's next, then previous, sibling when the chip leaves the DOM (WR-02, user decision 2026-10-11); list heading excluded +- [Phase 25]: 25-18: WR-01..WR-04 decisions recorded in UI-SPEC; CONTEXT G4 amended for WR-03 (user decision 2026-10-11) +- [Phase 25]: [Phase 25-19]: G-25-11 scroll anchor fallback widened to the nearest surviving card (no distance cap, next before previous at each distance), user decision 2026-10-12 +- [Phase 25]: G-25-12 (2026-10-12): keyboard-only focus move (click detail 0) to fallback card's chip label, else Your artists heading, via useScrollAnchor onFocusLost/useEffectEvent +- [Phase 25]: G-25-13 (user decision 2026-10-12): disabled Deezer button described via aria-describedby to a per-row useId sr-only hint; no visual change ### Pending Todos @@ -447,6 +487,8 @@ _Closed 2026-09-05: Phase 16 gap G-16-1 (n1-boot guard-adoption skip) — quick | 261007-ka1 | Tag and note error contract: services are the single validation point, coded error envelope, single limits module, code-point length checks (Phase 24 review candidates 2+3) | 2026-10-07 | 71d3bb1 | [261007-ka1-tag-and-note-error-contract-single-valid](./quick/261007-ka1-tag-and-note-error-contract-single-valid/) | | 261007-kt1 | Single owner for the tag vocabulary on the web: useTagVocabulary provider with one stale-response rule, fixes rememberTag race (Phase 24 review candidate 1) | 2026-10-07 | 7455c40 | [261007-kt1-single-owner-for-the-tag-vocabulary-on-t](./quick/261007-kt1-single-owner-for-the-tag-vocabulary-on-t/) | | 261007-lc4 | Phase 24 comment cleanup: comment-only trim to 1-3 line why-notes with one doc ref, fixed inaccurate WithTags/merge-trigger/buildTagSuggestions comments, removed plan narration | 2026-10-07 | be98a62 | [261007-lc4-phase-24-comment-cleanup](./quick/261007-lc4-phase-24-comment-cleanup/) | +| 261011-gq7 | Amend Phase 26 CONTEXT and UI-SPEC with grill decisions Q1–Q10 (derived selection, outcome-only bulk responses + refresh, ordered-lock bulk tag attach, single-statement remove/preferences, pending-alert disclosure) and add withdraw-pending-events todo | 2026-10-11 | 6408d28 | [261011-gq7-amend-phase-26-docs-with-grill-decisions](./quick/261011-gq7-amend-phase-26-docs-with-grill-decisions/) | +| 261011-moj | Amend Phase 26 CONTEXT and UI-SPEC with UI review U1–U4 (route-level confirm + close→clear→focus order, scroll-padding focus clearance, one-row phone bar, remove-tag Undo) | 2026-10-11 | 9ac478c | [261011-moj-amend-phase-26-ui-spec-and-context-with-](./quick/261011-moj-amend-phase-26-ui-spec-and-context-with-/) | ### Roadmap Evolution @@ -482,8 +524,8 @@ Items acknowledged and carried forward from previous milestone close: ## Session Continuity -Last session: 2026-10-06T21:45:00Z -Stopped at: Phase 24 complete, ready to plan Phase 25 +Last session: 2026-10-12T22:25:32.998Z +Stopped at: Phase 25 complete, ready to plan Phase 26 Resume file: None ## Operator Next Steps diff --git a/.planning/debug/chip-click-scroll-reset.md b/.planning/debug/chip-click-scroll-reset.md new file mode 100644 index 00000000..4b936bab --- /dev/null +++ b/.planning/debug/chip-click-scroll-reset.md @@ -0,0 +1,105 @@ +--- +status: diagnosed +trigger: "UAT G-25-4: Clicking a chip label far down a 50+ artist watchlist resets window.scrollY to 0" +created: 2026-10-11T00:00:00Z +updated: 2026-10-11T00:45:00Z +goal: find_root_cause_only +--- + +## Current Focus + +bug_class: Bohrbug (deterministic: same data + viewport -> same scrollY, reproduced on every run) +hypothesis: CONFIRMED (H2). No code scrolls the page. A chip click applies a tag filter that collapses the list, and the browser clamps scrollTop to the new, much shorter document. +next_action: none. Diagnose-only; return ROOT CAUSE FOUND to the orchestrator. +known_pattern_candidate: none (KB has no scroll/navigation entries) + +reasoning_checkpoint: + hypothesis: "scrollY drops to 0 because toggleFilterTag -> handleTagsChange filters `visible` down to the few cards carrying the tag. The document's scrollHeight falls to <= innerHeight (body is min-h-screen, so it floors at exactly innerHeight), and the browser clamps scrollTop to max(0, scrollHeight - innerHeight) = 0. React Router does not reset it: preventScrollReset is honored." + confirming_evidence: + - "CDP S1 (60 synthetic artists, chip on 2 cards): scrollY 8662 -> 0, scrollHeight 10021 -> 900 = innerHeight, ZERO window.scrollTo/scroll/scrollBy/scrollIntoView/focus calls" + - "CDP S3 (same click, body min-height 20000px): scrollY 8662 -> 183. Changing only the document height removes the drop to 0, with zero programmatic calls" + - "CDP S2 (chip on 40 cards): scrollY 8823 -> 5961 = exactly the new maxScroll. Clamped, not reset" + - "CDP R1 (user's REAL 12-artist data, dembow chip): scrollY 1317 -> 0, scrollHeight 2435 -> 900, cards 12 -> 1, zero programmatic calls" + falsification_test: "If RR or app code were resetting scroll, S3 would also end at 0 and the spy would log a window.scrollTo(0,0). Neither happened." + fix_rationale: "(diagnose-only) The fix must address the list collapse, not the router: preventScrollReset is already correct and sufficient for what it covers." + blind_spots: "Only Chromium tested (Chrome headless). Safari/Firefox clamp scrollTop the same way when content shrinks (no layout engine can scroll past content), but their scroll-anchoring behavior differs, which would only affect the partial-shrink case (S2/S3), not the drop to 0. The user's 50+ list was not available locally (local DB has 12 artists), so 60 synthetic rows stood in for it." + candidate_causes: + - "code: RR scrollTo(0,0) despite preventScrollReset. ELIMINATED (no scrollTo call; S3 non-zero)" + - "code: programmatic focus()/scrollIntoView in TagChips/route effects. ELIMINATED (spies logged nothing)" + - "code/design: filter collapses the list -> document shrinks below scrollY -> browser clamp. CONFIRMED" + - "environment: browser scroll anchoring (overflow-anchor). ELIMINATED as cause (S5 with anchoring off still 0); it actually mitigates partial shrinks (S3)" + - "data: each local tag is on exactly one artist, so any chip filter yields 1 card that fits in the viewport. CONTRIBUTING (decides 0 vs a partial drop)" + - "spec: UI-SPEC D-12 / UAT truth models ScrollRestoration as the only scroll-to-top cause and treats preventScrollReset as sufficient. CONTRIBUTING design gap" + and_gate: "yes, weakly. A drop to exactly 0 needs (1) no mechanism that keeps the clicked card's position when the list collapses (design gap) AND (2) a filtered result short enough to fit in the viewport (data). Without (2), the same mechanism gives a partial jump to the new bottom (S2: -2862px). (1) is the actionable root cause." + +## Symptoms + +expected: window.scrollY is preserved (not reset to 0) when clicking a tag chip label (button aria-label "Filter by {tag}" in web/app/components/watchlist/TagChips.tsx) on an artist card far down the Watchlist. +actual: User reported "it drops to 0 after clicking a tag" (window.scrollY becomes 0 after clicking a chip label). The `Has muted events` toggle was NOT reported as failing. +errors: none +reproduction: UAT test 4 in .planning/phases/25-find-filter-watchlist-and-history/25-UAT.md — scroll far down a 50+ artist watchlist, click a chip label. +started: discovered during phase 25 UAT + +## Eliminated + +- hypothesis: "H1: React Router calls window.scrollTo(0,0) on the chip-click navigation (preventScrollReset missing or dropped)" + evidence: "useUrlParams.write always passes preventScrollReset: true (shared by chip and muted paths); RR 7.18.2 setSearchParams forwards it verbatim and useScrollRestoration skips scrollTo(0,0) when it is true. At runtime the window.scrollTo spy logged zero calls in S1-S5/R1, and S3 (taller document, same navigation) ended at scrollY 183, not 0." + timestamp: 2026-10-11T00:30:00Z + +- hypothesis: "H3: a programmatic focus() or scrollIntoView() (TagChips focus effects, route effects) moves the viewport" + evidence: "HTMLElement.focus, Element.scrollIntoView and Element.scrollTo spies logged nothing during any click. The chip receives focus natively from the mouse click, which did not scroll (S3 stayed at 183)." + timestamp: 2026-10-11T00:30:00Z + +- hypothesis: "Browser scroll anchoring (overflow-anchor: auto) pulls scrollY to 0" + evidence: "S5 with overflow-anchor:none still went 8662 -> 0. With a tall document (S3), anchoring kept the clicked chip near its viewport position, which helps rather than hurts." + timestamp: 2026-10-11T00:30:00Z + +## Evidence + +- timestamp: 2026-10-11T00:00:00Z + checked: .planning/debug/knowledge-base.md + found: no entries about scroll, navigation, React Router, or URL params + implication: no known-pattern candidate; open investigation + +- timestamp: 2026-10-11T00:05:00Z + checked: web/app/components/watchlist/TagChips.tsx:188-193, web/app/routes/watchlist.tsx:439-462, web/app/lib/useUrlParams.ts + found: chip label onClick -> actions.toggleFilterTag(tag) -> handleTagsChange(ids) -> write(mutate) (push, no replace). The muted toggle -> handleFlagChange -> write(mutate) (push). useUrlParams.write ALWAYS calls setParams(next, { replace, preventScrollReset: true }). + implication: the chip path and the (not-failing) muted path share the exact same URL writer with preventScrollReset: true. A "missing preventScrollReset" hypothesis is unlikely unless RR ignores the flag; the difference must be elsewhere (e.g. what the filter does to page height). + +- timestamp: 2026-10-11T00:10:00Z + checked: web/node_modules/react-router/dist/development/chunk-62JRHF6Z.mjs (RR 7.18.2) useSearchParams (L10830-10862) and useScrollRestoration (L11036-11126); web/app/root.tsx ( in Layout); web/react-router.config.ts (ssr:false framework/SPA mode) + found: setSearchParams forwards navigateOptions verbatim to navigate("?"+params, opts). ScrollRestoration's layout effect only calls window.scrollTo(0,0) when restoreScrollPosition is not a number, there is no hash target, AND preventScrollReset !== true. WatchlistRow/TagChips have no /
wrapper; chip button is type="button". + implication: by code reading, RR should not be the one scrolling to 0 on a chip click. Needs a runtime check (spy on window.scrollTo) to eliminate formally. + +- timestamp: 2026-10-11T00:15:00Z + checked: 25-UI-SPEC.md:399 ("Scroll reset (D-12)") and :639, 25-RESEARCH.md Pitfall 7, 25-08-PLAN human-check; web/app/components/watchlist/WatchlistToolbar.tsx + found: the spec's only modelled cause of "jump to top" is RR ScrollRestoration; its only mitigation is preventScrollReset. Nothing in spec/plan/research addresses the list SHRINKING under the viewport when a filter is applied. The toolbar (incl. `Has muted events`) is a static, non-sticky flex group ABOVE the list, so it can only be clicked when scrolled near the top. + implication: (a) a design gap: filtering a 50+ list down to a few cards makes the document shorter than the current scrollY, and the browser must clamp scrollTop to max(0, scrollHeight - innerHeight); (b) explains why the muted toggle "did not fail": it is only reachable at scrollY near 0, so there is nothing to lose. + +- timestamp: 2026-10-11T00:30:00Z + checked: headless Chrome (CDP) against the running build at localhost:8080, viewport 1280x900, GET /watchlist intercepted with 60 synthetic artists (tag "rare" on cards 3+55, "common" on 40 cards, 2 muted). Spies on window.scrollTo/scroll/scrollBy, Element.scrollIntoView/scrollTo, HTMLElement.focus plus scroll events. Real mouse clicks via Input.dispatchMouseEvent. Script: scratchpad/cdp-scroll.mjs + found: | + S1 rare chip on card 55: scrollY 8662 -> 0; scrollHeight 10021 -> 900 (= innerHeight, body is min-h-screen); cards 60 -> 2; URL ?tags=901; programmatic scroll/focus calls: NONE; one scroll event at y=0. + S2 common chip on card 56: scrollY 8823 -> 5961 = new maxScroll exactly (scrollHeight 10021 -> 6861); cards 60 -> 40; calls: NONE. Clamped to the new bottom, NOT reset to 0. + S3 = S1 + body min-height 20000px: scrollY 8662 -> 183 (not 0); calls: NONE. With a tall document Chrome scroll-anchoring keeps the clicked chip near its viewport position. + S5 = S1 + overflow-anchor:none: still 8662 -> 0; calls: NONE. Anchoring is not the cause; the height clamp is. + S4 `Has muted events` at the deepest scroll where it is still visible: scrollY 285 -> 0 (cards 60 -> 2); calls: NONE. Same clamp, but from a tiny offset because the toolbar is not sticky. + history.scrollRestoration = "manual" in all runs (ScrollRestoration mounted and active). + implication: H2 CONFIRMED and H1/H3 eliminated. preventScrollReset works and nothing calls scrollTo or focus. scrollY goes to 0 because filtering shrinks the document to <= innerHeight and the browser clamps scrollTop. Changing ONLY the document height (S3) turns the same click and navigation into a non-zero scrollY. + +- timestamp: 2026-10-11T00:38:00Z + checked: same CDP harness, NO interception, the user's real data from localhost:8080/watchlist (12 artists; tags Hip-Hop, Melodic Rap, dembow each on exactly 1 artist; 0 muted). Clicked "Filter by dembow" on Sech's card scrolled into view. URL-only change in a throwaway headless profile; no API writes. + found: scrollY 1317 -> 0; scrollHeight 2435 -> 900; cards 12 -> 1; URL ?tags=81; programmatic scroll/focus calls NONE. + implication: reproduces the exact UAT report on real data. With every tag on one artist, any chip click leaves one card, which fits in any desktop viewport, so the drop to 0 happens every time. + +- timestamp: 2026-10-11T00:40:00Z + checked: grep -i scroll in web/app/lib/useUrlParams.test.tsx, web/app/routes/watchlist.test.tsx, web/app/components/watchlist/TagChips.test.tsx + found: the only scroll assertion is useUrlParams.test.tsx:10 "always prevents scroll reset and defaults to push", which pins the OPTION value (preventScrollReset: true), not a scroll outcome. + implication: why not caught. The guard verifies the router contract only; jsdom has no layout, so the collapse-and-clamp class is invisible to the suite and was left to UAT. + +## Resolution + +root_cause: "Filtering collapses the watchlist under the viewport, and nothing keeps the clicked card's position. A chip click (TagChips.tsx:193 -> watchlist.tsx toggleFilterTag/handleTagsChange) narrows `visible` from 50+ cards to the few carrying that tag. The document's scrollHeight drops to <= innerHeight (body is min-h-screen, so it floors at exactly innerHeight), and the browser clamps scrollTop to max(0, scrollHeight - innerHeight) = 0. React Router is NOT resetting scroll: preventScrollReset: true (useUrlParams.ts) is honored and no code calls scrollTo/scrollIntoView/focus. The design (UI-SPEC D-12, line 399) modelled as the only cause of a jump to the top, so preventScrollReset was the whole mitigation and the UAT truth assumed it was sufficient. `Has muted events` uses the same writer and clamps the same way (S4), but the toolbar is not sticky and only reachable near scrollY 0, so there is nothing visible to lose." +fix: (diagnose-only, not applied) +verification: (diagnose-only) +files_changed: [] diff --git a/.planning/debug/history-load-more-retention-leak.md b/.planning/debug/history-load-more-retention-leak.md new file mode 100644 index 00000000..d0f37cfa --- /dev/null +++ b/.planning/debug/history-load-more-retention-leak.md @@ -0,0 +1,125 @@ +--- +status: diagnosed +trigger: "Phase 25 UAT test 7 (G-25-7): using load more shows events older than the retention edge, rest pass" +created: 2026-10-11T15:00:00Z +updated: 2026-10-11T15:30:00Z +goal: find_root_cause_only +--- + +## Current Focus + +hypothesis: CONFIRMED. The retention filter works exactly as coded. "Older" events appear because retention and display/order use different date axes. Retention is keyed on created_at (detection time, Phase 10 D-01). The feed is ordered by and displays only release_date (quick task 260825-g6i). Seed-mode backfill gives years-old releases a fresh created_at. Load more walks back through release chronology into those back-catalogue rows, all of which are inside the created_at window. +bug_class: Bohrbug (deterministic; reproducible via API on every walk). Semantic/contract mismatch, not a missing predicate. +known_pattern_candidate: none (KB entries don't match) +next_action: return ROOT CAUSE FOUND to orchestrator (find_root_cause_only; no fix applied) + +reasoning_checkpoint: + hypothesis: "Load more surfaces events the user reads as 'older than the retention window' because ListEvents filters on created_at >= now()-90d, but orders by release_date DESC and EventCard shows only release_date. Seed-mode backfill rows carry years-old release_date with a recent created_at, so they pass the cutoff and fill every page past ~position 2-7, getting older with each Load more." + confirming_evidence: + - "queries/events.sql ListEvents: `AND created_at >= cutoff` is a top-level AND outside the cursor CASE, so it applies on page 1 and every cursor page and with tag_id alike. Generated SQL in internal/db/sqlc/events.sql.go is identical." + - "Live API walk (GET /events?tag_id=81 and tag_id=12, following next_cursor to exhaustion): 0 of 155 / 0 of 425 rows have created_at < cutoff, but 153/155 and 421/425 have release_date < 2026-07-13, reaching 2015-04-08 / 2014-06-21. Each page steps years further back (tag 12 p1 2025-05..2026-09, p2 2024-03..2025-05, p3 2023-05..2024-03, p18 2014..2016)." + - "Read-only DB: 4462 events, 0 outside the created_at window (min created_at 2026-08-23, 49 days ago; cutoff 2026-07-13); 3905 rows have release_date older than the cutoff date. Tag 81's rows were all created 2026-10-11 (seeded today) with release dates back to 2015." + - "Phase 10 10-CONTEXT D-01 explicitly chose created_at over release_date and records the consequence as 'accepted, expected behavior, not a bug'. D-01 was written when ListEvents ordered by id DESC (a created_at proxy). 260825-g6i (2026-08-25) switched ordering to release_date DESC and left retention untouched on purpose." + - "EventCard.tsx renders only `event.release_date ?? 'Release date unknown'`; created_at is never shown, so the user has no way to see the axis retention uses." + falsification_test: "Find any row returned by GET /events (any page, any tag) whose created_at < now()-EVENT_RETENTION_DAYS. One such row would prove a real predicate leak. Exhaustive cursor walks for tags 12 and 81 found none." + fix_rationale: "N/A (diagnose-only). The fix is a contract decision, not a missing WHERE clause. Either (A) keep D-01 and make the UI/UAT honest about the detection-date axis, or (B) reverse D-01 and add release-date-based visibility. Adding a predicate without that decision would re-open D-01's partial/NULL-date and hide-the-whole-seeded-catalogue concerns." + blind_spots: "I can't observe exactly which card the user judged 'older than the retention edge'. The inference that they compared release_date to 90 days rests on: (1) it's the only date on the card, (2) there are zero rows on the far side of the created_at edge in this DB, so no other reading is possible on this data. A small chance remains that the user saw a WR-01 stale-append (previous-tag rows) and called it 'older'. Even then, every request carries the cutoff, so it still couldn't be a created_at leak." + candidate_causes: + - "code: cutoff predicate missing on cursor pages / tag variant (ELIMINATED, see Eliminated)" + - "config/environment: wrong EVENT_RETENTION_DAYS or stale running binary (ELIMINATED, env=90, image built 2026-10-11T14:43Z, tag filter live)" + - "data: seed-mode backfill rows have recent created_at + old release_date (CONFIRMED contributing)" + - "spec/design: retention axis (created_at) differs from order/display axis (release_date); UAT truth doesn't name the axis (CONFIRMED contributing)" + and_gate: "yes. Both are required: (1) retention keyed on created_at with seed backfill stamping old releases as freshly created, AND (2) the feed ordered and labelled by release_date with no detection date shown. With (1) alone and the pre-g6i id-DESC order, Load more walked back in detection time and the feed ended at the retention edge, matching user intuition. With (2) alone and release-date retention, old releases would be hidden. Contributing: the dev DB has no row beyond the created_at edge (oldest 49d), so test 7's 'events on both sides of the retention window' precondition was unsatisfiable under the real contract, and only the release_date reading was observable." + +## Symptoms + +expected: With a History Tag filter active (and generally), paging via Load more never returns events older than the retention window; reload keeps the tag; Back restores the previous combination. +actual: User reported "using load more shows events older than the retention edge, rest pass" +errors: None reported +reproduction: Test 7 in .planning/phases/25-find-filter-watchlist-and-history/25-UAT.md +started: Discovered during phase 25 UAT + +## Eliminated + +- hypothesis: The retention cutoff is applied on the first page only; cursor (Load more) pages skip it. + evidence: queries/events.sql places `AND created_at >= sqlc.arg('cutoff')` as a top-level AND, outside the `cursor_id IS NULL OR CASE ...` group. Generated internal/db/sqlc/events.sql.go matches. Live cursor walks to exhaustion for tags 12 and 81 returned 0 rows with created_at < cutoff across 25 pages. + timestamp: 2026-10-11T15:20:00Z + +- hypothesis: The tag-filtered query variant lacks the cutoff (separate query path). + evidence: There is one static ListEvents SQL; tag_id is an optional sqlc.narg EXISTS inside it, so it cannot bypass the cutoff. Live walks used tag_id and found 0 aged rows. + timestamp: 2026-10-11T15:20:00Z + +- hypothesis: The frontend Load more drops the filters or bypasses retention. + evidence: history.tsx handleLoadMore calls the same fetchHistoryPage(filters, nextCursor) as page 1 (artistId/eventType/tagId + cursor). Retention is server-side and computed per request in events.Service.List, so the client cannot opt out. + timestamp: 2026-10-11T15:20:00Z + +- hypothesis: Environment misconfiguration (EVENT_RETENTION_DAYS unset/huge) or a stale running binary. + evidence: The app container env has EVENT_RETENTION_DAYS=90. The image was built 2026-10-11T14:43Z (post-phase-25). config.Load rejects <=0. The tag filter works live, so the phase-25 binary is running. + timestamp: 2026-10-11T15:20:00Z + +- hypothesis: sqlc drift (generated code lacks the cutoff that the .sql has). + evidence: The generated listEvents constant contains `AND created_at >= $6::timestamptz`, identical to the source. + timestamp: 2026-10-11T15:20:00Z + +- hypothesis: WR-01 stale Load-more append leaks out-of-window rows. + evidence: Each request, stale or not, carries the server cutoff, and 0 rows in the DB are beyond the created_at edge anyway. WR-01 can leak previous-filter rows but cannot produce a created_at-aged row. + timestamp: 2026-10-11T15:20:00Z + +## Evidence + +- timestamp: 2026-10-11T15:00:00Z + checked: .planning/debug/knowledge-base.md (Phase 0) + found: Two entries (guest-feature-label-missing, backlog-songs-trigger-discord). Neither concerns history paging/retention filtering. backlog-songs-trigger-discord involves release-date freshness gating (NOTIFY_MAX_RELEASE_AGE_DAYS), a different "age" concept. + implication: No known-pattern candidate; proceed with open investigation. + +- timestamp: 2026-10-11T15:05:00Z + checked: queries/events.sql ListEvents + internal/db/sqlc/events.sql.go (generated) + internal/events/service.go List + found: `AND created_at >= sqlc.arg('cutoff')::timestamptz` is a top-level AND, outside the cursor CASE, so it applies on page 1 and every cursor page, and in the tag/artist/type variants alike (one static query). The generated SQL matches the source (no sqlc drift). Service computes cutoff = time.Now().AddDate(0,0,-retentionDays) with Valid=true on every call. ORDER BY is `release_date DESC NULLS LAST, id DESC`. Retention is keyed on created_at (detection time), but ordering and display use release_date (the release's own date). + implication: The server cannot return a row with created_at < cutoff on any page. Retention axis (created_at) and display/order axis (release_date) differ. + +- timestamp: 2026-10-11T15:08:00Z + checked: web/app/routes/history.tsx fetchHistoryPage/handleLoadMore + found: Load more calls the same fetchHistoryPage(filters, nextCursor), passing artistId/eventType/tagId + the opaque cursor. No client-side date filtering. hasOlderEvents is used only for the empty state; nothing renders a retention-edge marker in a non-empty feed. + implication: Frontend sends the correct filters on Load more. The UI never indicates where the retention edge is or which date it is measured on. + +- timestamp: 2026-10-11T15:10:00Z + checked: contract docs - PROJECT.md L90/L115/L200, config.go, 25-VERIFICATION.md SC4/WR-01, queries/events.sql Phase 10 comment + found: Phase 10 DATA-01/02 defines retention as a read-side soft filter hiding events older than 90 days, measured on created_at. EVENT_RETENTION_DAYS default 90. 25-VERIFICATION SC4 was verified by TestRetention_TagFilterComposesWithCutoff, whose fixtures (insertTestEventAt) set created_at only and leave release_date NULL. UAT test 7 / VERIFICATION human check 7 say "events on both sides of the retention window" and "No event older than the retention window" without naming the date axis. + implication: The implemented contract is detection-age retention. The UAT wording is ambiguous, and the automated test never exercises "recent created_at + old release_date". + +- timestamp: 2026-10-11T15:16:10Z + checked: Running stack - docker app container env + read-only psql against drop-tracker-postgres-1 + found: App has EVENT_RETENTION_DAYS=90 (image built 2026-10-11T14:43Z, current). DB now=2026-10-11 15:16Z, cutoff=2026-07-13. events: total 4462; in-window by created_at 4462; aged-out by created_at 0; min(created_at)=2026-08-23 21:24Z (49 days ago); 3905 rows have release_date < cutoff date; 508 undated. + implication: No row in the dev DB is outside the retention window by its actual definition. has_older_events is false for every scope. Every "older" event the user saw is a back-catalogue release (old release_date) detected in the last 49 days. + +- timestamp: 2026-10-11T15:18:00Z + checked: Per-tag feed simulation (read-only SQL replicating ListEvents order + cutoff) + found: Hip-Hop (12): 425 feed rows, first release_date older than 90d at position 5, 20 such rows on page 1, oldest release 2014-06-21, oldest detected 2026-08-26. Melodic Rap (13): 265 rows, first at position 7, 18 on page 1, oldest 2018-05-27. dembow (81): 155 rows, first at position 2, 23 on page 1, oldest 2015-04-08, all detected 2026-10-11. + implication: Old release dates appear from page 1 onward; Load more makes them dominant and progressively older. That's why the user ties it to Load more. + +- timestamp: 2026-10-11T15:20:00Z + checked: EventCard.tsx date rendering + found: All three body variants render `event.release_date ?? "Release date unknown"`; created_at is never displayed. + implication: The only date a user can judge "older" by is release_date, which isn't the retention axis. + +- timestamp: 2026-10-11T15:22:00Z + checked: Phase 10 10-CONTEXT.md (2026-08-13) D-01 vs quick task 260825-g6i (2026-08-25) + found: D-01 chose created_at deliberately (release_date is partial/nullable free text) and records "a freshly-seeded artist's entire back-catalog is inserted with created_at = now() ... stays visible together for a full window ... accepted, expected behavior, not a bug". D-01 assumed ListEvents sorted by id DESC (a created_at proxy). g6i later switched ordering to release_date DESC NULLS LAST and explicitly did not touch the retention predicate. + implication: Before g6i, Load more walked back in detection time and the feed's tail was the retention edge, so the user's intuition held. After g6i the order axis and retention axis diverged, and Load more walks back in release time through rows retention correctly keeps. + +- timestamp: 2026-10-11T15:25:00Z + checked: Live API - node script walking GET /events?tag_id=N following next_cursor to exhaustion (scratchpad walk.js) + found: tag 81: 7 pages, 155 rows, aged_by_created_at=0, release_date older than 2026-07-13 = 153; p1 release 2024-05..2026-10, p2 2023-02..2024-05, p3 2021-04..2022-10, p7 2015-04..2018-06; has_older_events=false throughout. tag 12: 18 pages, 425 rows, aged_by_created_at=0, older-release=421; p1 2025-05..2026-09, p2 2024-03..2025-05, p18 2014-06..2016-10. + implication: Reproduces the user's observation end to end and proves zero created_at leakage. The symptom is release-date age, not retention violation. + +- timestamp: 2026-10-11T15:27:00Z + checked: Read-only simulation of shorter EVENT_RETENTION_DAYS for UAT re-test feasibility + found: At 45 days: Hip-Hop 55 visible / 370 hidden, Melodic Rap 8 / 257, dembow 156 / 0. At 30 days: Hip-Hop 4 / 421. + implication: Running the dev app with EVENT_RETENTION_DAYS=45 gives the Hip-Hop tag ~3 pages of feed with real rows on both sides of the created_at edge, which makes UAT test 7's precondition satisfiable. + +## Resolution + +root_cause: "Not a missing retention predicate. ListEvents applies `created_at >= cutoff` on every page and filter variant, and live cursor walks return zero out-of-window rows. The symptom is a contract/presentation mismatch that needs two conditions together (AND-gate): (1) retention is measured on created_at, the detection time (Phase 10 D-01), and seed-mode backfill stamps an artist's entire years-old back catalogue with a fresh created_at, so those rows are inside the 90-day window; AND (2) since quick task 260825-g6i the History feed is ordered by release_date DESC and EventCard shows only release_date, never the detection date. Load more therefore pages backward through release chronology into 2014-2018 releases that retention correctly keeps, and the user, who can only see release_date, reads them as 'older than the retention edge'. Contributing: the dev DB has no event beyond the created_at edge (oldest detection is 49 days), so UAT test 7's 'events on both sides of the retention window' precondition was unsatisfiable under the real contract, and the UAT/VERIFICATION wording never states which date retention uses." +fix: "" +verification: "" +files_changed: [] diff --git a/.planning/debug/search-input-squeezed-375.md b/.planning/debug/search-input-squeezed-375.md new file mode 100644 index 00000000..8562f41e --- /dev/null +++ b/.planning/debug/search-input-squeezed-375.md @@ -0,0 +1,108 @@ +--- +status: diagnosed +trigger: "G-25-9 (UAT test 9): at 375px the watchlist search shows only the first letter of the artist name; the 'Search MusicBrainz to add' button does not truncate" +created: 2026-10-11T00:00:00Z +updated: 2026-10-11T00:00:00Z +goal: find_root_cause_only +--- + +## Current Focus + +hypothesis: CONFIRMED. The squeezed text is the Deezer search-result row's artist name, not a search input. In SearchResultRow the name column is `flex-1 min-w-0` (flex-basis 0, so it only gets leftover space), while the trailing "Search MusicBrainz to add" Button has a 205px max-content basis (25-char label, `whitespace-nowrap`) and `shrink-0`, with no width cap. At 375px only 10px is left for the name. +test: done. Headless Chrome against localhost:8080 with /search and /watchlist stubbed and non-GET blocked, plus runtime CSS differential experiments A-G +expecting: n/a +next_action: return ROOT CAUSE FOUND to orchestrator (goal: find_root_cause_only; no fix applied) +bug_class: Bohrbug (deterministic CSS flex layout) +reasoning_checkpoint: + hypothesis: "The Deezer result row's name shows ~1 letter at 375px because the trailing disabled Button keeps its full 205px max-content width (nowrap label + shrink-0, no max-width), and the sibling name column is flex-1 (basis 0) + min-w-0, so it receives only the 10px of leftover space" + confirming_evidence: + - "Measured at 375px: li 311, button 205, name col 10, 0/9 chars before the ellipsis; screenshot shows 'B' and 'K' next to the full pill" + - "Capping the button at 96px (E) gives the name 119px and 9/9 chars; shortening the label (D) gives 74px and 8/9" + - "MusicBrainz row with the shorter 'Add to Watchlist' (135px) button shows 8/9 chars in the same layout" + falsification_test: "If capping or shortening the button did not widen the name column, or the name stayed squeezed with the button removed, the hypothesis would be wrong. Both experiments widened it" + fix_rationale: "n/a (diagnose-only). Direction: constrain the button's width or label so the name column gets the space. Making the button shrinkable alone is insufficient (exp. B)" + blind_spots: "Measured with null image_url (placeholder CoverArt, same 48px box), stub data, and Chrome headless with Inter Variable. Real Deezer names and Safari font metrics differ slightly but cannot cover a ~195px deficit" + candidate_causes: + - "code: Button label length + whitespace-nowrap + shrink-0 with no max-width (SearchResultsColumns.tsx:190-198, button.tsx:7)" + - "code: name column flex-basis 0 (flex-1), so button shrinkability never kicks in (SearchResultsColumns.tsx:174)" + - "config/layout: route-wide p-8 padding at 375px (watchlist.tsx:491), a minor contributor (+32px)" + - "environment: viewport/font metrics; ruled out as primary (deficit is structural, reproduces at 640px too)" + and_gate: "yes. Two code conditions combine: the unconstrained 205px nowrap button AND the 0-basis name column. Exp. B (button shrinkable only) and F (name basis auto only) each change nothing; only G (both) or capping/shortening the button (D/E) helps. Page p-8 padding is an aggravating, non-necessary factor" + +## Symptoms + +expected: Typing an artist name into the Watchlist search at 375px viewport width leaves most of the query visible; the adjacent "Search MusicBrainz to add" button shrinks/truncates instead. +actual: User reported that at 375px, when searching an artist, only the first letter shows and the rest is truncated, because the "Search MusicBrainz to add" button is never truncated. User wants the opposite: shorten the MusicBrainz button so more of the artist name shows. +errors: None reported +reproduction: Test 9 in .planning/phases/25-find-filter-watchlist-and-history/25-UAT.md (ad-hoc, found during the 375px truncation test 6) +started: Discovered during UAT + +## Eliminated + +- hypothesis: The watchlist search INPUT (SearchBox or the WatchlistToolbar filter input) is squeezed by an adjacent button + evidence: SearchBox.tsx renders the input alone in a `w-full max-w-xl` label with no sibling button. Measured at 375px, the input is 311px wide with value "Bad Bunny" fully visible (clientWidth 309 == scrollWidth 309). "Search MusicBrainz to add" renders only in SearchResultsColumns.tsx SearchResultRow, never next to any input. + timestamp: 2026-10-11T00:00:00Z + +- hypothesis: Making the button shrinkable (flex-shrink:1 / min-width:0) is enough to let the name win + evidence: exp. B left the button at 205px and the name at 10px. The name column is flex-1 (flex-basis 0%), so the line has +10px free space and never overflows, and flex-shrink never runs + timestamp: 2026-10-11T00:00:00Z + +- hypothesis: Route-level p-8 padding is the primary cause + evidence: exp. C (p-4) only raised the name from 10px to 42px (4/9 chars), and at 640px (sm two-column grid) the name is 0px regardless. Padding is an aggravating factor, not the cause + timestamp: 2026-10-11T00:00:00Z + +- hypothesis: A phase 25 regression + evidence: 25-UI-SPEC.md:188 marks "SearchBox + SearchResultsColumns (unchanged, D-01)". git log shows SearchResultsColumns.tsx last touched in phase 06 (74c9129, 8418587, aeb472b squash). This is pre-existing phase 6 layout, surfaced by phase 25's 375px UAT pass + timestamp: 2026-10-11T00:00:00Z + +## Evidence + +- timestamp: 2026-10-11T00:00:00Z + checked: .planning/debug/knowledge-base.md + found: no entry about layout, truncation, or 375px (only guest-feature-label-missing and backlog-songs-trigger-discord) + implication: no known-pattern candidate + +- timestamp: 2026-10-11T00:00:00Z + checked: grep "MusicBrainz to add" in web/app + found: the only render site is web/app/components/watchlist/SearchResultsColumns.tsx:190-198, the disabled trailing Button in SearchResultRow for Deezer (non-addable) results, className="shrink-0", size="sm". Its sibling name column is `flex min-w-0 flex-1` with a `truncate` span for artist.name. + implication: the button sits next to the search RESULT's artist name, not next to the toolbar search input. The "search" text squeezed to one letter is most likely the Deezer result row's name. + +- timestamp: 2026-10-11T00:00:00Z + checked: web/app/components/ui/button.tsx:7 (buttonVariants base) and :26 (size sm) + found: the base class already includes `shrink-0` and `whitespace-nowrap`; size sm is `h-8 px-3` text-sm. No max-width, no truncation of the label + implication: the button's flex basis equals its full single-line label width, and it never shrinks + +- timestamp: 2026-10-11T00:00:00Z + checked: SearchResultsColumns.tsx:172-184 row structure + found: li `flex items-center gap-3 p-3`; CoverArt fixed 48px (shrink-0); name column `flex min-w-0 flex-1 flex-col`; the name span has `truncate`; the type span (line 178) has NO truncate + implication: the name column takes leftover space only. The type line ("artist") can overflow its column and collide with the button + +- timestamp: 2026-10-11T00:00:00Z + checked: headless Chrome (CDP) against localhost:8080, /search + /watchlist stubbed, all non-GET blocked (request log was only "STUB /watchlist", "STUB /search?q=Bad+Bunny"), typed "Bad Bunny" + found: | + 375px: MusicBrainz row (Add to Watchlist, btn 135) name col 80px, 8/9 chars. Deezer row (Search MusicBrainz to add, btn 205) name col 10px, 0/9 and 0/14 chars fully visible before the ellipsis + 640px (sm, two columns): Deezer li 276, btn 205, name col 0px, nothing visible + 768px: Deezer name col 39px, 4/9 and 3/14 chars + 1024px: Deezer name col 167px, full names visible + Toolbar SearchBox input at 375px: 311px wide, query fully visible. No horizontal page overflow at any width + Screenshot row-375.png: Deezer rows show "B" / "K" beside the full pill, and the "artist" type text runs under the button's left edge + implication: reproduces the UAT report exactly, and is worse at the sm breakpoint (640px) than at 375px. Math at 375: 311 li - 24 padding - 48 cover - 24 gaps - 205 button = 10px + +- timestamp: 2026-10-11T00:00:00Z + checked: differential runtime-CSS experiments at 375px, Deezer row (no source edits) + found: | + A baseline: btn 205, name 10, 0/9 + B button flex-shrink:1 + min-width:0: btn 205, name 10, 0/9 (no effect) + C page padding p-8 -> p-4: btn 205, name 42, 4/9 + D label shortened to "MusicBrainz only": btn 141, name 74, 8/9 + E button max-width 96px + overflow hidden: btn 96, name 119, 9/9 + F name col flex:1 1 auto only: btn 205, name 10, 0/9 (no effect) + G F + B together: btn 156, name 59, 6/9 + implication: the cause is the unconstrained button width combined with the 0-basis name column. Fixing either flex property alone does nothing. Capping or shortening the button, or changing both basis and shrink, restores the name + +## Resolution + +root_cause: In SearchResultRow (web/app/components/watchlist/SearchResultsColumns.tsx:172-198), the Deezer row's disabled "Search MusicBrainz to add" Button keeps its full single-line max-content width (205px; buttonVariants base `shrink-0 whitespace-nowrap` plus an explicit className="shrink-0", and no max-width). The sibling name column is `flex-1 min-w-0` (flex-basis 0), so it only receives leftover space. At 375px, 311px li - 24px padding - 48px cover - 24px gaps - 205px button = 10px for the name, so the truncating span shows about one glyph and an ellipsis. Because the name basis is 0, the line never overflows, so making the button shrinkable alone has no effect. Pre-existing phase 6 layout (unchanged in phase 25 per UI-SPEC D-01). Secondary: the type span (line 178) lacks `truncate`, so "artist" spills under the button. +fix: +verification: +files_changed: [] diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-01-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-01-SUMMARY.md new file mode 100644 index 00000000..f0315027 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-01-SUMMARY.md @@ -0,0 +1,133 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 01 +subsystem: api +tags: [postgres, sqlc, lateral-join, watchlist, go] + +requires: + - phase: 24-artist-tags-and-notes + provides: "shared ListWatchlist/GetWatchlistEntry projection (D-26) and Entry shape" +provides: + - "latest_release_date and next_release_date on every GET /watchlist entry, from the single ListWatchlist statement" + - "Same two values on Add/UpdatePreferences/UpdateNote responses via the shared projection" +affects: [25-03 card rendering, 25-05 latest-release sort] + +actuals: + tokens: 5900 + tasks: 2 + commits: 2 +plan_head_before: 72f26dac1e2d0086b90bb9e5230fc6a4a23abfa3 + +tech-stack: + added: [] + patterns: + - "Two LEFT JOIN LATERAL ... LIMIT 1 plain-column joins (not aggregates) so sqlc emits *string and NULL scans cleanly" + - "Upcoming decided in SQL by comparing against UTC today truncated to the stored value's own length" + +key-files: + created: [] + modified: + - queries/watchlist.sql + - internal/db/sqlc/watchlist.sql.go + - internal/db/sqlc/querier.go + - internal/watchlist/service.go + - internal/watchlist/service_test.go + - internal/httpserver/watchlist_test.go + +key-decisions: + - "No retention parameter: ListWatchlist keeps its zero-arg signature and NewService is unchanged (G1)" + - "Date fields have no omitempty so a missing release serializes as a present null key" + +requirements-completed: [WLVW-03] + +coverage: + - id: D1 + description: "GET /watchlist returns latest_release_date and next_release_date per entry from one query, null (with 200) for artists without releases" + requirement: WLVW-03 + verification: + - kind: integration + ref: "internal/httpserver/watchlist_test.go#TestWatchlist_List_LatestRelease" + status: pass + human_judgment: false + - id: D2 + description: "Precision, mixed-precision ordering, event-type filtering, no-retention age and UTC-year boundary semantics" + requirement: WLVW-03 + verification: + - kind: integration + ref: "internal/watchlist/service_test.go#TestService_List_LatestRelease" + status: pass + human_judgment: false + - id: D3 + description: "Add, UpdatePreferences and UpdateNote return the same two dates as the matching List element" + requirement: WLVW-03 + verification: + - kind: integration + ref: "internal/watchlist/service_test.go#TestService_ProjectionParity" + status: pass + human_judgment: false + +duration: 25min +completed: 2026-10-10 +status: complete +--- + +# Phase 25 Plan 01: Latest and Next Release Dates Summary + +**GET /watchlist now carries `latest_release_date` and `next_release_date` per entry, computed by two `LEFT JOIN LATERAL ... LIMIT 1` joins inside the single ListWatchlist statement, with Postgres deciding "upcoming" at each date's own precision against UTC today.** + +## Performance + +- **Duration:** about 25 min +- **Completed:** 2026-10-10T16:22Z +- **Tasks:** 2 +- **Files modified:** 6 + +## Accomplishments +- Both ListWatchlist and GetWatchlistEntry gained identical `latest`/`upcoming` lateral joins; sqlc regenerated `LatestReleaseDate`/`NextReleaseDate` as `*string` on both row types, so the `sqlc.ListWatchlistRow(row)` conversion still compiles. +- `watchlist.Entry` exposes the two fields as always-present nullable JSON keys; no retention setting was added (G1). +- Service-level tests pin past/upcoming split, mixed precision (`1999-06` over `1999-05-17` over `1999`; `2999` over `2999-12-25` over `3000-01`), non-release event types and NULL dates being ignored, 400-day-old events still counting, and the current UTC year / next year boundary. +- `TestService_ProjectionParity` now seeds releases and asserts the added entry and every later write path equal the List element. + +## Task Commits + +1. **Task 1: GET /watchlist carries latest and next release dates (tracer)** - `627b159` (feat) +2. **Task 2: Pin latest/upcoming semantics and shared projection** - `a577970` (test) + +**Plan metadata:** committed separately after this summary (docs). + +## Files Created/Modified +- `queries/watchlist.sql` - two lateral joins and two selected columns on ListWatchlist and GetWatchlistEntry +- `internal/db/sqlc/watchlist.sql.go`, `internal/db/sqlc/querier.go` - regenerated by sqlc +- `internal/watchlist/service.go` - `Entry.LatestReleaseDate` / `NextReleaseDate`, mapped in `entryFromRow` +- `internal/httpserver/watchlist_test.go` - `TestWatchlist_List_LatestRelease` +- `internal/watchlist/service_test.go` - `insertReleaseEvent`, `TestService_List_LatestRelease`, extended `TestService_ProjectionParity` + +## Decisions Made +None beyond the plan (G1-G3 followed as specified). + +## Deviations from Plan + +### Process notes + +**1. [Process] Task 2 tests passed on first run (no RED phase)** +- The tracer in Task 1 already implemented the full SQL semantics, so the Task 2 pinning tests exercised existing behavior and were green immediately. They are regression pins, not new behavior; there is no separate failing-test commit. + +**2. [Environment] Race detector unavailable** +- `make test` runs `go test -race`, but this box has no cgo (`-race requires cgo`). The full suite was run without `-race` using the same coverage flags: all 25 packages passed, backend coverage 90.18% (gate 80%). + +**Total deviations:** 0 code deviations, 2 environment/process notes. **Impact:** none on correctness. + +## Issues Encountered +None. + +## Known Stubs +None. + +## Threat Flags +None. The new fields are read-only values from the existing events table behind the existing auth gate (T-25-01..04 handled as planned: LIMIT 1 laterals in one statement, plain-column laterals so NULL scans, no new parameters). + +## Self-Check: PASSED + +- Acceptance criteria: 4 `LEFT JOIN LATERAL` lines; `LatestReleaseDate *string` x2, `NextReleaseDate *string` x2; `ListWatchlist(ctx context.Context) (` signature unchanged; each JSON tag present once; 10 subtest PASS lines in `TestService_List_LatestRelease`. +- Plan verification: `go build ./...`, `go vet ./...`, `golangci-lint run` (0 issues), sqlc regenerate + `git diff --exit-code -- internal/db/sqlc/` clean, full test suite green, coverage 90.18%. +- Commits `627b159` and `a577970` exist on `gsd/phase-25-find-filter-watchlist-and-history`. diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-02-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-02-SUMMARY.md new file mode 100644 index 00000000..350be9cb --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-02-SUMMARY.md @@ -0,0 +1,150 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 02 +subsystem: api +tags: [go, chi, sqlc, postgres, events, tags, retention] + +requires: + - phase: 25-find-filter-watchlist-and-history + provides: "25-01 sqlc regeneration (serialized to keep sqlc-check clean)" +provides: + - "GET /events?tag_id=N filtered by an artist_tags EXISTS predicate inside ListEvents and HasOlderEvents" + - "events.ListParams.TagID passed to both queries from one retention value" + - "tag_id validation (400 invalid tag_id) before the store is called" +affects: [25-06 History Tag control, listEvents({ tagId }) client] + +actuals: + tokens: 5340 + tasks: 2 + commits: 2 +plan_head_before: 66054460b461e37f24f92bad404ff72ef5523624 +commits: 2 + +tech-stack: + added: [] + patterns: + - "Optional sqlc.narg filter as an EXISTS semi-join, repeated verbatim in the list query and its has-older companion" + +key-files: + created: [] + modified: + - queries/events.sql + - internal/db/sqlc/events.sql.go + - internal/events/service.go + - internal/httpserver/events.go + - internal/httpserver/events_test.go + +key-decisions: + - "Tag predicate keys on events.artist_id via artist_tags, so removed artists (no watchlist row) still match (TAG-07)." + - "Reused parseOptionalPositiveInt64 for tag_id: absent/empty is no filter, malformed/<1 is 400 invalid tag_id." + +patterns-established: + - "Any new History filter goes into both ListEvents and HasOlderEvents so retention and has_older_events stay scoped together." + +requirements-completed: [HIST-02] + +coverage: + - id: D1 + description: "GET /events?tag_id=N lists only events of artists carrying tag N, including artists with no watchlist row" + requirement: HIST-02 + verification: + - kind: integration + ref: "internal/httpserver/events_test.go#TestListEvents_TagFilter" + status: pass + human_judgment: false + - id: D2 + description: "No aged-out event appears under a tag filter on the first page, any cursor page, or in has_older_events" + requirement: HIST-02 + verification: + - kind: integration + ref: "internal/httpserver/events_test.go#TestRetention_TagFilterComposesWithCutoff" + status: pass + human_judgment: false + - id: D3 + description: "tag_id composes with artist_id and event_type; unknown tag id gives an empty page with has_older_events false" + requirement: HIST-02 + verification: + - kind: integration + ref: "internal/httpserver/events_test.go#TestListEvents_TagFilterComposesWithOtherFilters" + status: pass + human_judgment: false + - id: D4 + description: "Malformed tag_id returns 400 invalid tag_id without calling the store; TagID reaches both queries" + requirement: HIST-02 + verification: + - kind: unit + ref: "internal/httpserver/events_test.go#TestHandleListEvents_Validation" + status: pass + human_judgment: false + +duration: 25min +completed: 2026-10-10 +status: complete +--- + +# Phase 25 Plan 02: History tag filter API Summary + +**GET /events accepts tag_id via an artist_tags EXISTS predicate placed inside the retention-aware ListEvents and HasOlderEvents queries, so a tag filter can never surface aged-out events.** + +## Performance + +- **Duration:** 25 min +- **Started:** 2026-10-10T16:12:00Z +- **Completed:** 2026-10-10T16:37:06Z +- **Tasks:** 2 +- **Files modified:** 5 + +## Accomplishments +- Tag predicate added to both queries; sqlc regenerated with `TagID *int64` on both param structs. +- `events.ListParams.TagID` flows into both queries alongside the single retention cutoff. +- `handleListEvents` parses `tag_id` with the fixed `invalid tag_id` 400 message. +- Tests prove removed-artist matching, retention on the first page and every cursor page, `has_older_events` scoping, AND composition, the unknown-tag empty page, and the validation contract. + +## Task Commits + +1. **Task 1: Filter GET /events by one tag, end to end** - `c421197` (feat) +2. **Task 2: Retention, paging, composition, and validation under a tag filter** - `00e0ada` (test) + +**Plan metadata:** committed separately (docs: complete plan) + +_Note: Task 2 is tdd="true", but the Task 1 tracer already shipped the implementation, so its tests passed on first run with no separate RED commit._ + +## Files Created/Modified +- `queries/events.sql` - `sqlc.narg('tag_id')` EXISTS predicate in ListEvents and HasOlderEvents +- `internal/db/sqlc/events.sql.go` - regenerated; `TagID` on both param structs +- `internal/events/service.go` - `ListParams.TagID`, passed to both queries +- `internal/httpserver/events.go` - `tag_id` parsing and doc comment +- `internal/httpserver/events_test.go` - tag/link helpers, three integration tests, validation cases, `recordingQuerier.gotOlderParams` + +## Decisions Made +None - followed plan as specified. + +## Deviations from Plan + +None - plan executed exactly as written. + +## Issues Encountered +- `make` is not on PATH and cgo is disabled, so the Makefile recipes were run directly without `-race`. `sqlc-check` was run as `git add internal/db/sqlc`, `sqlc generate`, `git diff --exit-code` (clean); the sqlc version matches the pinned v1.31.1. +- Coverage gate recipe run directly: backend total 90.25% (floor 80%). Go printed `no such tool "covdata"` for `internal/testutil` (no test files); the profile and total are unaffected. +- `gofmt -l` lists five pre-existing files (`artistart/match_test.go`, `httpserver/server.go`, `settings_test.go`, `notifier/timeout_test.go`, `sqlscan/lex.go`). None were touched by this plan. + +## Known Stubs +None. + +## Threat Flags +None - the only new surface is the `tag_id` query parameter covered by T-25-05 to T-25-07. It is validated before the store and bound as a sqlc parameter. + +## User Setup Required +None - no external service configuration required. + +## Next Phase Readiness +- `GET /events?tag_id=N` is ready for 25-06's History Tag control (`listEvents({ tagId })`). + +## Self-Check: PASSED + +- FOUND: queries/events.sql, internal/db/sqlc/events.sql.go, internal/events/service.go, internal/httpserver/events.go, internal/httpserver/events_test.go +- FOUND commits: c421197, 00e0ada + +--- +*Phase: 25-find-filter-watchlist-and-history* +*Completed: 2026-10-10* diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-03-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-03-SUMMARY.md new file mode 100644 index 00000000..1b42fa0e --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-03-SUMMARY.md @@ -0,0 +1,127 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 03 +subsystem: web +tags: [watchlist, release-dates, formatter, react, vitest] +requires: + - phase: 25-01 + provides: "GET /watchlist latest_release_date / next_release_date fields" +provides: + - "WatchlistEntry.latest_release_date / next_release_date (string | null)" + - "formatReleaseDate(raw): stored-precision, timezone-free date formatter" + - "makeEntry(overrides) test fixture builder for later Phase 25 plans" + - "Latest release / Upcoming lines on every Watchlist card" +affects: [25-04, 25-05, 25-07, 25-08] +tech-stack: + added: [] + patterns: + - "String-only date formatting (fixed month table, anchored regexes, no Date)" + - "Shared test fixture builder under app/lib/test/" +key-files: + created: + - web/app/lib/test/fixtures.ts + modified: + - web/app/lib/api.ts + - web/app/lib/format.ts + - web/app/lib/format.test.ts + - web/app/components/watchlist/WatchlistRow.tsx + - web/app/routes/watchlist.tsx + - web/app/routes/watchlist.test.tsx + - web/app/lib/api.test.ts + - web/app/components/history/HistoryFilters.test.tsx + - web/app/components/watchlist/ArtistNote.test.tsx + - web/app/components/watchlist/PreferenceToggles.test.tsx + - web/app/components/watchlist/SearchResultsColumns.test.tsx + - web/app/components/watchlist/TagChips.test.tsx + - web/app/components/watchlist/TagCombobox.test.tsx +key-decisions: + - "Release dates render via string-only formatReleaseDate; malformed input passes through unchanged as plain JSX text" + - "Client never compares dates to today; server decides what is upcoming (G2/G3)" +requirements-completed: [WLVW-03] +status: complete +commits: 2 +plan_head_before: f6687a68fcafd1397bb02e8c2c130c9cacf80d69 +actuals: + tokens: 9000 + tasks: 2 + commits: 2 +duration: 8 min +completed: 2026-10-10 +coverage: + - deliverable: "Watchlist cards show latest release or 'No releases yet' plus conditional Upcoming" + verification: + - kind: test + ref: "web/app/routes/watchlist.test.tsx#renders each card's latest release and a conditional Upcoming segment (D-03)" + status: pass + human_judgment: false + - deliverable: "formatReleaseDate pins year / year-month / full-date table and passes malformed input through" + verification: + - kind: test + ref: "web/app/lib/format.test.ts#formatReleaseDate" + status: pass + human_judgment: false + - deliverable: "Release dates render as literal text (no markup injection)" + verification: + - kind: test + ref: "web/app/routes/watchlist.test.tsx#renders a release date containing markup as literal text (T-25-09)" + status: pass + human_judgment: false + - deliverable: "makeEntry fixture builder and outdated route comment replaced" + verification: + - kind: command + ref: "corepack pnpm --dir web run typecheck" + status: pass + human_judgment: false +--- + +# Phase 25 Plan 03: Latest and upcoming release on Watchlist cards Summary + +Each Watchlist card now shows `Latest release: {date}` (or `No releases yet`) and, only when the server sends a next date, `Upcoming: {date}`, formatted at stored precision by a string-only `formatReleaseDate` with no Date construction. + +## Accomplishments + +- `WatchlistEntry` gained `latest_release_date` and `next_release_date` (`string | null`), matching plan 25-01's JSON contract; all existing fixtures updated and typecheck is green. +- `formatReleaseDate` renders `2024`, `May 2024`, `17 May 2024` from a fixed English month table and returns anything else unchanged. +- `WatchlistRow` renders a single wrapping line in the name column, with the Upcoming segment conditional on `next_release_date`. +- `makeEntry` fixture builder added in `web/app/lib/test/fixtures.ts` for the rest of the phase. +- The `watchlist.tsx` module comment no longer claims "never re-sorts" or "no release activity"; replaced with a 3-line Phase 25 D-02/D-03 note. + +## Task Commits + +1. Task 1 (tracer): `aa50c00` feat(25-03): show latest and upcoming release on Watchlist cards +2. Task 2 (tdd): `07e47ee` test(25-03): pin release-date table, add makeEntry fixture, refresh route comment + +## Deviations from Plan + +### Notes + +**1. [Plan criterion quirk] Date-construction grep matches the function signature** +- **Found during:** Task 1 acceptance check +- **Issue:** `awk '/export function formatReleaseDate/,/^}/' format.ts | grep -c 'Date('` prints `1`, not `0`, because the signature line `formatReleaseDate(raw: string)` contains the substring `Date(`. No Date object is constructed anywhere in the function body. +- **Fix:** None needed; intent (no Date construction) is met. Verified by reading the function. + +**2. [TDD ordering] Task 2 formatter tests were green on first run** +- **Issue:** `formatReleaseDate` was already implemented in the Task 1 tracer (the tracer's route test needed it), so Task 2's table tests could not be observed RED. The tests pin the table and were confirmed passing. + +**Total deviations:** 0 auto-fixed, 2 notes. **Impact:** none on behavior or scope. + +## Verification + +- `vitest run app/lib/format.test.ts app/routes/watchlist.test.tsx`: 69 passed +- `corepack pnpm run typecheck`: exit 0 +- `prettier --check "**/*.{ts,tsx}"`: clean +- `corepack pnpm test` (full web suite with coverage): passed + +## Known Stubs + +None. + +## Threat Flags + +None. T-25-09 (XSS) and T-25-10 (date integrity) mitigated as planned: plain JSX text nodes, literal-`` route test, no Date or timezone math. + +## Self-Check: PASSED + +- FOUND: web/app/lib/test/fixtures.ts +- FOUND commits aa50c00, 07e47ee +- Task 1 and Task 2 acceptance criteria re-run and passing (see note 1) diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-04-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-04-SUMMARY.md new file mode 100644 index 00000000..91986817 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-04-SUMMARY.md @@ -0,0 +1,145 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 04 +subsystem: web +tags: [watchlist, name-filter, url-state, sticky-cards, react, vitest] +requires: + - phase: 25-03 + provides: "makeEntry fixture and the Watchlist card list this plan wraps in a section" +provides: + - "watchlistView.ts: foldName, matchesName, parseWatchlistParams, selectVisible, isFiltered, formatArtistCount, CLEAR_FILTER_KEYS" + - "useUrlParams: the single URL writer (ref-based, preventScrollReset always on)" + - "WatchlistToolbar with the Filter watchlist name filter and Clear filters" + - "renderRoute initialEntry parameter and LocationProbe test helper" + - "Your artists section with N of M count, filtered-to-zero state, count announcements, and the G4 sticky set" +affects: [25-05, 25-07, 25-08] +tech-stack: + added: [] + patterns: + - "Pure view-model module with no runtime import from ~/lib/api (route tests automock it)" + - "All URL writes through one ref-based helper so same-tick writes compose and scroll never resets" + - "Local input state filters instantly; debounced replace write to the URL; resync only when the URL differs from the last own write" +key-files: + created: + - web/app/lib/watchlistView.ts + - web/app/lib/watchlistView.test.ts + - web/app/lib/useUrlParams.ts + - web/app/lib/useUrlParams.test.tsx + - web/app/components/watchlist/WatchlistToolbar.tsx + - web/app/components/watchlist/WatchlistToolbar.test.tsx + modified: + - web/app/lib/test/routeStub.tsx + - web/app/routes/watchlist.tsx + - web/app/routes/watchlist.test.tsx +key-decisions: + - "q is filtered from local state on every keystroke and written to the URL 300ms trailing with replace; the input resyncs from the URL only when view.q differs from lastWrittenQ (Back/forward), never on an echo" + - "refresh() now resolves with the loaded rows (null on failure) so the 409 add path can mark the existing artist sticky; it never touches the sticky set" + - "Sticky set is route state keyed by id, cleared only by a search text change or Clear filters" +requirements-completed: [WLVW-01, WLVW-06] +status: complete +commits: 5 +plan_head_before: f444b94b66b0a27598e5b5647f743005fb997f6e +actuals: + tokens: 23000 + tasks: 3 + commits: 5 +duration: 18 min +completed: 2026-10-10 +coverage: + - deliverable: "Name filter narrows the list per keystroke over name only, with N of M count and no extra requests" + verification: + - kind: test + ref: "web/app/routes/watchlist.test.tsx#Watchlist name filter (WLVW-01)" + status: pass + human_judgment: false + - deliverable: "Fold table (accents, A$AP, Mø, Ænima, Straße) and blank-query behavior" + verification: + - kind: test + ref: "web/app/lib/watchlistView.test.ts#matchesName" + status: pass + human_judgment: false + - deliverable: "URL writer composes same-tick writes, keeps external params, and always sets preventScrollReset" + verification: + - kind: test + ref: "web/app/lib/useUrlParams.test.tsx" + status: pass + human_judgment: false + - deliverable: "Name filter clear x and Esc keep focus; Enter does nothing" + verification: + - kind: test + ref: "web/app/components/watchlist/WatchlistToolbar.test.tsx" + status: pass + human_judgment: false + - deliverable: "Clear filters, filtered-to-zero state, announcements, and sticky adds/Undo (G4)" + verification: + - kind: test + ref: "web/app/routes/watchlist.test.tsx#Watchlist filtered state (WLVW-06)" + status: pass + human_judgment: false + - deliverable: "No request during search and no browser history throttling on key hold (real browser network panel)" + verification: [] + human_judgment: true + rationale: "Network-panel request counting and history throttling are only observable in a real browser; deferred to end-of-phase UAT per the plan's human-check" +--- + +# Phase 25 Plan 04: Watchlist name filter, count, URL state, and sticky cards Summary + +The Watchlist gains a `Your artists` section with a live `Filter watchlist` name filter (accent and `$ø æ ß` folding), an always-visible `N of M artists` count, debounced replace-mode URL state through a single scroll-safe writer, a Clear filters flow that preserves sort, and a sticky set so adds and Undo restores never succeed invisibly. + +## Accomplishments + +- `watchlistView.ts` is a pure view-model (`foldName`, `matchesName`, `selectVisible`, `isFiltered`, `formatArtistCount`, `CLEAR_FILTER_KEYS`) with no runtime import from `~/lib/api`, so it survives route-test automocking. +- `useUrlParams` is the one URL writer: it builds on a ref so two same-tick writes compose, keeps params from external navigation, and always passes `preventScrollReset: true`. +- `WatchlistToolbar` renders the `Filter and sort artists` group with the name filter (label, `Filter by name…`, `ListFilter` icon), a `Clear name filter` x, Esc-to-clear, and the ghost `Clear filters` button. +- The route filters from local state on every keystroke with zero requests, writes `q` 300ms trailing with replace, and resyncs the input only on Back/forward or external URL changes. +- Filtered-to-zero shows `No artists match these filters` with a single default `Clear filters`; the toolbar button renders only when filtered and N > 0, so no duplicate buttons. +- Clear filters deletes `q`, `tags`, `muted`, `custom` in one push, keeps `sort`, focuses the input, and announces `Filters cleared. Showing {M} of {M} artists.`; search changes announce the count 500ms after the last keystroke through the existing status region. +- In-card edits (entry change, tag add/remove), add-from-search (including the 409 path), and Undo restores mark their card sticky; a search text change or Clear filters empties the set; `refresh()` never does. + +## Task Commits + +1. Task 1 (tracer): `115e13f` feat(25-04): filter the Watchlist by name with live count and URL state +2. Task 2 RED: `3062fd9` test(25-04): pin fold table, URL writer, and name filter keys +3. Task 2 GREEN: `e11bb20` feat(25-04): add the name filter clear button and Esc handling +4. Task 3 RED: `e431c13` test(25-04): pin clear filters, announcements, and sticky adds +5. Task 3 GREEN: `882a047` feat(25-04): add Clear filters, filtered-to-zero state, announcements, sticky cards + +## Deviations from Plan + +### Notes + +**1. [TDD ordering] Fold-table and URL-writer tests were green on first run** +- **Issue:** `foldName`, `matchesName`, `useUrlParams`, and `formatArtistCount` were already built in the Task 1 tracer, so those Task 2 tests pinned existing behavior and could not be observed RED. The toolbar clear x and Esc cases were genuinely RED (2 failing on the planned assertions) and drove the Task 2 feature commit. + +**2. [Test harness] LocationProbe's `` has an implicit status role** +- **Found during:** Task 3 GREEN +- **Issue:** with the probe rendered beside the route, `getByRole("status")` matched two elements. +- **Fix:** the test helper selects the route status region by `aria-atomic="true"`. Production markup is unchanged; the "exactly one status region" test renders without the probe and still passes. +- **Commit:** `882a047` + +**3. [Scope] No test for "in-card edit keeps a card that stopped matching"** +- **Issue:** with only the name axis, an in-card edit cannot change whether a card matches, so the behavior is unobservable until tag and preference filters land (plans 25-07/08). `markSticky` is wired into `handleEntryChange`, `addTag`, `removeTag` (grep shows 6 call sites plus the definition) and should be covered in those plans. + +**Total deviations:** 0 auto-fixed, 3 notes. **Impact:** none on behavior or scope. + +## Verification + +- `corepack pnpm test` (full web suite with coverage): 28 files, 432 tests passed +- `corepack pnpm run typecheck`: exit 0 +- `prettier --check "**/*.{ts,tsx}"`: clean +- Acceptance greps: `preventScrollReset: true` x1, `useUrlParams()` x1, `watchlist-artists-heading` x1, `LocationProbe` export x1, `Clear name filter` x1, `A$AP Rocky` in tests, `No artists match these filters` x2, `Filters cleared. Showing` x1, `markSticky(` x7 +- Pending human check: network panel on a 50+ artist dev DB (one `GET /watchlist`, no requests while typing, no history throttling warning). Deferred to end-of-phase UAT. + +## Known Stubs + +None. + +## Threat Flags + +None. T-25-11 (`q` rendered only as controlled input value and JSX text), T-25-12 (300ms debounced replace writes), T-25-13 (route test asserts one `listWatchlist` call and no `listTags`/`listEvents`), and T-25-14 (sticky set keyed by POST response id, add and Undo tests) are mitigated as planned. + +## Self-Check: PASSED + +- FOUND: web/app/lib/watchlistView.ts, web/app/lib/useUrlParams.ts, web/app/components/watchlist/WatchlistToolbar.tsx, and the three new test files +- FOUND commits 115e13f, 3062fd9, e11bb20, e431c13, 882a047 +- All task acceptance criteria re-run and passing diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-05-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-05-SUMMARY.md new file mode 100644 index 00000000..375e10b7 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-05-SUMMARY.md @@ -0,0 +1,83 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 05 +subsystem: web +tags: [watchlist, sort, intl-collator, url-state, react, vitest] +requires: + - phase: 25-01 + provides: "latest_release_date / next_release_date on GET /watchlist" + - phase: 25-04 + provides: "Watchlist toolbar, useUrlParams, watchlistView.ts, sticky cards" +provides: + - "SortKey, DEFAULT_SORT, SORT_OPTIONS, compareForSort, canonicalWatchlistParams in watchlistView.ts" + - "selectVisible sorts after filtering" + - "Sort by Select in WatchlistToolbar (vendored Select, six options)" + - "handleSortChange (push, clears sticky, announces) and the D-13 canonicalization effect (replace)" +affects: [25-07, 25-08] +plan_head_before: 20bbfad2ee01eb47426a2358f72e31b57543e19b +actuals: + tokens: 9500 + tasks: 2 + commits: 3 +tech-stack: + added: [] + patterns: + - "One comparator per direction instead of reversing an ascending sort, so id ties and null-last survive" + - "canonicalWatchlistParams owns its keys only; later plans extend it" +key-files: + created: [] + modified: + - web/app/lib/watchlistView.ts + - web/app/lib/watchlistView.test.ts + - web/app/components/watchlist/WatchlistToolbar.tsx + - web/app/components/watchlist/WatchlistToolbar.test.tsx + - web/app/routes/watchlist.tsx + - web/app/routes/watchlist.test.tsx +key-decisions: + - "Latest release compares raw text with code-unit < / > so 2024 sorts before 2024-05; nulls sort last in both directions" + - "Sort change cancels any pending count announcement and announces Sorted by {label}. immediately" + - "The canonicalization effect copies only the sort key into the mutated params, never touching keys it does not own" +requirements-completed: [WLVW-02, WLVW-03] +duration: 5 min +completed: 2026-10-10 +status: complete +commits: 3 +--- + +# Phase 25 Plan 05: Watchlist Sort Summary + +Six-mode Sort by control on the Watchlist (name, date added, latest release, each in both directions) with artist-id-stable ties, null-last latest-release ordering, URL round-trip, invalid-sort cleanup, and a live announcement. + +## What was built + +- `watchlistView.ts`: `SortKey`, `DEFAULT_SORT` (`name-asc`), `SORT_OPTIONS` (UI-SPEC order, en-dash labels), `parseWatchlistParams` now yields `sort` (unknown maps to default), `compareForSort`, `canonicalWatchlistParams`, and `selectVisible` sorts after filtering. +- Name modes use a single module-level `Intl.Collator(undefined, { sensitivity: "base" })`; descending negates only the collator term. Date-added compares `Date.parse(created_at)`. Latest-release compares raw text and ignores `next_release_date`. Every mode ends on `artist_id` ascending; no `.reverse()` is used. +- `WatchlistToolbar`: `Sort by` label (`#watchlist-sort-label`) and vendored `Select` built from `SORT_OPTIONS`, with `sort` / `onSortChange` props. +- `watchlist.tsx`: `handleSortChange` (push write, `sort` omitted for the default, clears sticky, cancels the count timer, announces) and a `useEffect` that rewrites non-canonical params with replace. + +## TDD / task flow + +- Task 1 (tracer): `0c914ef` feat. Tracer re-verified end to end (route tests + typecheck) before expanding. +- Task 2 RED: `19bea20` test. Failed on the target assertions: `canonicalWatchlistParams is not a function`, bogus/default sort left in the URL, and missing `Sorted by` announcement. Comparator-table tests already passed because Task 1 implemented the comparators (expected for the tracer slice). +- Task 2 GREEN: `5fad3d0` feat. No refactor commit. + +## Deviations from Plan + +None - plan executed exactly as written. Existing `selectVisible` / `WatchlistToolbar` tests were updated for the new required `sort` field/prop (in-scope test maintenance; one sticky-order expectation changed from input order to sorted order). + +## Known Stubs + +None. + +## Threat Flags + +None. T-25-15 mitigated: `sort` is allow-listed against the six keys and rewritten away with replace when unknown. + +## Verification + +- `corepack pnpm test`: 28 files, 477 tests passed; `typecheck` and `prettier --check` clean. +- Human check (perceived stability on real data) remains for phase verification: flip each direction on a list with shared names / latest dates and `No releases yet` artists. + +## Self-Check: PASSED + +Files modified exist; commits 0c914ef, 19bea20, 5fad3d0 present. diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-06-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-06-SUMMARY.md new file mode 100644 index 00000000..2073358a --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-06-SUMMARY.md @@ -0,0 +1,171 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 06 +subsystem: web +tags: [history, tag-filter, combobox, empty-state, react, vitest] +requires: + - phase: 25-02 + provides: "tag_id query param on GET /events" + - phase: 25-03 + provides: "tag-aware watchlist cards and GET /tags consumers" +provides: + - "listEvents({ tagId }) sending tag_id" + - "sortTagsByName: base-collation tag ordering with id tie-break" + - "History Tag combobox after Event type, fed by one GET /tags per mount" + - "Combobox hardening: max-h-72 scroll, scrollIntoView on highlight, max-w-64 truncated titled trigger, triggerLabel/triggerRef props" + - "Tag trigger degraded labels (Loading tags…, Tag unavailable)" + - "Tag-specific History empty state with Show all tags" +affects: [25-07, 25-09] +plan_head_before: c6c7e46e46a8d1a1ccf6887d24861adfb257ae73 +actuals: + tokens: 7700 + tasks: 3 + commits: 5 +tech-stack: + added: [] + patterns: + - "Hand-rolled History Combobox stays alongside the base-ui Combobox used on Watchlist (G7 add-alongside)" + - "Optional-call scrollIntoView?.() so jsdom needs only a beforeEach stub" +key-files: + created: [] + modified: + - web/app/lib/api.ts + - web/app/lib/api.test.ts + - web/app/lib/tags.ts + - web/app/lib/tags.test.ts + - web/app/components/history/HistoryFilters.tsx + - web/app/components/history/HistoryFilters.test.tsx + - web/app/routes/history.tsx + - web/app/routes/history.test.tsx +key-decisions: + - "Tag degraded trigger labels apply only when a tag is selected; with no tag selected the trigger always reads All tags, matching the plan's behavior block over its looser prose" + - "showAllTags focuses the Tag trigger synchronously through tagTriggerRef, since the trigger never unmounts" + - "Unknown tag name falls back to this tag in the route; that path is not UI-reachable until plan 25-09 moves filter state into the URL" +requirements-completed: [HIST-02] +duration: 14 min +completed: 2026-10-10 +status: complete +commits: 5 + +coverage: + - id: D1 + description: "Picking a tag on History sends tag_id composed with Artist and Event type; Load more carries it; GET /tags is requested once per mount" + requirement: HIST-02 + verification: + - kind: unit + ref: "web/app/routes/history.test.tsx#lists tags from GET /tags once, sorted by name, and a picked tag narrows the feed and survives Load more" + status: pass + - kind: unit + ref: "web/app/lib/api.test.ts#sends tagId as tag_id" + status: pass + human_judgment: false + - id: D2 + description: "Tag trigger never silently reads All tags while a selected tag is loading or failed; failed tags offer only All tags" + requirement: HIST-02 + verification: + - kind: unit + ref: "web/app/components/history/HistoryFilters.test.tsx#Tag control" + status: pass + human_judgment: false + - id: D3 + description: "Shared combobox listbox scrolls (max-h-72) and keeps the highlighted option in view; trigger truncates with a title" + requirement: HIST-02 + verification: + - kind: unit + ref: "web/app/components/history/HistoryFilters.test.tsx#listbox hardening" + status: pass + human_judgment: false + - id: D4 + description: "Visual truncation of a 32-character tag or long artist name inside the max-w-64 trigger at 375px" + verification: [] + human_judgment: true + rationale: "Pixel-level truncation at a 375px viewport is not asserted by jsdom; the plan marks it a held-out backstop check" + - id: D5 + description: "Tag-specific empty state with precedence over unfiltered copy, under retention copy, and a Show all tags action that refocuses the Tag trigger" + requirement: HIST-02 + verification: + - kind: unit + ref: "web/app/routes/history.test.tsx#History tag empty state" + status: pass + human_judgment: false +--- + +# Phase 25 Plan 06: History Tag Filter Summary + +**History gains a Tag combobox that sends tag_id alongside Artist and Event type, with a hardened scrollable/truncating shared combobox, honest loading/unavailable trigger labels, and a tag-specific empty state with a Show all tags way out.** + +## Performance + +- **Duration:** 14 min +- **Started:** 2026-10-10T16:36:00Z +- **Completed:** 2026-10-10T16:50:00Z +- **Tasks:** 3 +- **Files modified:** 8 + +## Accomplishments +- `listEvents` forwards `tagId` as `tag_id`; `sortTagsByName` orders tags with one module-level `Intl.Collator` and an id tie-break (consumed by 25-07). +- History fetches `GET /tags` once per mount, feeds a third `Tag` combobox (`All tags` first), and resets/refetches page one on a tag change; Load more keeps the tag. +- The module-private `Combobox` now caps its listbox at `max-h-72` with scroll, scrolls the highlighted option into view, truncates a titled label inside a `max-w-64` trigger, and accepts `triggerLabel`/`triggerRef`. +- Empty state precedence is retention, then tag, then filtered, then unfiltered, with the two tag body variants and a secondary `Show all tags` button that clears only the tag and refocuses the trigger. + +## Task Commits + +1. **Task 1: Pick a tag on History and the feed narrows to it (tracer)** - `cac69cd` (feat) +2. **Task 2: Harden the shared combobox and Tag trigger labels (TDD)** - `9cb0416` (test, RED), `200b13f` (feat, GREEN) +3. **Task 3: Tag-specific empty state with Show all tags (TDD)** - `227222c` (test, RED), `fa9483f` (feat, GREEN) + +**Plan metadata:** committed with this summary (docs). + +## Files Created/Modified +- `web/app/lib/api.ts` - `tagId` param on `listEvents` +- `web/app/lib/tags.ts` - `sortTagsByName` +- `web/app/components/history/HistoryFilters.tsx` - Tag control, `Combobox` hardening, degraded labels +- `web/app/routes/history.tsx` - single `listTags` mount effect, `tagId` in requests/deps, `emptyStateCopy` tag branch, `showAllTags` +- Matching `*.test.ts(x)` files extended for each behavior; existing HistoryFilters tests updated for the new required props + +## Decisions Made +- Degraded trigger labels (`Loading tags…`, `Tag unavailable`) apply only while a tag is selected, so a failed GET /tags with no selection still reads `All tags`. +- `Show all tags` focuses the trigger synchronously through the ref; the trigger stays mounted so no effect or rAF is needed. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 3 - Blocking] Existing HistoryFilters tests needed the new required props and `tagId`** +- **Found during:** Task 1 +- **Issue:** `HistoryFiltersValue.tagId` and the `tags`/`tagsStatus` props made the existing test file fail typecheck. +- **Fix:** Added `tagId: null` to fixtures and a shared `baseProps`. +- **Files modified:** `web/app/components/history/HistoryFilters.test.tsx` +- **Committed in:** `cac69cd` + +### Other notes + +- **Acceptance grep not literally satisfiable:** `grep -c 'max-h-72 overflow-y-auto'` expects adjacent classes, but `prettier-plugin-tailwindcss` orders them `max-h-72 min-w-full overflow-y-auto`. Both classes are present and asserted with `toHaveClass("max-h-72", "overflow-y-auto")`. +- **Tag-name fallback untested:** the `this tag` fallback is implemented in the route, but History filter state is `useState` here, so a selected tag always has a known name. It becomes reachable once 25-09 moves state to the URL; add the test there. +- **`sortTagsByName` test green at RED time:** the helper shipped in Task 1 as the plan specifies, so its Task 2 test case passed immediately. The other six Task 2 tests failed on assertions as intended. + +--- + +**Total deviations:** 1 auto-fixed (1 blocking), 3 notes +**Impact on plan:** No scope creep. + +## Issues Encountered +None. + +## Known Stubs +None. + +## User Setup Required +None - no external service configuration required. + +## Next Phase Readiness +- 25-07 can reuse `sortTagsByName`; 25-09 can lift `HistoryFiltersValue` (now including `tagId`) into the URL and should add the unknown-tag-name fallback test. +- Held-out visual check for 375px truncation remains for UAT. + +## Self-Check: PASSED +- All eight modified files exist; commits `cac69cd`, `9cb0416`, `200b13f`, `227222c`, `fa9483f` present. +- Acceptance greps pass (see note on the adjacency grep). Full web suite: 28 files, 445 tests pass; typecheck and prettier --check clean. + +--- +*Phase: 25-find-filter-watchlist-and-history* +*Completed: 2026-10-10* diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-07-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-07-SUMMARY.md new file mode 100644 index 00000000..3ee2ba60 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-07-SUMMARY.md @@ -0,0 +1,153 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 07 +subsystem: web +tags: [watchlist, tag-filter, combobox, url-state, react, vitest] +requires: + - phase: 25-04 + provides: "useUrlParams writer, sticky set, Clear filters that already deletes tags" + - phase: 25-05 + provides: "param canonicalization effect and sort" + - phase: 25-06 + provides: "sortTagsByName and the hardened combobox vendored parts" +provides: + - "watchlistView.ts: tagIds axis, matchesAnyTag, tagFilterOptions, TagFilterOption, tags parse and canonicalization" + - "WatchlistTagFilter: any-of multi-select on the vendored base-ui Combobox with chips" + - "ComboboxChip removeLabel prop and contrast-safe remove button" + - "Route tag-id lifecycle: first-load validation, Manage tags delete/merge sync, Back resurrection handling" +affects: [25-08] +tech-stack: + added: [] + patterns: + - "Tag names cached by id in a ref so a chip outlives its last carrier" + - "Retired-id map (deleted -> null, merged -> target) consulted by the canonicalization effect so Back cannot resurrect a dead tag" + - "Filter options derived from the loaded payload; GET /tags is never called" +key-files: + created: + - web/app/components/watchlist/WatchlistTagFilter.tsx + - web/app/components/watchlist/WatchlistTagFilter.test.tsx + modified: + - web/app/components/ui/combobox.tsx + - web/app/components/watchlist/WatchlistToolbar.tsx + - web/app/components/watchlist/WatchlistToolbar.test.tsx + - web/app/lib/watchlistView.ts + - web/app/lib/watchlistView.test.ts + - web/app/routes/watchlist.tsx + - web/app/routes/watchlist.test.tsx +key-decisions: + - "Esc on a closed popup makes base-ui clear every chip through onValueChange (reason escape-key); the filter cancels that reason so a second Esc does nothing" + - "Multiple mode already keeps the popup open on pick in jsdom (A2 held), so the item-press onOpenChange cancel is a defensive pin rather than a fix" + - "A1 held: base-ui's default filter is a case-insensitive contains, so no explicit filter prop was added" + - "tags in the URL is written through URLSearchParams, so commas serialize as %2C (?tags=5%2C9); the decoded value is the ascending comma-separated id list" +requirements-completed: [WLVW-04, WLVW-06] +status: complete +commits: 5 +plan_head_before: 27d472fe2dd9026e0d194a355762bf969cdb172e +actuals: + tokens: 30000 + tasks: 3 + commits: 5 +duration: 40 min +completed: 2026-10-10 +coverage: + - deliverable: "Tags: any of narrows the list to carriers (OR), empty selection is no constraint, options come from the payload with one request" + verification: + - kind: test + ref: "web/app/routes/watchlist.test.tsx#Watchlist tag filter (WLVW-04)" + status: pass + human_judgment: false + - deliverable: "Multi-select contract: stays open and focused after a pick, deselect, Backspace and chip x removal, Esc, placeholder, sorted chips, counts, empty copy" + verification: + - kind: test + ref: "web/app/components/watchlist/WatchlistTagFilter.test.tsx" + status: pass + human_judgment: false + - deliverable: "Tag axis in the view model (parse cap and shape, canonicalization, matchesAnyTag, tagFilterOptions ordering, selectVisible AND)" + verification: + - kind: test + ref: "web/app/lib/watchlistView.test.ts#tag axis" + status: pass + human_judgment: false + - deliverable: "First-load validation, Manage tags delete/merge sync, Back resurrection, detach and refresh leave the filter alone" + verification: + - kind: test + ref: "web/app/routes/watchlist.test.tsx#Watchlist tag filter consistency (D-13)" + status: pass + human_judgment: false + - deliverable: "Chip x meets 3:1 contrast against the chip fill, and a 32-character tag truncates at 375px" + verification: [] + human_judgment: true + rationale: "Contrast and truncation at width are only measurable in the rendered app; deferred to end-of-phase UAT per the plan's human-check" +--- + +# Phase 25 Plan 07: Watchlist tag filter Summary + +The Watchlist gains a `Tags: any of` multi-select (vendored base-ui Combobox with chips) fed by tags the loaded artists carry, narrowing the list with OR semantics and a `tags=` URL param that stays truthful across first load, Manage tags delete and merge, Back, detaches, and refreshes. + +## Accomplishments + +- `watchlistView.ts` owns the tag axis: `parseWatchlistParams` reads at most 200 `tags` tokens matching `^[1-9]\d*$` (ascending, de-duplicated), `canonicalWatchlistParams` rewrites or deletes the param, `matchesAnyTag` and `tagFilterOptions` (collator then id order) are pure, `selectVisible` ANDs the tag axis with name, and `isFiltered` counts it. +- `WatchlistTagFilter` is built only from vendored parts in `multiple` mode (no TagCombobox, no create option). Chips are `h-6` neutral and truncate with `title`; options show a `tabular-nums` carrier count; placeholder `All tags` only while empty; `No tags yet` and `No tags match` copy through `ComboboxEmpty`. +- `ComboboxChip` takes `removeLabel` (the remove button's aria-label) and uses `text-muted-foreground hover:text-foreground` instead of 50% opacity. +- The route caches tag names by id, so a filter chip survives its last carrier leaving. Picking pushes `tags`, clears sticky, and schedules the count announcement. +- First successful load drops URL tag ids no artist carries (replace) exactly once. Manage tags delete and merge rewrite `tags` with replace (merge swaps source for target, de-duplicated). A retired-id map lets the canonicalization effect drop or swap a deleted or merged id that Back brings into the URL. `refresh()` and chip detach never touch `tags`. + +## Task Commits + +1. Task 1 (tracer): `438144f` feat(25-07): filter the Watchlist by any of several tags +2. Task 2 RED: `a10a412` test(25-07): pin the tag filter's keep-open, keys, and empty copy +3. Task 2 GREEN: `5267ea8` feat(25-07): keep the tag filter open on pick and explain empty results +4. Task 3 RED: `4fb8ef1` test(25-07): pin tag filter validation, Manage tags sync, and Back +5. Task 3 GREEN: `beef586` feat(25-07): keep the tag filter consistent with Manage tags, Back, and first load + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Esc on a closed popup cleared every selected tag** +- **Found during:** Task 2 (RED test "keeps the selection when Esc is pressed with the popup closed") +- **Issue:** base-ui fires `onValueChange` with reason `escape-key` and an empty list when Escape is pressed in the input with the popup closed, so the second Esc wiped the filter. The plan's "a second Esc does nothing" requires otherwise. +- **Fix:** `onValueChange` cancels the `escape-key` reason before calling `onChange`. +- **Files modified:** `web/app/components/watchlist/WatchlistTagFilter.tsx` +- **Commit:** `5267ea8` + +### Notes + +**2. [Assumption result] A1 and A2 both held** +- The default filter is a case-insensitive contains, and multiple mode keeps the popup open on pick without help. The `item-press` close cancel (required by the plan's acceptance grep) is kept as a defensive pin; no controlled `open` state or explicit `filter` was needed. + +**3. [Test harness] Accessible name of the chips input** +- The input carries both `aria-labelledby="watchlist-tags-label"` and `aria-label="Filter by tags, any of"` as the UI-SPEC specifies, so its computed role name is `Tags: any of`. Tests locate it with `getByLabelText("Filter by tags, any of")`. + +**4. [Test harness] Open popup hides the page from role queries** +- While the combobox or Manage tags dialog is open, the rest of the page is aria-hidden, so route tests that read cards or filter chips behind them query with `hidden: true`. + +**5. [Scope] `%2C` in the URL** +- `URLSearchParams` serializes the comma, so multi-tag URLs read `?tags=5%2C9`. A hand-typed `?tags=5,9` is normalized to that form with a single replace on load. Single-tag URLs (`?tags=3`) are unaffected. + +**6. [TDD ordering] View-model cases and the detach/refresh route cases were green on first run** +- The Task 2 view-model tests and Task 3's detach-keeps-filter and add-refresh-keeps-filter tests pin behavior already delivered by Task 1's tracer and the 25-04 sticky design. The genuinely RED tests (empty copy, Esc clears chips, first-load validation, delete/merge sync, Back) drove the GREEN commits. + +**Total deviations:** 1 auto-fixed, 5 notes. **Impact:** none on scope. + +## Verification + +- `corepack pnpm test` (full web suite with coverage): 30 files, 539 tests passed +- `corepack pnpm run typecheck`: exit 0 +- `prettier --check "**/*.{ts,tsx}"`: clean +- Acceptance greps: `opacity-50 hover:opacity-100` x0, `removeLabel` x3 in `combobox.tsx`, `multiple` present and `TagCombobox` x0 in `WatchlistTagFilter.tsx`, `id="watchlist-tags-label"` x1, `item-press` present, `No tags yet — add one with` x1, `No tags match` x1, `tagIdsValidatedRef` x4, `retiredTagIdsRef` x4 +- Pending human check: measure the toolbar chip x against the `#27272a` chip fill (expected about 5.8:1), confirm a selected tag survives an add-and-refresh in the running app, and eyeball a 32-character tag at 375px. Deferred to end-of-phase UAT. + +## Known Stubs + +None. + +## Threat Flags + +None. T-25-20 (200-token cap, shape filter, first-load intersection), T-25-21 (tag names only as JSX text and escaped attributes) and T-25-22 (filter never calls attach/detach/delete; Manage tags only reports) are mitigated as planned. + +## Self-Check: PASSED + +- FOUND: web/app/components/watchlist/WatchlistTagFilter.tsx and WatchlistTagFilter.test.tsx +- FOUND commits 438144f, a10a412, 5267ea8, 4fb8ef1, beef586 +- All task acceptance criteria re-run and passing diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-08-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-08-SUMMARY.md new file mode 100644 index 00000000..699fdec4 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-08-SUMMARY.md @@ -0,0 +1,143 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 08 +subsystem: web +tags: [watchlist, preference-filter, tag-chips, url-state, react, vitest] +requires: + - phase: 25-04 + provides: "useUrlParams writer, sticky set, Clear filters deleting muted/custom via CLEAR_FILTER_KEYS" + - phase: 25-07 + provides: "tags URL param, handleTagsChange, tagNamesRef, toolbar tag filter" +provides: + - "watchlistView.ts: DEFAULT_RELEASE_TYPES, muted/custom view axes, hasMutedEvents, hasCustomReleaseTypes, AND composition in selectVisible and isFiltered" + - "WatchlistToolbar: Show only fieldset with two aria-pressed toggles" + - "TagChips: chip label is a Filter by button; data-chip-remove focus targeting; TagActions.filterTagIds and toggleFilterTag" + - "Route: handleFlagChange for muted/custom, toggleFilterTag, filterTagIds set passed to each row" +affects: [] +tech-stack: + added: [] + patterns: + - "Chip focus logic selects [data-chip-remove] so extra buttons inside a chip cannot shift the index" + - "Boolean URL flags honor only the value 1; canonicalization effect rewrites anything else away with replace" +key-files: + created: [] + modified: + - web/app/lib/watchlistView.ts + - web/app/lib/watchlistView.test.ts + - web/app/components/watchlist/WatchlistToolbar.tsx + - web/app/components/watchlist/WatchlistToolbar.test.tsx + - web/app/components/watchlist/TagChips.tsx + - web/app/components/watchlist/TagChips.test.tsx + - web/app/routes/watchlist.tsx + - web/app/routes/watchlist.test.tsx +key-decisions: + - "Custom release types is a set comparison against the default four, so order and duplicates are ignored and an empty list counts as custom" + - "The existing canonicalization effect owns muted and custom too; no separate cleanup effect" + - "Chip label click reuses handleTagsChange (push, clear sticky, count announcement) and seeds tagNamesRef first so the toolbar chip has its name" +requirements-completed: [WLVW-05, WLVW-04, WLVW-06] +status: complete +commits: 4 +plan_head_before: 3aea8c3ab3caa79d0d0b22f897661f833f681b24 +actuals: + tokens: 28000 + tasks: 2 + commits: 4 +duration: 25 min +completed: 2026-10-10 +coverage: + - deliverable: "Has muted events and Custom release types toggles narrow the list, round-trip through the URL, keep focus, and AND-compose with name and tags" + verification: + - kind: test + ref: "web/app/routes/watchlist.test.tsx#Watchlist preference toggles (WLVW-05)" + status: pass + - kind: test + ref: "web/app/lib/watchlistView.test.ts#muted and custom axes" + status: pass + - kind: test + ref: "web/app/components/watchlist/WatchlistToolbar.test.tsx#WatchlistToolbar Show only" + status: pass + human_judgment: false + - deliverable: "In-card edit keeps a card that stopped matching until the next filter, sort, or search change" + verification: + - kind: test + ref: "web/app/routes/watchlist.test.tsx#keeps a card whose last mute was just unchecked until the next filter change" + status: pass + human_judgment: false + - deliverable: "Chip label toggles the shared tag filter without attach or detach; x still detaches; Phase 24 chip focus contract holds" + verification: + - kind: test + ref: "web/app/components/watchlist/TagChips.test.tsx#chip label filter toggle (D-08)" + status: pass + - kind: test + ref: "web/app/routes/watchlist.test.tsx#Watchlist card chip click-to-filter (D-08)" + status: pass + human_judgment: false + - deliverable: "Scroll position holds on chip and toggle clicks in a 50+ artist list; pressed-toggle Check meets 3:1 against #27272a; 32-character label with Check truncates at 375px" + verification: [] + human_judgment: true + rationale: "Scroll restoration, measured contrast, and truncation at width are only observable in a browser; deferred to end-of-phase UAT per the plan's human-check" +--- + +# Phase 25 Plan 08: Preference toggles and chip click-to-filter Summary + +The Watchlist gains `Show only` toggles for `Has muted events` and `Custom release types` (URL params `muted=1` / `custom=1`, AND-composed with name, tags, and sort), and each card's tag chip label becomes a one-click `Filter by {tag}` toggle that shares the toolbar's `tags` filter while the x remains the only detach. + +## Accomplishments + +- `watchlistView.ts` adds `DEFAULT_RELEASE_TYPES`, the `muted`/`custom` view fields (only `1` is honored), `hasMutedEvents`, and `hasCustomReleaseTypes` (set comparison). `selectVisible` ANDs both axes and `isFiltered` counts them, so `Clear filters` shows for a lone toggle. +- `WatchlistToolbar` renders a `Show only` fieldset between the tag filter and `Clear filters`: two default-height `Button`s, `outline` off and `secondary` on, `aria-pressed`, and a leading `Check` only when on. +- The route writes the flags with push, clears sticky cards, and schedules the count announcement. The existing canonicalization effect now also drops invalid `muted`/`custom` values with replace. +- `TagChips` focus now targets `[data-chip-remove]` (both confirmed and pending x buttons), so the new label buttons cannot shift focus on removal. All Phase 24 focus tests stayed green. +- Confirmed chips carry a `Filter by {tag}` button with `aria-pressed`; an active tag shows a `Check` and `ring-1 ring-inset ring-foreground/30`. Pending chips keep plain-text labels. +- The route passes `filterTagIds` and `toggleFilterTag` on `tagActions`; the label click reuses `handleTagsChange` and seeds `tagNamesRef` first. + +## Task Commits + +1. Task 1 (tracer): `d888568` feat(25-08): filter the Watchlist to muted events and custom release types +2. Task 2 refactor: `e9e8370` refactor(25-08): target chip focus by data-chip-remove and wire tag filter actions +3. Task 2 RED: `30cdd64` test(25-08): pin the chip label filter toggle and its separation from detach +4. Task 2 GREEN: `f7d460b` feat(25-08): let a card's tag chip label toggle the tag filter + +## Deviations from Plan + +### Auto-fixed Issues + +None. + +### Notes + +**1. [Ordering] Focus fix and action wiring landed as a behavior-neutral refactor commit before RED** +- The plan puts the `data-chip-remove` fix first with `TagChips.test.tsx` green before touching markup. Making `filterTagIds` and `toggleFilterTag` required on `TagActions` forced the route and existing test helpers to change at the same time, so that step became its own `refactor` commit. The full chip and route suites were green on it. + +**2. [Test harness] Tailwind class ordering** +- `prettier-plugin-tailwindcss` reorders the active-ring classes, so the test asserts each of `ring-1`, `ring-inset`, `ring-foreground/30` on the badge classList rather than one joined string. + +**3. [Test harness] Checkbox accessible names** +- The mute checkbox name includes its visible label, so the route test queries it with `/^Mute guest feature alerts/`. + +**4. [Unresolved by design] WLVW-05 edge row and sticky chip deselect** +- Per the plan's flagged assumptions, the WLVW-05 edge row stays unresolved; empty sets, set equality, and AND composition are pinned by unit tests. When a chip click deselects a tag on a card that was visible only because it was sticky, the card can leave the view and focus is left to the browser. Flagged for UI review. + +**Total deviations:** 0 auto-fixed, 4 notes. **Impact:** none on scope. + +## Verification + +- `corepack pnpm test` (full web suite with coverage): 30 files, 566 tests passed; coverage 95.18 / 90.57 / 95.75 / 97.18 +- `corepack pnpm run typecheck`: exit 0 +- `prettier --check "**/*.{ts,tsx}"`: clean +- Acceptance greps: `Has muted events` x1 and `Custom release types` x1 in the toolbar, `aria-pressed` present, `export function hasCustomReleaseTypes` x1, `data-chip-remove` x3, `Filter by ` x1, `toggleFilterTag` x2 in `TagChips.tsx` +- Pending human check: scroll position on 50+ artists, adding an artist under an active toggle, `Check` contrast against `#27272a`, and a 32-character tag with `Check` at 375px. Deferred to end-of-phase UAT. + +## Known Stubs + +None. + +## Threat Flags + +None. T-25-23 (label only calls `toggleFilterTag`; tests assert `attachTag`/`detachTag` are never called from it; distinct names and hover fills), T-25-24 (only `1` honored, anything else rewritten away with replace), and T-25-25 (tag names only as JSX text and escaped attributes) are mitigated as planned. + +## Self-Check: PASSED + +- FOUND commits d888568, e9e8370, 30cdd64, f7d460b +- FOUND modified files listed in key-files +- All task acceptance criteria re-run and passing diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-09-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-09-SUMMARY.md new file mode 100644 index 00000000..9d41326d --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-09-SUMMARY.md @@ -0,0 +1,159 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 09 +subsystem: web +tags: [history, url-state, react-router, filters, vitest] +requires: + - phase: 25-04 + provides: "useUrlParams single URL writer; routeStub/LocationProbe test seam" + - phase: 25-06 + provides: "History Tag filter, degraded Tag trigger labels, tag empty state" +provides: + - "historyParams: HISTORY_EVENT_TYPES, parseHistoryParams, canonicalHistoryParams" + - "History Artist/Event type/Tag filters backed by artist/type/tag URL params (push on change)" + - "Once-per-source stale tag and artist cleanup with replace; never on a failed fetch" + - "HistoryFilters onArtistsLoaded prop and Loading artists… / Artist unavailable trigger labels" +affects: [25-10] +plan_head_before: b1ff19cac5ea8a2a17efc62ac964a0103307337d +actuals: + tokens: 4100 + tasks: 2 + commits: 3 +tech-stack: + added: [] + patterns: + - "Latest-callback ref for a mount-once effect that reports upward (onArtistsLoadedRef)" + - "Canonicalization effect rewrites only owned keys with replace; foreign keys untouched" +key-files: + created: + - web/app/lib/historyParams.ts + - web/app/lib/historyParams.test.ts + modified: + - web/app/routes/history.tsx + - web/app/routes/history.test.tsx + - web/app/components/history/HistoryFilters.tsx + - web/app/components/history/HistoryFilters.test.tsx +key-decisions: + - "Artist id check runs inside onArtistsLoaded against a filtersRef, so it sees the URL at load time, not at first render" + - "Stale-id cleanup is gated by tagIdChecked/artistIdChecked refs: one check per source per mount" +requirements-completed: [HIST-02] +duration: 4 min +completed: 2026-10-10 +status: complete +commits: 3 + +coverage: + - id: D1 + description: "History filters live in the URL: reload with a tag reproduces the filtered feed before GET /tags resolves; control changes push and keep other params" + requirement: HIST-02 + verification: + - kind: unit + ref: "web/app/routes/history.test.tsx#History URL state (HIST-02)" + status: pass + human_judgment: false + - id: D2 + description: "Malformed artist/type/tag params parse to null and are dropped with replace; foreign keys kept" + requirement: HIST-02 + verification: + - kind: unit + ref: "web/app/lib/historyParams.test.ts" + status: pass + - kind: unit + ref: "web/app/routes/history.test.tsx#drops malformed params with replace and never requests with them" + status: pass + human_judgment: false + - id: D3 + description: "Stale tag/artist ids dropped once their source loads; a failed or pending fetch never drops them and the trigger says Tag unavailable / Artist unavailable / Loading ..." + requirement: HIST-02 + verification: + - kind: unit + ref: "web/app/routes/history.test.tsx#History stale and malformed params (D-13)" + status: pass + - kind: unit + ref: "web/app/components/history/HistoryFilters.test.tsx#Artist source states" + status: pass + human_judgment: false + - id: D4 + description: "Multi-page History across the retention edge with a real dataset; Back/forward restores the previous filter combination in a real browser" + verification: [] + human_judgment: true + rationale: "VALIDATION.md manual-only item: needs a real dataset and real browser history; not assertable in jsdom" +--- + +# Phase 25 Plan 09: History Filters in the URL Summary + +**History's Artist, Event type, and Tag filters now live in `artist`/`type`/`tag` URL params so a filtered view survives reload and Back, with malformed or stale ids dropped via replace once their source has loaded and never because a fetch failed.** + +## Performance + +- **Duration:** 4 min +- **Started:** 2026-10-10T16:57:33Z +- **Completed:** 2026-10-10T17:01:00Z +- **Tasks:** 2 +- **Files modified:** 6 (2 created) + +## Accomplishments +- `historyParams.ts` parses `artist`/`tag` with `^[1-9]\d*$` and `type` against a three-value allow-list; `canonicalHistoryParams` rewrites only those keys. +- `history.tsx` reads filters from `useUrlParams()` (the `useState` is gone); control changes and `Show all tags` write with push, and page one fetches with `tag_id` from the URL while `GET /tags` is still pending. +- A canonicalization effect drops malformed params with replace; `tagIdChecked`/`artistIdChecked` refs drop a stale tag or artist once, only after a successful load. +- `HistoryFilters` gained `onArtistsLoaded` (success only) and Artist trigger labels `Loading artists…` / `Artist unavailable`, mirroring the locked Tag copy. +- Added the `this tag` fallback test that 25-06 deferred: with `GET /tags` failed and `tag=3` in the URL the empty state reads "No events for “this tag”". + +## Task Commits + +1. **Task 1: Reload History with a tag filter in the URL (tracer)** - `7bafd67` (feat) +2. **Task 2: Drop malformed and stale History params (TDD)** - `e050de4` (test, RED), `578b5fc` (feat, GREEN) + +**Plan metadata:** committed with this summary (docs). + +## Files Created/Modified +- `web/app/lib/historyParams.ts` - parse/canonical helpers and the event-type allow-list +- `web/app/lib/historyParams.test.ts` - parse and canonical cases +- `web/app/routes/history.tsx` - URL-backed filters, canonicalization effect, once-per-source stale checks +- `web/app/routes/history.test.tsx` - URL-state and stale/malformed route tests (adds a `sonner` mock to assert no toast) +- `web/app/components/history/HistoryFilters.tsx` - `onArtistsLoaded`, `artistsStatus`, Artist trigger label +- `web/app/components/history/HistoryFilters.test.tsx` - Artist source-state tests + +## Decisions Made +- The artist stale check reads `filtersRef` inside `onArtistsLoaded`, so it uses the URL at load time rather than the first render's closure; `HistoryFilters` keeps the latest callback in a ref so the mount-once fetch effect never goes stale. +- Cleanup is once per source per mount; a later Back to a URL carrying a stale id is not re-checked (matches the plan's "once-per-source" wording). + +## Deviations from Plan + +### Auto-fixed Issues + +None - plan executed exactly as written. + +### Other notes + +- **TDD RED partially green:** `parseHistoryParams`/`canonicalHistoryParams` shipped in Task 1 as the plan's interfaces block requires, so `historyParams.test.ts` and the "tag kept on GET /tags failure" and `this tag` route tests passed at RED time. The other nine new tests failed on assertions as intended. +- **Copy outside the locked contract (plan flag A3):** `Loading artists…` / `Artist unavailable` have no UI-SPEC row. Flag for `/gsd-ui-review`. + +--- + +**Total deviations:** 0 auto-fixed +**Impact on plan:** None. + +## Issues Encountered +None. + +## Known Stubs +None. + +## Threat Flags +None. T-25-26/27/28 mitigations are in place: only `^[1-9]\d*$` ids and the three-value type allow-list survive parsing, and param values are never rendered. + +## User Setup Required +None - no external service configuration required. + +## Next Phase Readiness +- Manual-only check remains for UAT: pick a tag spanning the retention window, press Load more to the end, reload, then Back (VALIDATION.md). +- Full web suite: 29 files, 506 tests pass; typecheck and prettier --check clean. + +## Self-Check: PASSED +- Created/modified files exist; commits `7bafd67`, `e050de4`, `578b5fc` present (3 measured from the plan ledger). +- Acceptance greps: `useUrlParams()` 1, `useState` 0, `export function parseHistoryParams` 1, `onArtistsLoaded` 5, `Artist unavailable` 1, `tagIdChecked` 3. + +--- +*Phase: 25-find-filter-watchlist-and-history* +*Completed: 2026-10-10* diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-10-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-10-SUMMARY.md new file mode 100644 index 00000000..8153a4f7 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-10-SUMMARY.md @@ -0,0 +1,92 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 10 +subsystem: embedded-spa-and-phase-gate +tags: [go-embed, spa-bundle, definition-of-done, phase-gate] +requires: [25-02, 25-08, 25-09] +provides: + - "Rebuilt internal/webassets/build/client carrying the Watchlist toolbar, latest-release lines, and History Tag control" + - "Green Definition of Done evidence for Phase 25" +affects: [internal/webassets/build/client] +tech-stack: + added: [] + patterns: ["make web recipe run directly (frozen install, build, replace) when make/pnpm are not on PATH"] +key-files: + created: [] + modified: + - internal/webassets/build/client +key-decisions: + - "No gate fix was needed, so Task 2 produced no commit" +status: complete +commits: 1 +plan_head_before: 4ec4cc761c7055c0065ac702a4d692040d0a749e +actuals: + tokens: 0 + tasks: 2 + commits: 1 +metrics: + completed: 2026-10-10 +--- + +# Phase 25 Plan 10: Rebuild Embedded SPA and Phase Gate Summary + +Rebuilt the go:embed bundle from Phase 25's `web/` so a Go-only build serves the new UI, then ran the full Definition of Done and the blast-radius check; everything is green. + +## Tasks + +| Task | Name | Commit | +| ---- | ---- | ------ | +| 1 | Rebuild embedded SPA (tracer) | c38d832 | +| 2 | Definition of Done and blast radius | no commit (no gate fix needed) | + +### Task 1: rebuild + +- Ran the `make web` recipe directly: `corepack pnpm --dir web install --frozen-lockfile`, `run build`, replace `internal/webassets/build/client`. +- The frozen install left `web/package.json` and `web/pnpm-lock.yaml` untouched. +- Bundle contains `Filter watchlist` and `Latest release:` (watchlist chunk) and `Show all tags` (history chunk). +- `go build ./...` passes. The commit swapped hashed assets (8 old files deleted, new ones added, `index.html` updated); the deletions are the superseded hashed bundle files, intended. + +### Task 2: gates + +| Gate | Result | +| ---- | ------ | +| `go vet ./...` | pass | +| `golangci-lint run` | 0 issues | +| Integration suite with coverage profile | all packages pass (see Deviations for the `covdata` note) | +| Backend coverage (`cmd/coverage-report --mode=total`) | 90.19% vs 80% floor | +| `sqlc generate` + `git diff --exit-code internal/db/sqlc/` | clean | +| `pnpm typecheck` | pass | +| `prettier --check "**/*.{ts,tsx}"` | pass | +| `pnpm test` | 30 files, 566 tests pass; statements 95.18%, branches 90.57%, functions 95.75%, lines 97.18% (all above 70%) | +| Blast radius vs `9030bfa` | see Deviations (one baseline artifact) | + +No migration, `go.mod`/`go.sum`, `web/package.json`, `web/app/root.tsx`, or notifier/detection/discord/musicbrainz/deezer/poller change. + +## Deviations from Plan + +**1. [Baseline artifact, not a Phase 25 change] `web/pnpm-lock.yaml` differs from `9030bfa`.** +The blast-radius diff against `9030bfa` lists `web/pnpm-lock.yaml`. Its only commit in that range is `7eaa989 feat(24): artist tags and notes (#9)`, a Phase 24 merge that landed after the planning-time head (pnpm overrides plus patch bumps for `@modelcontextprotocol/sdk`, `brace-expansion`, `proxy-addr`, `source-map-js`, `undici`). No Phase 25 commit touched the lockfile, and the frozen install in Task 1 left it unchanged, so T-25-SC holds. The `9030bfa` base for that file was stale; the true baseline is `7eaa989`. + +**2. [Environment] `go: no such tool "covdata"`.** +`go test ./... -coverpkg=...` prints this for `internal/testutil` (a package with no tests) on this Windows toolchain and returns non-zero for that package alone. Every package with tests passed, `coverage.out` was produced, and the coverage total (90.19%) was computed from it. CI's Linux job is the authoritative gate. `-race` was not run (cgo disabled); CI is the race gate. + +## Out of scope (pre-existing) + +`gofmt -l` still lists the five files already on main: `internal/artistart/match_test.go`, `internal/httpserver/server.go`, `internal/httpserver/settings_test.go`, `internal/notifier/timeout_test.go`, `internal/sqlscan/lex.go`. golangci-lint passes; left untouched. + +## Known Stubs + +None. + +## Threat Flags + +None. + +## Manual UAT carried to /gsd-verify-work (human-check, end-of-phase) + +Run the built binary against the dev DB with 50+ watched artists and walk the UI-SPEC UAT list: one request on open; `beyonce` finds Beyoncé; sort-direction stability with `No releases yet` last; reload/Back reproduce the view; toolbar chip x and pressed-toggle Check at 3:1 or better against `#27272a`; unchecking the last mute under `Has muted events` keeps the card until the next filter change; no scroll jump on chip clicks/toggles; adding an artist under `Has muted events` shows it; a selected tag survives the post-add refresh. On History, pick a tag and page with Load more across the retention edge. Plus UAT items deferred by plans 25-01..25-09. + +## Self-Check: PASSED + +- Commit c38d832 exists; `internal/webassets/build/client` contains the phase strings. +- Measured `commits: 1` from the persisted ledger (`4ec4cc7..HEAD`). diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-11-PLAN.md b/.planning/phases/25-find-filter-watchlist-and-history/25-11-PLAN.md new file mode 100644 index 00000000..b6c2c9c7 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-11-PLAN.md @@ -0,0 +1,247 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 11 +type: execute +wave: 1 +depends_on: [] +files_modified: + - web/app/lib/useScrollAnchor.ts + - web/app/lib/useScrollAnchor.test.tsx + - web/app/components/watchlist/TagChips.tsx + - web/app/components/watchlist/TagChips.test.tsx + - web/app/routes/watchlist.tsx + - web/app/routes/watchlist.test.tsx +autonomous: true +gap_closure: true +gap_ids: [G-25-4] +requirements: [WLVW-04] + +estimate: + tokens: 60000 + raw_tokens: 60000 + tasks: 2 + confidence: low + +must_haves: + truths: + - "G-25-4: after a chip-label click far down the Watchlist, the clicked chip, and so its card, keeps its on-screen position across the filter re-render. The chip's getBoundingClientRect().top is recorded before the tags param is written, and a layout effect keyed on the filter state scrolls the window by the difference before paint." + - "When the filtered page is shorter than the window, the browser clamps the scroll, so the clicked card ends where the clamp leaves it. The code never pads the page height and never calls scrollTo(0, 0) or scrollIntoView to fake the position." + - "Both directions anchor. Turning a tag on (the list shrinks) and turning it off (the list grows and cards appear above) both keep the clicked card in place." + - "If the clicked card leaves the filtered list (for example deselecting its only matching tag from an any-of selection), no scroll correction runs and nothing throws." + - "The anchor is consumed by exactly one filter re-render. A later sort, search, toggle, or Back never re-applies it, and a zero difference calls no scroll API." + - "The router still never resets scroll: useUrlParams and urlWriteOptions are unchanged and always pass preventScrollReset: true. The chip label still filters, keeps focus on itself, and never attaches or detaches (D-08)." + artifacts: + - path: web/app/lib/useScrollAnchor.ts + provides: "useScrollAnchor(key) returning a capture(el) function; a layout effect keyed on `key` scrolls by the anchor's on-screen drift" + exports: ["useScrollAnchor"] + - path: web/app/components/watchlist/TagChips.tsx + provides: "Chip label passes its own button element to toggleFilterTag as the scroll anchor" + contains: "e.currentTarget" + - path: web/app/routes/watchlist.tsx + provides: "toggleFilterTag captures the anchor before writing the tags param" + contains: "useScrollAnchor(filterKey)" + key_links: + - from: "web/app/components/watchlist/TagChips.tsx label button onClick" + to: "web/app/routes/watchlist.tsx toggleFilterTag" + via: "actions.toggleFilterTag(tag, e.currentTarget)" + pattern: "toggleFilterTag\\(tag, e\\.currentTarget\\)" + - from: "web/app/routes/watchlist.tsx toggleFilterTag" + to: "web/app/lib/useScrollAnchor.ts layout effect" + via: "captureScrollAnchor(anchor) -> write tags -> filterKey changes -> useLayoutEffect([key]) -> window.scrollBy(0, delta)" + pattern: "captureScrollAnchor\\(" + prohibitions: + - statement: "MUST NOT change web/app/lib/useUrlParams.ts or drop preventScrollReset from any URL write." + requirement_id: WLVW-04 + category: safety + verification: test + - statement: "MUST NOT fake scroll preservation by padding the document height (min-height on body or the list) or by calling window.scrollTo(0, 0) or scrollIntoView." + requirement_id: WLVW-04 + category: safety + verification: test + - statement: "No new npm package; web/package.json and web/pnpm-lock.yaml stay unchanged." + requirement_id: WLVW-04 + category: safety + verification: test +--- + + +Close UAT gap G-25-4 (major). Clicking a card's chip label far down a 50+ artist Watchlist drops `window.scrollY` to 0. + +Diagnosis (`.planning/debug/chip-click-scroll-reset.md`): nothing calls a scroll API, and `preventScrollReset` already works. The filter +collapses the list under the viewport. The document gets no taller than the window (body is `min-h-screen`), so the browser clamps +`scrollY` to `max(0, scrollHeight - innerHeight)`, which is 0. UI-SPEC D-12 treated `` as the only cause of a jump. + +User decision (2026-10-11, binding): anchor the clicked card. After a chip-label filter, the clicked chip and card keep their on-screen +position, and the browser clamps it when the page is short. The UI-SPEC D-12 and UAT truth rewording is done in 25-14 (shared doc +files), not here. + +Purpose: WLVW-04's chip click-to-filter (D-08) stops throwing the user's place away. +Output: `useScrollAnchor` hook, the chip label passing itself as the anchor, route wiring, and jsdom tests with stubbed layout. + +Browser-level evidence: the repo has no browser test runner, and adding Playwright would add an npm package this milestone rules out +(the 25-10 blast-radius gate pins `web/package.json`). The jsdom tests stub `getBoundingClientRect` and spy on `window.scrollBy` to pin +the arithmetic. The real-browser check is a `human-check` (human_verify_mode: end-of-phase) that re-runs UAT test 4. + +Commit discipline (CLAUDE.md): run `corepack pnpm --dir web exec prettier --write "**/*.{ts,tsx}"` before staging. Stage only this +plan's files (wave 1 runs in parallel with 25-12 and 25-13 in the same tree). Comments are 1-3 lines and cite G-25-4 at most once per +file. Hooks stay on, and no AI attribution trailer. + + + +@~/.claude/gsd-core/workflows/execute-plan.md +@~/.claude/gsd-core/templates/summary.md + + + +@.planning/phases/25-find-filter-watchlist-and-history/25-UAT.md +@.planning/debug/chip-click-scroll-reset.md +@web/app/routes/watchlist.tsx +@web/app/components/watchlist/TagChips.tsx +@web/app/lib/useUrlParams.ts + + + +Current shapes the executor builds on (verified at planning time): + +- `web/app/components/watchlist/TagChips.tsx`: the `TagActions` interface has `toggleFilterTag(tag: TagRef): void`, and the chip label + ``. +- `web/app/components/ui/button.tsx` base classes include `inline-flex shrink-0 items-center justify-center whitespace-nowrap`. It is + not edited. +- `SearchResultsColumns.test.tsx`: line ~163 resolves `getByRole("button", { name: "Search MusicBrainz to add" })` and checks it is + disabled. Line ~222-224 asserts the Deezer row's `textContent` is exactly `${deezerArtist.name}${deezerArtist.type}Search MusicBrainz to add`. + The fixture `deezerArtist` has `type: "Artist"`. + + + + + + Task 1: The Deezer row's artist name gets the row, and the button label truncates instead + web/app/components/watchlist/SearchResultsColumns.tsx, web/app/components/watchlist/SearchResultsColumns.test.tsx + + - `web/app/components/watchlist/SearchResultsColumns.tsx` lines 152-216 (`SearchResultRow`). + - `web/app/components/watchlist/SearchResultsColumns.test.tsx` lines 140-230 (the Deezer cases). + - `.planning/debug/search-input-squeezed-375.md` Evidence (the measured widths and experiments A-G). + + + - New test "caps the Deezer button and truncates its label so the name keeps the row (G-25-9)": render a Deezer-only response. `getByRole("button", { name: "Search MusicBrainz to add" })` resolves, its classList contains `max-w-[40%]` and `lg:max-w-none`, and it has exactly one child `span` with class `truncate` whose textContent is `Search MusicBrainz to add`. The button keeps the existing `title` text. + - The existing tests at ~163 (disabled) and ~222 (row textContent) pass unchanged. + + +Write the failing test first, then change only the Deezer branch of `SearchResultRow`: +- Add `max-w-[40%] lg:max-w-none` to its `className`, keeping `shrink-0`. The percentage resolves against the `li`'s content box, and a + max-width clamps the flex item's hypothetical size, which the debug run proved works where shrinkability alone did not. +- Wrap the label text in `Search MusicBrainz to add`. With `truncate`'s overflow hidden, the span's + automatic minimum width is 0, so it shrinks inside the inline-flex button and shows an ellipsis. +- Keep the `title`, `variant`, `size`, and `disabled` props as they are. The visible copy doesn't change, so the accessible name stays the + full label. + +Add at most a one-line comment citing G-25-9. Don't edit `ui/button.tsx`, the `Add to Watchlist` or `Already watching` branches, or the +long existing doc comment block above the function. + + + corepack pnpm --dir web exec vitest run app/components/watchlist/SearchResultsColumns.test.tsx --coverage.enabled=false + corepack pnpm --dir web run typecheck + + + - The G-25-9 test fails before the change and passes after it. The existing Deezer tests pass unchanged. + - `git diff --name-only -- web/app/components/ui/button.tsx` prints nothing. + - `grep -c 'max-w-\[40%\] lg:max-w-none' web/app/components/watchlist/SearchResultsColumns.tsx` prints 1 (after prettier, the two classes may sit apart; if so, check each class appears once in the Deezer Button's className). + + The Deezer button no longer starves the artist name. Committed. + + + + Task 2: Truncate the type line and confirm the other trailing controls are untouched + web/app/components/watchlist/SearchResultsColumns.tsx, web/app/components/watchlist/SearchResultsColumns.test.tsx + + - `web/app/components/watchlist/SearchResultsColumns.tsx` line 178 (the type span). + + + - New test: in a Deezer row, the element with text `Artist` (the type) has class `truncate`. In a MusicBrainz row, the type element (`Group`) also has class `truncate`. + - New test: the `Add to Watchlist` button and the `Already watching` button carry no `max-w-[40%]` class, and their direct text is unchanged (no inner span required). + - Every existing assertion passes unchanged. + + +Add `truncate` to the type span's className (`text-label text-muted-foreground` becomes `truncate text-label text-muted-foreground`; prettier +settles the order). The type comes from a third-party catalogue and can be long. Truncating it matches the name and secondary spans, and +the debug screenshot showed it running under the button. Write the two tests above. Run prettier on `web/` before staging, then commit +with hooks enabled. + + + corepack pnpm --dir web exec vitest run app/components/watchlist/SearchResultsColumns.test.tsx app/routes/watchlist.test.tsx --coverage.enabled=false + corepack pnpm --dir web run typecheck && corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}" + + Run the app, search an artist whose Deezer column has results (for example `Bad Bunny`), and view the results at 375px, 640px, and 1024px viewports (DevTools device toolbar). + At 375px and 640px each Deezer row shows most of the artist name next to a truncated `Search Music…` pill. The type line truncates and never runs under the pill, and nothing scrolls sideways. At 1024px the full `Search MusicBrainz to add` label shows. MusicBrainz rows look as before. + jsdom has no layout. Widths need a real browser (UAT test 9, human_verify_mode: end-of-phase). + + + + - Both new tests pass, and every existing SearchResultsColumns and watchlist route test passes unchanged. + - Prettier check and typecheck exit 0. + + The Deezer row's text column truncates cleanly at narrow widths, and the visual check is queued for UAT. Committed. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| none new | CSS class and markup change on already-rendered, third-party catalogue strings | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-25-36 | Tampering / XSS | SearchResultRow label and type spans | low | accept | Still plain JSX text nodes. No new data source and no raw-HTML rendering. The change is class names plus a wrapping span | +| T-25-37 | Repudiation / UX safety | Disabled Deezer button | low | mitigate | Accessible name and title unchanged. The button stays disabled, so a Deezer id can never be posted as an mbid (Phase 6 guard untouched) | + + + +- `corepack pnpm --dir web exec vitest run app/components/watchlist/SearchResultsColumns.test.tsx app/routes/watchlist.test.tsx --coverage.enabled=false` passes. +- Typecheck and prettier check exit 0. `web/app/components/ui/button.tsx` is unchanged. + + + +At 375px and 640px, Deezer search results show readable artist names, and the MusicBrainz hint button truncates instead. The full label +stays accessible, and the shared Button base is untouched. + + + +Create `.planning/phases/25-find-filter-watchlist-and-history/25-13-SUMMARY.md` when done. + diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-13-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-13-SUMMARY.md new file mode 100644 index 00000000..84a48976 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-13-SUMMARY.md @@ -0,0 +1,124 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 13 +subsystem: ui +tags: [react, tailwind, vitest, watchlist, search, responsive] +requires: + - phase: 06-search-and-add + provides: SearchResultsColumns Deezer row with disabled "Search MusicBrainz to add" button +provides: + - Deezer search-result button capped at 40% of the row below lg, with a truncating label + - Truncating type line in search-result rows +affects: [25-UAT test 9 held-out 375/640/1024px visual check] +actuals: + tokens: 1500 + tasks: 2 + commits: 4 +plan_head_before: fc7d23df0d0954bbeeb7807658d40b9a9bca2003 +tech-stack: + added: [] + patterns: + - "Cap a trailing flex control with max-w-[N%] + inner truncate span so the flex-1 min-w-0 text column keeps the row" +key-files: + created: [] + modified: + - web/app/components/watchlist/SearchResultsColumns.tsx + - web/app/components/watchlist/SearchResultsColumns.test.tsx +key-decisions: + - "Cap is 40% of the row below lg and lifted at lg (1024px+); only the Deezer button changes, Add to Watchlist and Already watching stay as-is" +requirements-completed: [WLVW-01] +coverage: + - id: D1 + description: "Deezer row button is capped (max-w-[40%] lg:max-w-none) with its label in a truncating span; accessible name, title, and disabled state unchanged" + requirement: WLVW-01 + verification: + - kind: unit + ref: "web/app/components/watchlist/SearchResultsColumns.test.tsx#caps the Deezer button and truncates its label so the name keeps the row (G-25-9)" + status: pass + human_judgment: false + - id: D2 + description: "Type line truncates in Deezer and MusicBrainz rows; Add to Watchlist and Already watching buttons uncapped" + requirement: WLVW-01 + verification: + - kind: unit + ref: "web/app/components/watchlist/SearchResultsColumns.test.tsx#truncates the type line in Deezer and MusicBrainz rows (G-25-9)" + status: pass + - kind: unit + ref: "web/app/components/watchlist/SearchResultsColumns.test.tsx#leaves the Add to Watchlist and Already watching buttons uncapped" + status: pass + human_judgment: false + - id: D3 + description: "At 375px and 640px a Deezer row shows a readable artist name beside a truncated button label; from 1024px the full label shows" + requirement: WLVW-01 + verification: [] + human_judgment: true + rationale: "jsdom has no layout engine; real widths need a browser (UAT test 9, end-of-phase human verify)" +duration: 5min +completed: 2026-10-11 +status: complete +--- + +# Phase 25 Plan 13: Deezer Row Width Cap Summary + +**Deezer search-result button capped at 40% of the row below lg with a truncating label, plus a truncating type line, so the artist name no longer collapses to one letter at 375px (G-25-9)** + +## Performance + +- **Duration:** 5 min +- **Completed:** 2026-10-11T20:28:15Z +- **Tasks:** 2 +- **Files modified:** 2 + +## Accomplishments +- The disabled `Search MusicBrainz to add` button now has `max-w-[40%] shrink-0 lg:max-w-none` and its label sits in an inner ``. The name column gets the remaining row width. The accessible name, `title`, and disabled state are unchanged. +- The `artist.type` span now truncates like the name and secondary label, so it can't run under the button. +- `ui/button.tsx`, the `Add to Watchlist` branch, and the `Already watching` branch are untouched. + +## Task Commits + +1. **Task 1: Deezer row's name gets the row** - `ef6d74c` (test, RED), `a027206` (feat, GREEN) +2. **Task 2: Truncate the type line** - `71a9d4a` (test, RED), `1ef7bc5` (feat, GREEN) + +**Plan metadata:** committed separately (docs: complete plan) + +## Files Created/Modified +- `web/app/components/watchlist/SearchResultsColumns.tsx` - capped Deezer button with truncating label span, truncating type span +- `web/app/components/watchlist/SearchResultsColumns.test.tsx` - three new class-level tests for G-25-9 + +## Decisions Made +None - followed plan as specified. + +## Deviations from Plan + +None - plan executed exactly as written. + +## TDD Gate Compliance + +RED (`test(25-13)`) precedes GREEN (`feat(25-13)`) for both tasks. Task 1's RED failed on the `max-w-[40%]` class assertion. Task 2's type-line test failed on `truncate`. The "uncapped buttons" test in Task 2 passed on first run because it guards against regression rather than adding behavior. + +## Issues Encountered +None + +## Verification +- `vitest run SearchResultsColumns.test.tsx watchlist.test.tsx`: 82 passed +- `pnpm --dir web run typecheck` and `prettier --check "**/*.{ts,tsx}"`: exit 0 +- `git diff -- web/app/components/ui/button.tsx`: empty +- Tracer gate: automated verify re-run passed, so expansion continued. + +## Known Stubs +None. + +## Threat Flags +None. The change is class names plus a wrapping span on already-rendered text (T-25-36 accepted; T-25-37 mitigated, since the button stays disabled with an unchanged accessible name and title). + +## User Setup Required +None - no external service configuration required. + +## Next Phase Readiness +The 375/640/1024px visual check (Task 2 `human-check`) is queued for UAT test 9 at end of phase. + +--- +*Phase: 25-find-filter-watchlist-and-history* +*Completed: 2026-10-11* + +## Self-Check: PASSED diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-14-PLAN.md b/.planning/phases/25-find-filter-watchlist-and-history/25-14-PLAN.md new file mode 100644 index 00000000..6fa151ce --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-14-PLAN.md @@ -0,0 +1,301 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 14 +type: execute +wave: 2 +depends_on: [25-11, 25-12, 25-13] +files_modified: + - internal/webassets/build/client + - .planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md + - .planning/phases/25-find-filter-watchlist-and-history/25-UAT.md + - .planning/phases/25-find-filter-watchlist-and-history/25-VERIFICATION.md +autonomous: true +gap_closure: true +gap_ids: [G-25-4, G-25-7, G-25-9] +requirements: [WLVW-01, WLVW-04, HIST-02] + +estimate: + tokens: 50000 + raw_tokens: 50000 + tasks: 3 + confidence: low + +must_haves: + truths: + - "The embedded SPA in `internal/webassets/build/client` is rebuilt from the gap-closure `web/`, so a Go-only build serves the chip scroll anchor (G-25-4), the History `Detected` line (G-25-7), and the capped Deezer button (G-25-9)." + - "25-UI-SPEC D-12 says the router never resets scroll (preventScrollReset) and that a chip-label click anchors the clicked card, which keeps its on-screen position, clamped by the browser on short pages. The UAT carry item says the same." + - "25-UI-SPEC's History section and Copywriting table say retention counts from detection (`created_at`, Phase 10 D-01), not the release date, and that each card shows `Detected {date}`." + - "25-UAT tests 4, 7, and 9 and their G-25-4, G-25-7, and G-25-9 gap truths carry the reworded contracts. Test 7 says retention counts from detection and asks for a re-run with EVENT_RETENTION_DAYS=45. No `result:`, `status:`, or Summary count is changed, because /gsd-verify-work owns those." + - "G-25-4's `decision:` line equals its `461267f` form with only the final closing double quote moved after the user's 2026-10-11 addendum (every original character kept verbatim, checked by an exact full-line match): for the muted toggle, test 4 promises only that the router never resets scroll, and the browser clamp on a short filtered list is a documented, expected limitation." + - "25-VERIFICATION human checks 4 and 7 carry the same rewording in both the frontmatter `human_verification` list and the body list: check 4's expected says the router never resets scroll, check 7's expected says retention counts from detection and its test uses EVENT_RETENTION_DAYS=45. `status: human_needed` and `score:` are unchanged." + - "Every Definition of Done gate passes: go vet, golangci-lint, the integration suite with a coverage profile, the 80% backend coverage gate, sqlc-check (with its sqlc-version-check guard first), typecheck, prettier check, and the web suite at 70% or above on all four axes. Race runs only in CI, an exception the user accepted on 2026-10-11 and the SUMMARY records." + - "Since the gap-closure planning head `461267f`, nothing changed under queries/, internal/db/, internal/events/, internal/db/migrations, go.mod, go.sum, web/package.json, web/pnpm-lock.yaml, web/app/components/ui/, web/app/lib/useUrlParams.ts, or web/app/root.tsx." + artifacts: + - path: internal/webassets/build/client + provides: "Rebuilt go:embed SPA bundle carrying the three gap fixes" + - path: .planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md + provides: "D-12 scroll-anchor contract and the detection-date retention wording" + contains: "anchors the clicked" + key_links: + - from: "web/ (plans 25-11, 25-12, 25-13)" + to: "internal/webassets/build/client" + via: "make web recipe (frozen install, build, replace) -> the committed tree go:embed serves" + pattern: "Detected" + - from: "25-UAT.md tests 4/7/9 expected wording" + to: "/gsd-verify-work re-verification" + via: "gap_ids reconciliation on resume" + pattern: "gap_id: G-25-" + prohibitions: + - statement: "MUST NOT change any `result:`, `status:`, `severity:`, `root_cause:`, or Summary count in 25-UAT.md, and MUST NOT alter any character of an existing `decision:` value. Only the `expected:` lines of tests 4, 7, and 9 and the three gap `truth:` lines are reworded, plus one addendum appended to the end of G-25-4's `decision:` value, whose closing double quote moves after the addendum (user decision 2026-10-11)." + requirement_id: HIST-02 + category: safety + verification: test + - statement: "No migration, Go module, or npm package change, and no edit to the retention SQL (blast radius against 461267f)." + requirement_id: HIST-02 + category: safety + verification: test +--- + + +Ship the three UAT gap fixes. Rebuild the embedded SPA so the Go binary serves them, reword the phase's spec, UAT, and verification +contracts as the user's 2026-10-11 decisions require, and run the full Definition of Done with a blast-radius check. + +The doc rewording lives here, not in 25-11/12/13, because all three gaps touch the same three files (25-UI-SPEC.md, 25-UAT.md, +25-VERIFICATION.md). Same-wave plans can't share files, and putting the docs here keeps 25-11, 25-12, and 25-13 parallel. + +User decisions (binding, from 25-UAT.md `decision:` fields): +- G-25-4: reword the truth to "the router never resets scroll and the clicked card stays where it was on screen". Update UI-SPEC D-12 + and the UAT truth. Addendum (user, 2026-10-11): narrow the muted toggle. Test 4 promises only that the router never resets scroll on + it, and the browser clamp on a short filtered list is a documented, expected limitation. Record this as an addendum appended to + G-25-4's `decision:` in 25-UAT.md (Task 2). +- G-25-7: keep D-01. Reword UAT test 7 and 25-VERIFICATION human check 7 to say retention counts from detection. +- G-25-9: no doc decision. Test 9's expected line is clarified to name the element the diagnosis found (the Deezer result row's artist + name, not the search input), and the UI-SPEC "unchanged" note for SearchResultsColumns gets a G-25-9 exception. + +Tooling on this box: `make` and bare `pnpm` are not on the bash PATH, and `golangci-lint` and `sqlc` live in `$HOME/go/bin`. Run each +Makefile target's underlying commands (25-10 precedent), including each target's prerequisites (`sqlc-check` runs `sqlc-version-check` +first). `go test -race` doesn't work on this Windows box (cgo disabled). The user accepted CI-only race for this gap-closure run +(2026-10-11): the local run omits `-race`, CI's Linux job is the race gate, and the SUMMARY must record this accepted exception with that +date. `go: no such tool "covdata"` for `internal/testutil` is a known environment quirk (25-10 SUMMARY deviation 2). + +Output: the refreshed `internal/webassets/build/client`, the three reworded docs, and a green gate run. + + + +@~/.claude/gsd-core/workflows/execute-plan.md +@~/.claude/gsd-core/templates/summary.md + + + +@.planning/phases/25-find-filter-watchlist-and-history/25-UAT.md +@.planning/phases/25-find-filter-watchlist-and-history/25-10-PLAN.md +@.planning/phases/25-find-filter-watchlist-and-history/25-10-SUMMARY.md +@Makefile + + + + + + Task 1: Rebuild the embedded SPA and prove the Go-served bundle carries all three fixes + Plans 25-11, 25-12, and 25-13 are committed (`git status --porcelain web/ internal/httpserver/` prints nothing), and `corepack pnpm --dir web install --frozen-lockfile` can resolve from the local store or the registry. + internal/webassets/build/client + + - `Makefile` `web` target (lines 139-144): frozen install, build, then replace `internal/webassets/build/client`. + - `.planning/phases/25-find-filter-watchlist-and-history/25-10-PLAN.md` Task 1 (the same rebuild run without `make`). + + +Run the `make web` recipe directly: +- `corepack pnpm --dir web install --frozen-lockfile` +- `corepack pnpm --dir web run build` +- `rm -rf internal/webassets/build/client` +- `mkdir -p internal/webassets/build` +- `cp -r web/build/client internal/webassets/build/client` + +If the install modifies `web/package.json` or `web/pnpm-lock.yaml`, restore both with `git checkout --` and stop, because the frozen +lockfile must not drift. + +Confirm the built assets carry each fix by comparing marker counts against the pre-change build committed at `461267f` (the embedded +tree is unchanged between `461267f` and the start of this plan). A bare presence check is not enough: `Detected` already appears twice in +the vendor `jsx-runtime` chunk, and `scrollBy` is generic. Each marker's occurrence count under `internal/webassets/build/client/assets` +must be strictly greater than its count in the `461267f` tree: `Detected` (G-25-7, only 25-12 adds it), `scrollBy` (G-25-4, only 25-11 +adds it), and the escaped `max-w-\[40\%\]` selector (G-25-9, only 25-13 adds it). The vendor chunks can't account for an increase, +because the lockfile is frozen and the blast-radius check pins `web/package.json` and `web/pnpm-lock.yaml`. Then run `go build ./...` so the +embed compiles. Commit the refreshed tree with hooks enabled and no AI attribution trailer. The deleted files are the superseded hashed +assets, which is intended. + + + corepack pnpm --dir web install --frozen-lockfile && corepack pnpm --dir web run build && rm -rf internal/webassets/build/client && mkdir -p internal/webassets/build && cp -r web/build/client internal/webassets/build/client && out=$(git status --porcelain internal/webassets/build/client) && test -n "$out" + non-zero exit from the install, build, or copy; or `test -n` failing because the bundle didn't change + ok=1; git cat-file -e 461267f:internal/webassets/build/client/assets || ok=0; for m in 'Detected' 'scrollBy' 'max-w-\[40\%\]'; do bo=$(git grep -o -F -e "$m" 461267f -- internal/webassets/build/client/assets; test $? -le 1) || ok=0; b=$(printf '%s' "$bo" | grep -c .); n=$(grep -r -o -F -e "$m" internal/webassets/build/client/assets | wc -l); echo "$m: $b -> $n"; [ "$n" -gt "$b" ] || ok=0; done; [ "$ok" = 1 ] && go build ./... + non-zero exit: the `461267f` baseline tree can't be read (git error, never counted as zero); a marker's count in the rebuilt assets is not strictly greater than in that pre-change tree (the printed `before -> after` line names it; on the pre-change tree this check fails with `Detected: 2 -> 2`); or the embed doesn't compile + + + - `git status --porcelain internal/webassets/build/client` is non-empty before staging. + - Each of `Detected`, `scrollBy`, and `max-w-\[40\%\]` occurs more often under `internal/webassets/build/client/assets` than in the `461267f` tree, so each count rose because of its gap fix and not because of a vendor chunk. + - `git diff --name-only -- web/package.json web/pnpm-lock.yaml` prints nothing, and `go build ./...` exits 0. + + The committed go:embed tree serves all three gap fixes. Committed. + + + + Task 2: Reword the UI-SPEC, UAT, and verification contracts to match the user's decisions + .planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md, .planning/phases/25-find-filter-watchlist-and-history/25-UAT.md, .planning/phases/25-find-filter-watchlist-and-history/25-VERIFICATION.md + + - `25-UAT.md` (whole file: tests 4, 7, and 9 and the three `## Gaps` entries with their `decision:` fields). + - `25-UI-SPEC.md` line 188 (SearchBox + SearchResultsColumns "unchanged, D-01"), line 399 ("Scroll reset (D-12)"), line 414 ("Load more carries the same tag_id"), lines ~480-488 (Copywriting table), and line 639 (the scroll carry item). + - `25-VERIFICATION.md` frontmatter `human_verification` items 4 (lines ~98-100) and 7 (lines ~107-109), and the body "Human Verification Required" items 4 and 7 (lines ~234 and ~237). + - `.planning/phases/25-find-filter-watchlist-and-history/25-11-SUMMARY.md`, `25-12-SUMMARY.md`, `25-13-SUMMARY.md` (as built, especially the hook name and the Detected copy). + + +Use scoped `Edit` calls only. Never rewrite a whole file with `Write`. + +25-UI-SPEC.md: +- Line ~399: rename the bullet "Scroll reset and anchoring (D-12, G-25-4)". Keep the existing preventScrollReset sentence. Add that this + stops the router from resetting scroll, but a filter that shrinks the list can leave the page shorter than the old `scrollY`, and the + browser then clamps it. So a chip-label click also anchors the clicked chip: `useScrollAnchor` records its `getBoundingClientRect().top` + before the tags param is written, and a layout effect scrolls by the drift after the re-render. The clicked card keeps its on-screen + position, and on a short page the browser clamps it. Drop the old closing claim that preventScrollReset alone stops a 60-cards-down + chip click from jumping. +- Line ~414 ("Load more carries the same tag_id ..."): add that retention counts from detection (`created_at`, Phase 10 D-01), not the + release date. The feed orders by release date, so Load more can reach releases that are years old but were detected inside the window, + and each card shows `Detected {date}` so the user can see the date retention uses (G-25-7). Keep "the UI needs no retention logic of + its own". +- Copywriting table: after the `Upcoming line` row, add a row: History event card, detection line, `Detected {date}` (for example + `Detected 11 Oct 2026`), shown in a ` + + grep -c "anchors the clicked" .planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md && grep -c "Detected {date}" .planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md && grep -c "EVENT_RETENTION_DAYS=45" .planning/phases/25-find-filter-watchlist-and-history/25-UAT.md + any grep prints 0 (non-zero exit): a reworded contract is missing + f=.planning/phases/25-find-filter-watchlist-and-history/25-UAT.md; d=$(git diff -U0 461267f -- "$f") && test -n "$d" && test "$(printf '%s\n' "$d" | grep -cE '^[-+] *(result|status|severity|root_cause|total|passed|issues|pending|skipped|blocked):')" -eq 0 && test "$(printf '%s\n' "$d" | grep -cE '^- *decision:')" -eq 1 && test "$(printf '%s\n' "$d" | grep -cE '^\+ *decision:')" -eq 1 && base=$(git show 461267f:"$f") && orig=$(printf '%s\n' "$base" | grep -E '^ decision: "User chose \(2026-10-11\): anchor the clicked card\.') && test "$(printf '%s\n' "$orig" | wc -l)" -eq 1 && case "$orig" in *'"') ;; *) false ;; esac && want="${orig%?} Addendum (2026-10-11): user narrowed test 4 for the Has muted events toggle to promise only that the router never resets scroll. The browser clamp on a short filtered list is a documented, expected limitation, not a failure.\"" && grep -qxF -- "$want" "$f" + non-zero exit: git failed; 25-UAT.md is unchanged since `461267f`; a result, status, severity, root cause, or Summary count line was changed; more or fewer than one `decision:` line changed; the `461267f` G-25-4 decision line can't be found exactly once or doesn't end in a double quote; or no line in 25-UAT.md is exactly that original line minus its closing quote, plus one space, the addendum, and a closing quote (so any changed original character, a kept original quote, or a missing re-close fails) + v=.planning/phases/25-find-filter-watchlist-and-history/25-VERIFICATION.md; fm=$(awk '/^---$/{n++; next} n==1' "$v") && body=$(awk '/^---$/{n++; next} n>=2' "$v") && printf '%s\n' "$fm" | grep -qiE '^ expected: .*router never resets scroll' && printf '%s\n' "$fm" | grep -qiE '^ expected: .*retention counts from detection' && printf '%s\n' "$fm" | grep -qE '^ - test: .*EVENT_RETENTION_DAYS=45' && printf '%s\n' "$body" | grep -qiE '^4\. .*router never resets scroll' && printf '%s\n' "$body" | grep -qiE '^7\. .*retention counts from detection' && printf '%s\n' "$body" | grep -qE '^7\. .*EVENT_RETENTION_DAYS=45' && grep -qx 'status: human_needed' "$v" && vd=$(git diff -U0 461267f -- "$v") && test "$(printf '%s\n' "$vd" | grep -cE '^[-+](status|score):')" -eq 0 + non-zero exit: the frontmatter `human_verification` list (between the first two `---` lines) lacks an `expected:` with the router-never-resets-scroll wording, an `expected:` with the retention-counts-from-detection wording, or a `test:` with EVENT_RETENTION_DAYS=45; body item 4 lacks the router-never-resets-scroll wording; body item 7 lacks the retention-counts-from-detection wording or EVENT_RETENTION_DAYS=45; `status: human_needed` is gone; or a `status:`/`score:` line changed since `461267f`. On the unedited file this check fails (the phrases appear nowhere in it at `461267f`) + + + - UI-SPEC: the D-12 bullet names `useScrollAnchor` and the browser clamp. The Load more bullet names `created_at` and Phase 10 D-01. The Copywriting table has the `Detected {date}` row. Line ~188 notes the G-25-9 exception. The carry item reads "keeps the clicked card where it was on screen". + - UAT: tests 4, 7, and 9 `expected:` and the three gap `truth:` lines are reworded, and the diff touches no `result:`/`status:`/`severity:`/`root_cause:` line or Summary count. + - UAT: the only changed `decision:` line is G-25-4's, and it exactly equals its `461267f` form minus the closing double quote, plus one space, the user's 2026-10-11 addendum narrowing the muted toggle to "the router never resets scroll" (browser clamp on a short list documented as expected), and a closing double quote. + - VERIFICATION: human checks 4 and 7 are reworded in both the frontmatter and the body (4 says the router never resets scroll; 7 says retention counts from detection and uses EVENT_RETENTION_DAYS=45). `status: human_needed` and `score:` are unchanged. + + The phase's contracts describe the behavior the user chose, and /gsd-verify-work will re-test against them. Committed. + + + + Task 3: Run the Definition of Done and confirm the blast radius + The dev Postgres answers on localhost:5432 (`docker compose ps postgres` healthy). + internal/webassets/build/client + + - `.claude/CLAUDE.md` "Definition of Done" (gate order, and never `--no-verify`). + - `Makefile` `test-integration` (`COVER_PKGS`), `coverage-gate`, and `sqlc-check` recipes (lines 75-130). + - `.planning/phases/25-find-filter-watchlist-and-history/25-10-SUMMARY.md` Deviations (the covdata quirk and the lockfile baseline note). + + +Run the Definition of Done in order. If a gate goes red, fix it in the file of the plan that owns it (25-11, 25-12, or 25-13), then +re-run: +1. `go vet ./...` +2. `golangci-lint run` (from `$HOME/go/bin`) +3. The integration suite with a coverage profile, using the Makefile's `COVER_PKGS` equivalent (first verify command), without `-race`. + This is the user-accepted exception (2026-10-11): cgo is disabled on this Windows box, so race runs only in CI's Linux job. Record it in + the SUMMARY under an `Accepted exceptions` heading, as the line `Accepted exception: CI-only race (user decision 2026-10-11)` followed by + the reason (cgo disabled locally; CI's Linux test job runs `-race`). +4. The coverage gate: `go run ./cmd/coverage-report --mode=total --profile=coverage.out` must print 80 or more +5. The full `make sqlc-check` recipe, including its `sqlc-version-check` prerequisite. Run `make sqlc-check` if `make` is on PATH. + Otherwise replicate it in order (first verify command): read `SQLC_VERSION` from the Makefile, fail with + `sqlc version mismatch: want X, got Y` unless `sqlc version` (from `$HOME/go/bin`) prints exactly that value, then `sqlc generate`, then + `git diff --exit-code -- internal/db/sqlc/`. Never run `sqlc generate` before the version guard passes. +6. `corepack pnpm --dir web run typecheck`, `corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}"`, then `corepack pnpm --dir web test` (70% on all four axes) + +Confirm the blast radius against `461267f`, the gap-closure planning head (third verify command). This task only commits if a gate fix +was needed. If so, rebuild the bundle again with Task 1's recipe, then commit with hooks enabled and no AI attribution. + + + go vet ./... && PATH="$HOME/go/bin:$PATH" golangci-lint run && { TEST_DATABASE_URL='postgres://drop_tracker:drop_tracker@localhost:5432/drop_tracker?sslmode=disable' go test ./... -count=1 -coverprofile=coverage.out -coverpkg="$(go list ./... | grep -vE '(^|/)(internal/db/sqlc|cmd/coverage-report|cmd/migrate|cmd/migration-check)$' | paste -sd, -)" > "${TMPDIR:-/tmp}/gotest-25-14.log" 2>&1; true; } && ! grep -E '^(FAIL|--- FAIL)' "${TMPDIR:-/tmp}/gotest-25-14.log" | grep -v 'internal/testutil' && cov=$(go run ./cmd/coverage-report --mode=total --profile=coverage.out) && echo "Backend coverage: $cov" && awk -v c="$cov" 'BEGIN { exit !(c + 0 >= 80) }' && want=$(sed -n 's/^SQLC_VERSION := //p' Makefile) && got=$(PATH="$HOME/go/bin:$PATH" sqlc version) && test -n "$want" && { [ "$got" = "$want" ] || { echo "sqlc version mismatch: want $want, got $got" >&2; false; }; } && PATH="$HOME/go/bin:$PATH" sqlc generate && git diff --exit-code -- internal/db/sqlc/ + any vet or lint diagnostic; any `FAIL` or `--- FAIL` line in the test log other than internal/testutil's known `go: no such tool "covdata"` failure (25-10 SUMMARY deviation 2); coverage below 80; `SQLC_VERSION` missing from the Makefile or a `sqlc version mismatch` (the `sqlc-version-check` guard, which runs before generate); or a diff under internal/db/sqlc/ + corepack pnpm --dir web run typecheck && corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}" && corepack pnpm --dir web test + non-zero exit, any "error TS" line, prettier listing files under "Code style issues", any "FAIL" line, or a line beginning "ERROR: Coverage for" + out=$(git diff --name-only 461267f -- queries internal/db internal/events go.mod go.sum web/package.json web/pnpm-lock.yaml web/app/components/ui web/app/lib/useUrlParams.ts web/app/root.tsx) && test -z "$out" + non-zero exit: the retention SQL, generated code, events service, a migration, a dependency file, the vendored ui/ components, useUrlParams, or root.tsx changed + + Run the built binary (or `docker compose up` with the rebuilt image) against the dev DB and re-run UAT tests 4, 7, and 9 as reworded in Task 2. Test 4: chip-label clicks far down the list, with a one-artist tag and a many-artist tag, on and off. Test 7: set `EVENT_RETENTION_DAYS=45`, then on History pick a tag (Hip-Hop has rows on both sides of the edge), press Load more to the end, reload, then Back. Test 9: Deezer search results at 375px, 640px, and 1024px. + Test 4: the clicked card stays where it was on screen (clamped on short pages), with no router scroll reset. The muted toggle only promises no router reset; a browser clamp on a short filtered list is expected (user decision 2026-10-11). Test 7: every card's Detected date is inside the 45-day window, even for releases years old, and the tag survives reload and Back. Test 9: readable artist names, and a truncated hint pill below 1024px. + Real-browser layout, scroll, and retention-edge data (human_verify_mode: end-of-phase). These are the /gsd-verify-work re-test items for G-25-4, G-25-7, and G-25-9. + + + + - Backend coverage prints 80 or more, and the web suite reports 70% or more on statements, branches, functions, and lines. + - The sqlc version guard passes (`sqlc version` equals the Makefile's `SQLC_VERSION`) before `sqlc generate` runs, and the sqlc diff is clean. + - The blast-radius diff against `461267f` is empty. + - The SUMMARY records `Accepted exception: CI-only race (user decision 2026-10-11)` with its reason. + + Every Definition of Done gate is green, the gap closure touched only its intended surface, and the three re-test items are ready for /gsd-verify-work. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| Package registry → `pnpm install --frozen-lockfile` | Build-time dependency resolution for the bundle | +| Built bundle → go:embed → served SPA | The committed tree is what the binary serves | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-25-SC | Tampering | npm installs during the rebuild | high | mitigate | No package is added. `--frozen-lockfile`, plus the blast-radius diff asserting `web/package.json` and `web/pnpm-lock.yaml` are unchanged since `461267f` | +| T-25-38 | Tampering | Stale embedded bundle | medium | mitigate | The rebuild is a dedicated task, and its verify asserts the bundle changed and that each fix's marker (`Detected`, `scrollBy`, `max-w-[40%]`) occurs more often than in the pre-change `461267f` tree, so a vendor-chunk hit can't pass for a fix | +| T-25-39 | Elevation / scope creep | Retention SQL and vendored components | low | mitigate | The blast-radius diff over queries/, internal/db, internal/events, web/app/components/ui, useUrlParams.ts, and root.tsx must be empty | +| T-25-40 | Repudiation | UAT result tampering | low | mitigate | Task 2's verify fails if any result, status, severity, root-cause, or Summary count line in 25-UAT.md changes, or if any `decision:` change other than the append-only G-25-4 addendum (original text kept verbatim) appears | + + + +- The Go chain, the web chain, and the blast-radius diff above exit 0 (apart from the documented covdata line). +- The bundle carries all three fix markers above their `461267f` counts, and the three docs carry the reworded contracts. +- After the SUMMARY is written: `grep -qF 'Accepted exception: CI-only race (user decision 2026-10-11)' .planning/phases/25-find-filter-watchlist-and-history/25-14-SUMMARY.md` exits 0. + + + +The Go binary serves all three gap fixes, the phase's spec, UAT, and verification documents describe the user's chosen contracts, every +CLAUDE.md gate is green, and G-25-4, G-25-7, and G-25-9 are ready for re-verification. + + + +Create `.planning/phases/25-find-filter-watchlist-and-history/25-14-SUMMARY.md` when done. Its `Accepted exceptions` section must carry +the line `Accepted exception: CI-only race (user decision 2026-10-11)`, with the reason (cgo disabled on this Windows box) and the note that +CI's Linux test job is the race gate. + diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-14-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-14-SUMMARY.md new file mode 100644 index 00000000..6528f950 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-14-SUMMARY.md @@ -0,0 +1,139 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 14 +subsystem: ui +tags: [go-embed, spa-bundle, uat, gap-closure, docs] +requires: + - phase: 25-find-filter-watchlist-and-history + provides: "Gap fixes from plans 25-11 (scroll anchor), 25-12 (Detected line), 25-13 (Deezer button cap)" +provides: + - "Rebuilt internal/webassets/build/client serving G-25-4, G-25-7, and G-25-9 fixes from a Go-only build" + - "UI-SPEC, UAT, and VERIFICATION contracts reworded to the user's 2026-10-11 decisions" + - "Green Definition of Done run and blast-radius check against 461267f" +affects: [gsd-verify-work re-verification of G-25-4, G-25-7, G-25-9] +actuals: + tokens: 175000 + tasks: 3 + commits: 2 +plan_head_before: 900706e25bb213359e61da02d27a40dc7756b709 +tech-stack: + added: [] + patterns: + - "Bundle-fix proof: compare marker counts against the pre-change tree, never bare presence" +key-files: + created: [] + modified: + - internal/webassets/build/client + - .planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md + - .planning/phases/25-find-filter-watchlist-and-history/25-UAT.md + - .planning/phases/25-find-filter-watchlist-and-history/25-VERIFICATION.md +key-decisions: + - "G-25-4 decision line extended append-only with the 2026-10-11 addendum; every original character kept" +requirements-completed: [WLVW-01, WLVW-04, HIST-02] +coverage: + - id: D1 + description: "Embedded SPA rebuilt from the gap-closure web/ so the Go binary serves the scroll anchor, Detected line, and capped Deezer button" + requirement: WLVW-01 + verification: + - kind: other + ref: "marker counts vs 461267f: Detected 2->3, scrollBy 0->1, max-w-\\[40\\%\\] 0->1; go build ./..." + status: pass + human_judgment: false + - id: D2 + description: "UI-SPEC, UAT (tests 4/7/9 and three gap truths), and VERIFICATION (checks 4 and 7) reworded; no result/status/severity/root_cause/Summary count change" + requirement: HIST-02 + verification: + - kind: other + ref: "Task 2 verify: exact-match decision line, UAT diff has no result/status lines, VERIFICATION status/score unchanged" + status: pass + human_judgment: false + - id: D3 + description: "Definition of Done gates and blast radius against 461267f" + requirement: WLVW-04 + verification: + - kind: integration + ref: "go vet, golangci-lint, go test ./... with coverage (90.32%), sqlc version guard + generate + diff, typecheck, prettier, web suite (95.16/90.47/95.8/97.09)" + status: pass + human_judgment: false + - id: D4 + description: "Real-browser re-test of UAT tests 4, 7 (EVENT_RETENTION_DAYS=45), and 9 at 375/640/1024px" + verification: [] + human_judgment: true + rationale: "Scroll, layout, and retention-edge data only show in a real browser (end-of-phase human verify, /gsd-verify-work)" +duration: 4min +completed: 2026-10-11 +status: complete +--- + +# Phase 25 Plan 14: Embed Rebuild, Contract Rewording, and Definition of Done Summary + +**Embedded SPA rebuilt so a Go-only build serves all three gap fixes, the UI-SPEC/UAT/VERIFICATION contracts reworded to the user's 2026-10-11 decisions, and every Definition of Done gate green with an empty blast radius against 461267f.** + +## Performance + +- **Duration:** 4 min +- **Started:** 2026-10-11T20:31:19Z +- **Completed:** 2026-10-11T20:35:06Z +- **Tasks:** 3 +- **Files modified:** 17 (14 bundle files, 3 docs) + +## Accomplishments +- Ran the `make web` recipe directly (frozen install, build, replace, copy). The lockfile did not drift. Marker counts under `internal/webassets/build/client/assets` rose strictly above the `461267f` tree: `Detected` 2 to 3, `scrollBy` 0 to 1, `max-w-\[40\%\]` 0 to 1. `go build ./...` compiles the embed. +- UI-SPEC: D-12 bullet renamed "Scroll reset and anchoring (D-12, G-25-4)" and names `useScrollAnchor` and the browser clamp. The Load more bullet says retention counts from `created_at` (Phase 10 D-01). The Copywriting table has the `Detected {date}` row. The SearchResultsColumns note carries the G-25-9 exception. The carry item says the clicked card keeps its on-screen position. +- UAT: `expected:` of tests 4, 7, 9 and the three gap `truth:` lines reworded. G-25-4's `decision:` is its `461267f` form with only the closing quote moved after the addendum (exact full-line match verified). No `result:`, `status:`, `severity:`, `root_cause:`, or Summary count touched. +- VERIFICATION: human checks 4 and 7 reworded in the frontmatter list and the body. `status: human_needed` and `score:` unchanged. +- Definition of Done: vet clean, golangci-lint 0 issues, backend coverage 90.32% (gate 80), sqlc 1.31.1 matched the Makefile before generate and the diff is clean, typecheck and prettier pass, web coverage 95.16 statements / 90.47 branches / 95.8 functions / 97.09 lines (gate 70). + +## Task Commits + +1. **Task 1: Rebuild the embedded SPA** - `afb881c` (chore) +2. **Task 2: Reword UI-SPEC, UAT, VERIFICATION** - `1d3017f` (docs) +3. **Task 3: Definition of Done and blast radius** - no commit (no gate fix needed) + +**Plan metadata:** committed separately (docs: complete plan) + +## Files Created/Modified +- `internal/webassets/build/client` - rebuilt hashed assets and `index.html` (superseded hashed files deleted) +- `.planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md` - D-12 anchoring, retention wording, Detected row, G-25-9 exception, carry item +- `.planning/phases/25-find-filter-watchlist-and-history/25-UAT.md` - reworded tests 4/7/9, gap truths, G-25-4 decision addendum +- `.planning/phases/25-find-filter-watchlist-and-history/25-VERIFICATION.md` - human checks 4 and 7 + +## Decisions Made +None - followed plan as specified. + +## Accepted exceptions + +Accepted exception: CI-only race (user decision 2026-10-11) + +Reason: cgo is disabled on this Windows box, so `go test -race` cannot run locally. The local integration run omitted `-race`. CI's Linux test job runs `-race` and is the race gate. + +## Deviations from Plan + +None - plan executed exactly as written. + +The plan's known `go: no such tool "covdata"` quirk for `internal/testutil` did not appear in this run: no `FAIL` or `--- FAIL` line was in the test log, and every package reported `ok`. + +## TDD Gate Compliance + +Not applicable (no `tdd="true"` tasks, plan type `execute`). + +## Issues Encountered +None. The tracer gate ran in end-of-phase mode with an automated-only verify, so the verify result stood in for a re-run and expansion continued. + +## Known Stubs +None. + +## Threat Flags +None. No new endpoints, auth paths, or trust-boundary schema changes. T-25-SC, T-25-38, T-25-39, and T-25-40 mitigations all held: frozen lockfile, marker-count check, empty blast-radius diff, and the append-only decision check. + +## User Setup Required +None - no external service configuration required. + +## Next Phase Readiness +Ready for `/gsd-verify-work`. Three real-browser re-test items are queued: test 4 (chip-label click far down a 50+ artist list; muted toggle only promises no router reset), test 7 (run with `EVENT_RETENTION_DAYS=45`, tag, Load more to the end, reload, Back), and test 9 (Deezer rows at 375/640/1024px). + +--- +*Phase: 25-find-filter-watchlist-and-history* +*Completed: 2026-10-11* + +## Self-Check: PASSED diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-15-PLAN.md b/.planning/phases/25-find-filter-watchlist-and-history/25-15-PLAN.md new file mode 100644 index 00000000..b017461c --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-15-PLAN.md @@ -0,0 +1,386 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 15 +type: execute +wave: 1 +depends_on: [] +files_modified: + - web/app/routes/watchlist.tsx + - web/app/routes/watchlist.test.tsx + - web/app/lib/watchlistView.ts + - web/app/lib/watchlistView.test.ts +autonomous: true +gap_closure: true +gap_ids: [G-25-4] +review_ids: [WR-01, WR-03] +requirements: [WLVW-04, WLVW-06] + +estimate: + tokens: 88000 + raw_tokens: 88000 + tasks: 2 + confidence: low + +must_haves: + truths: + - "G-25-4 (sticky path, review WR-01): the router never resets scroll, and after a chip-label click far down a 50+ artist list the clicked card stays where it was on screen (clamped by the browser on short pages), including when the clicked card is sticky. Start at `/?tags=3`, detach reggaeton from Bad Bunny (now sticky and still carrying trap), then click Bad Bunny's `Filter by trap`. The URL becomes `?tags=3%2C7`, the same label element is still connected and still has focus (D-08), and `window.scrollBy` is called once with the drift." + - "Sticky cards are keyed to the filterKey they were created under, and the set is derived during render. Every filter change drops them in the same render as the URL change, whether it comes from a G4 handler (tags, chip click, toggles, sort) or from Back/Forward. No commit ever renders the old filter without its sticky cards, and after Back no painted frame still shows a stale sticky card." + - "Turning a filter off again never brings back a card that was sticky before the filter changed." + - "User decision 2026-10-11 (gap planning), WR-03: the route's own tag rewrites keep sticky cards. Three writes carry the current sticky set to the filterKey they land on: a Manage tags merge or delete of a selected tag (replaceSelectedTag), the first-load tag validation, and the URL canonicalization write. Only G4 user actions, Back/Forward, and reload clear the set." + - "A carry belongs to the one write that made it (WLVW-06). The first navigation that lands after it consumes it, even when that landing leaves the filter unchanged. It keeps the cards only when that landing is a replace navigation onto the carry's target filterKey. It is never created when an unlanded filter change already sits under the write, or when the write lands on the current filterKey. So after a route-own rewrite followed in the same tick by a write back to the current filter, a later G4 action (push) or Back/Forward (pop) onto the old target still drops the cards." + - "Typing in the name filter still clears sticky cards in the same render as the local query change. Clear filters clears them explicitly only when no URL filter axis (tags, muted, custom) is active. Otherwise the filter-key change clears them in the same render as the URL." + - "The router contract is unchanged. useUrlParams.ts is untouched, and every write still passes preventScrollReset: true." + artifacts: + - path: web/app/routes/watchlist.tsx + provides: "Sticky state `{ key, loc, ids, carryTo }` reconciled during render against filterKey, the router location key, and the navigation type; writeKeepingSticky for the route's own tag rewrites" + contains: "reconcileSticky" + - path: web/app/lib/watchlistView.ts + provides: "Pure sticky transitions: `StickyState`, `reconcileSticky` (the first landing consumes a carry; ids are kept only for a replace landing on the carry target), `carrySticky` (no carry over an unlanded filter change or onto the current key)" + contains: "carryTo" + exports: ["StickyState", "reconcileSticky", "carrySticky", "EMPTY_IDS"] + - path: web/app/lib/watchlistView.test.ts + provides: "Unit tests F-I for the carry lifetime, including the stale-carry sequence (route-own rewrite, same-tick write back, then a G4 landing on the old target)" + - path: web/app/routes/watchlist.test.tsx + provides: "Tests: a sticky card's chip click keeps its anchor and focus; a Back same-commit probe; turning a toggle off doesn't bring a card back; Manage tags merge and delete keep sticky cards" + key_links: + - from: "router location commit (filterKey or location key changes)" + to: "sticky render-time reconcile" + via: "reconcileSticky(sticky, { key: filterKey, loc: location.key, replace }) returns a new state -> setSticky during render -> selectVisible receives it in the same commit as the new location" + pattern: "reconcileSticky\\(" + - from: "replaceSelectedTag, writeTagIds, canonical-params effect" + to: "sticky carry" + via: "writeKeepingSticky(mutate, { replace: true }) -> carrySticky(prev, from, to) inside write's synchronous mutator -> reconcileSticky keeps ids only when the landing is a replace onto carryTo" + pattern: "writeKeepingSticky\\(" + - from: "useNavigationType()" + to: "reconcileSticky replace flag" + via: "navigationType === NavigationType.Replace; G4 writes push and Back/Forward pops, so neither can apply a carry" + pattern: "NavigationType\\.Replace" + prohibitions: + - statement: "MUST NOT change web/app/lib/useUrlParams.ts or drop preventScrollReset from any URL write." + requirement_id: WLVW-04 + category: safety + verification: test + - statement: "MUST NOT carry sticky cards across a filter change made by a G4 handler write (tags, chip click, toggles, sort, Clear filters) or by Back/Forward, and a carry MUST NOT outlive the first navigation that lands after it. Only the route's own replace-mode tag rewrites carry them across a filter change, and only onto their own landing. A landing that leaves filterKey unchanged (any navigation type) keeps the set, because G4 clears on filter changes only (plan-checker iteration 2 override, user decision 2026-10-12)." + requirement_id: WLVW-06 + category: safety + verification: test + - statement: "No new npm package; web/package.json and web/pnpm-lock.yaml stay unchanged." + requirement_id: WLVW-04 + category: safety + verification: test +--- + + +Close the one failed must-have from 25-VERIFICATION (G-25-4 on the sticky-card path, review WR-01), and apply the user's WR-03 +decision in the same state rewrite. + +Root cause (25-VERIFICATION gap, 25-REVIEW WR-01): `handleTagsChange` calls the sticky clear as a sync-lane state write. React Router +7.18.2 applies the new location inside `React.startTransition` (`node_modules/react-router/dist/development/chunk-62JRHF6Z.mjs` +~6752). So the first commit renders the old filter without the sticky card, which unmounts it. The second commit mounts a new node. +`useScrollAnchor`'s captured chip is then disconnected, so no correction runs and focus falls to ``. + +Locked decisions (binding; they override CONTEXT/UI-SPEC ambiguity). Cite them as "user decision 2026-10-11 (gap planning)": +- WR-01 fix: make sticky invalidation part of the same render as the URL change. Key the sticky set to the filterKey it was created + under and derive it during render (25-REVIEW WR-01 sketch). Drop the sync clear from `handleTagsChange`, `handleFlagChange`, + `handleSortChange`, and the filterKey effect. Keep the q (local-state) path. +- WR-03: keep sticky cards across the route's own tag rewrites. A Manage tags merge or delete of a selected tag (`replaceSelectedTag`) + and the canonicalization write (`settleTagIds`) carry the sticky set to the new filterKey. Only G4 user actions (and Back/Forward) + clear it. Back must also show no post-paint flash. + +Design choice (Claude's discretion, recorded): the reviewer's sketch derives `ids` only when `sticky.key === filterKey`. Done that way, +it brings sticky cards back when the user returns to an earlier filter key (toggle on, then off), which breaks G4. This plan resets the +state during render with React's "adjust state while rendering" pattern, so any key change is final. The carry target (`carryTo`) is +how the route's own rewrites opt out of that reset. + +Carry lifetime (Claude's discretion, recorded; plan-checker iteration 1): a carry must apply only to the landing it was made for. If a +carry were cleared only on a filterKey change, a route-own rewrite followed in the same tick by a write back to the current key would +leave it pending, and a later G4 action onto the old target would keep stale ids. Clearing it on every render can't work either: the +router commits the location in a transition, so a render with the old location always runs between the write and its landing, and +that render would drop the carry before it lands. So the carry is tied to its write in three ways. The router location key (fresh +for every navigation) lets the first landing consume it. `useNavigationType()` limits it to a replace landing (every route-own rewrite +uses `{ replace: true }`, G4 writes push, and Back/Forward pops). And the write's starting filter key must equal the reconciled key, +so a write layered on an unlanded G4 change in the same tick never carries. The transitions live as pure functions in +`watchlistView.ts`, because same-tick write ordering can't be driven deterministically through the route in jsdom. + +Purpose: WLVW-04's chip click-to-filter keeps the user's place on every card. WLVW-06's sticky-card contract (G4) holds through Manage +tags and Back. +Output: the rewritten sticky state in `watchlist.tsx`, the pure sticky transitions in `watchlistView.ts` with unit tests for the carry +lifetime, plus route tests that pin the sticky chip click, Back in the same commit, turning a toggle off without bringing a card back, +and merge/delete carry. + +Spec-less probe fallback: visibly skipped for this run. This is gap_closure mode against an executed phase with no SPEC.md, so the +VERIFICATION gap and the REVIEW findings carry the edge coverage. + +Commit discipline (CLAUDE.md): run `corepack pnpm --dir web exec prettier --write "**/*.{ts,tsx}"` before staging. Every task commit +must clear CLAUDE.md DoD item 6: `corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}"` and the full +`corepack pnpm --dir web test` (coverage thresholds included). Each task's verify runs both. Commit once per task, after GREEN, and +never commit the RED state. Go gates are deferred to 25-18, recorded: this plan changes no Go file, and `internal/webassets` is +rebuilt there. Stage only this plan's files, because wave 1 shares the main tree with 25-16 (`use_worktrees: false` runs them one +after the other). If a gate failure is confined to 25-16's files (`SearchResultsColumns.*`) while they are uncommitted, wait for +that plan's commit and re-run. Never edit them. Comments are 1-3 lines with at most one design-doc reference each. Hooks stay on +(never `--no-verify`), and no AI attribution trailer. + + + +@~/.claude/gsd-core/workflows/execute-plan.md +@~/.claude/gsd-core/templates/summary.md + + + +@.planning/phases/25-find-filter-watchlist-and-history/25-VERIFICATION.md +@.planning/phases/25-find-filter-watchlist-and-history/25-REVIEW.md +@web/app/routes/watchlist.tsx +@web/app/lib/useUrlParams.ts + + + +Current shapes the executor builds on (verified at planning head `8ccde67`): + +- `web/app/routes/watchlist.tsx`: + - `const { params, write } = useUrlParams()` and `const view = parseWatchlistParams(params)` at lines 65-66. + - `stickyIds` is `useState>` at 74-76, with `markSticky` at 84-86 and the clear helper at 88-90. + - `handleQueryChange` (125-140) and the `view.q` re-sync effect (144-151) both clear stickies. Keep both. + - Lines 153-161: the comment "Back/Forward changes filters without going through a handler (G4).", the inline `filterKey` template + `${view.sort}|${view.tagIds.join(",")}|${view.muted}|${view.custom}`, `useScrollAnchor(filterKey)`, a previous-key ref, and the + `useEffect` that clears stickies on any key change. That effect is the WR-03 regression and the Back flash. + - The canonical-params effect is at 165-184 (`write(..., { replace: true })`), `writeTagIds` at 102-110, and `replaceSelectedTag` at + 274-286. These are the three route-own replace writes. + - `handleTagsChange` (441-448), `handleFlagChange` (450-457), and `handleSortChange` (467-476) each call the clear after `write`. + `toggleFilterTag` (459-465) captures the scroll anchor, then calls `handleTagsChange`. `clearFilters` (478-490) clears + unconditionally. + - `selectVisible(entries, { ...view, q: query }, stickyIds)` at 436-438. +- `web/app/lib/useUrlParams.ts`: `write(mutate, opts)` clones `latestRef.current`, calls `mutate(next)` exactly once and synchronously, + then calls `setParams(next, { replace, preventScrollReset: true })`. Do not touch this file. +- `web/app/lib/watchlistView.ts`: exports `interface WatchlistView` (line 47), `parseWatchlistParams` (77), + `canonicalWatchlistParams` (90), and `selectVisible(entries, view, stickyIds)` (187). It is pure and imports nothing from the router. + `web/app/lib/watchlistView.test.ts` imports its exports at lines 1-20 and has describes per export (`selectVisible` at 53, + `canonicalWatchlistParams` at 238). +- `react-router` 7.18.2 (installed): `useLocation()` returns a `Location` whose `key` is new for every push, replace, or pop + navigation. `useNavigationType()` returns the `Action` enum, which is exported as `NavigationType`, so compare with + `NavigationType.Replace` and not a string literal. `watchlist.tsx` has no `react-router` import today. +- Write modes in `watchlist.tsx`: the G4 writes (`handleTagsChange`, `handleFlagChange`, `handleSortChange`, `clearFilters`) call + `write(mutate)` with default options, which is a push. The three route-own writes and the debounced `q` write pass + `{ replace: true }`. +- `web/app/routes/watchlist.test.tsx`: the imports at lines 1-34 (`within`, `useNavigate`, `makeEntry`, `LocationProbe`, `renderRoute`, + and `mockDetachTag`/`mockRenameTag`/`mockMergeTag`/`mockDeleteTag`/`mockListTags`). Describes: "Watchlist tag filter consistency + (D-13)" at 1581, with helpers `openManageTags`, `deleteTagInDialog`, `mergeRapIntoTrap`, and `mockVocabulary`, and rows Bad Bunny + [rap 5], Daddy Yankee [trap 9], and Travis Scott [trap 9]; "Watchlist preference toggles (WLVW-05)" at 1849, with rows Bad Bunny, + Bad Two, Bad Three, Drake, and Eminem, `mutedToggle`/`customToggle`, and the Back test at 1982; and "chip click keeps the clicked card + on screen (G-25-4)" at 2100, with `stubTop(el, whenOff, whenOn)` keyed on `aria-pressed` and rows Bad Bunny/Daddy Yankee [reggaeton 3] + and Travis Scott [trap 7]. `web/vitest.config.ts` sets `mockReset: true`. The G-25-4 describe restores mocks in `afterEach`. + + + +Noun: the sticky set. Decision: no-change in identity (ids stay watchlist entry ids). The set gains a scope key, which is the filter +it belongs to, not a second identity. Rationale: the change is about when the set is invalid, not what it identifies. + + + + + + Task 1: End-to-end "a sticky card's chip click keeps its anchor and focus": sticky set keyed to the filter and reset during render + web/app/routes/watchlist.tsx, web/app/routes/watchlist.test.tsx + + - `web/app/routes/watchlist.tsx` lines 56-161 (state, sticky helpers, q path, filterKey effect) and 435-490 (visible list, G4 handlers, `toggleFilterTag`, `clearFilters`). + - `web/app/lib/useScrollAnchor.ts` (21 lines; unchanged here, but its `isConnected` guard is why the remount loses the anchor). + - `web/app/routes/watchlist.test.tsx` lines 1-34 (imports), 1849-2023 (WLVW-05 describe incl. the Back test at 1982), 2100-2211 (G-25-4 describe and `stubTop`). + - `.planning/phases/25-find-filter-watchlist-and-history/25-VERIFICATION.md` frontmatter `gaps:` (probe steps and observed failure). + - `.planning/phases/25-find-filter-watchlist-and-history/25-REVIEW.md` WR-01 (fix sketch). + + + - Test A, in the G-25-4 describe: "keeps a sticky card's chip anchored and focused when the chip filters (WR-01)". The test uses local rows: Bad Bunny tagged reggaeton (3) and trap (7), plus the describe's Daddy Yankee and Travis Scott. `mockDetachTag` resolves. Start at `/?tags=3` and click `Remove tag reggaeton from Bad Bunny`. Bad Bunny stays, because it is sticky. Take Bad Bunny's card (`screen.getByText("Bad Bunny").closest("li")`) and get its `Filter by trap` label with `within`. Spy on `window.scrollBy` with a no-op, call `stubTop(label, 400, 120)`, then click the label. Expected results: `search()` is `?tags=3%2C7`; `shown()` is Bad Bunny, Daddy Yankee, Travis Scott; `label.isConnected` is true; querying the fresh Bad Bunny card for `Filter by trap` returns the same element; the label has focus; and `window.scrollBy` was called exactly once, with `(0, -280)`. It fails on the pre-change code: the label is disconnected, focus is on BODY, and scrollBy is never called. + - Test B, in the WLVW-05 describe: "drops a sticky card in the same commit as Back's URL change". Repeat the steps of the existing Back test (start `/?muted=1`, press `Custom release types`, then uncheck Bad Bunny's `Mute guest feature alerts`). Render a `CommitProbe` inside the wrapper. It reads `useLocation()` and, in a `useLayoutEffect` keyed on the location, pushes `{ search: location.search, shown: shown() }` into a test-local array. Empty the array just before clicking `history back`. After `shown()` settles to Bad Two, Drake, the first recorded entry equals `{ search: "?muted=1", shown: ["Bad Two", "Drake"] }`. It fails on the pre-change code, because that entry still lists Bad Bunny: the old effect clears after the location commit. + - Test C, in the WLVW-05 describe: "does not bring a sticky card back when a toggle is turned off again". Start at `/?muted=1` and uncheck Bad Bunny's mute (Bad Bunny stays sticky). Press `Custom release types`, and `shown()` is Drake. Press it again: the URL is `?muted=1` and `shown()` is Bad Two, Drake. This regression guard passes before and after the change. A purely derived `key === filterKey` set would fail it. + - Every existing test in watchlist.test.tsx still passes unchanged, including "drops a sticky card when the sort changes", "drops a sticky card when Back restores a filter it does not match", "keeps a card whose last mute was just unchecked until the next filter change", and the G-25-4 shrink, grow, and leaves-the-list cases. + + +Write tests A, B, and C first, and run them against the current code. A and B must fail (RED), and C must pass. Record the RED output +in the SUMMARY. Then implement (GREEN), per user decision 2026-10-11 (gap planning) on WR-01: + +In `web/app/routes/watchlist.tsx`: +- Import `type WatchlistView` from `~/lib/watchlistView`. At module scope, add `const EMPTY_IDS: ReadonlySet = new Set()` and + `function filterKeyOf(view: WatchlistView): string`, which returns exactly today's format: sort, comma-joined tag ids, muted, and + custom, joined by `|`. +- Compute `const filterKey = filterKeyOf(view)` directly after `view` is parsed. Remove the old inline declaration, and keep exactly one + `useScrollAnchor(filterKey)` call. +- Replace the `stickyIds` state with `const [sticky, setSticky] = useState<{ key: string; ids: ReadonlySet }>(() => ({ key: + filterKey, ids: EMPTY_IDS }))`. Directly after it, add the render-time reset: when `sticky.key !== filterKey`, build + `{ key: filterKey, ids: EMPTY_IDS }`, call `setSticky` with it, and use it as this render's value. Then expose + `const stickyIds = current.ids` to `selectVisible`, which is otherwise unchanged. The guard converges after one pass (React's + documented "adjust state while rendering" pattern), so the reset lands in the same commit as the router's location update. +- `markSticky(id)` becomes a functional updater that adds `id` to `prev.ids` and keeps `prev.key` (it returns `prev` when the id is + already present). The clear helper `clearSticky()` becomes a functional updater that empties `ids` and keeps `key` (it returns `prev` + when already empty). In event handlers and async callbacks, `prev.key` is always the last rendered filterKey, so neither helper needs + the key passed in. +- Remove the `clearSticky()` call from `handleTagsChange`, `handleFlagChange`, and `handleSortChange`. Their URL writes change the + filterKey, and the render-time reset clears the set in that same render. +- Delete the filterKey effect at lines 153-161: its comment, its previous-key ref, and its `useEffect`. +- In `clearFilters`, call `clearSticky()` only when `view.tagIds.length === 0 && !view.muted && !view.custom`. Only the local query + changes then, so the key doesn't. Otherwise, the key change clears the set in the same render as the URL write. +- Keep `handleQueryChange` and the `view.q` re-sync effect as they are, since the query is local state. +- Add one comment of 1-2 lines on the sticky state saying why it is keyed: the router commits the URL in a transition, so the reset has + to happen in that render. Cite G4 once. + +Do not touch `useUrlParams.ts`, `useScrollAnchor.ts`, `TagChips.tsx`, or the toolbar wiring. Test B needs `useLocation` from +`react-router` and `useLayoutEffect` from `react`, added to the test file's imports. + +Before staging, run `corepack pnpm --dir web exec prettier --write "**/*.{ts,tsx}"`. Then run the DoD item 6 gate (the third verify +command) and commit only when it passes. Make no commit of the RED tests on their own. + + + corepack pnpm --dir web exec vitest run app/routes/watchlist.test.tsx --coverage.enabled=false + non-zero exit, or "failed" in the vitest summary line + corepack pnpm --dir web run typecheck && f=web/app/routes/watchlist.tsx && ok=1 && for fn in handleTagsChange handleFlagChange handleSortChange; do n=$(sed -n "/function $fn(/,/^ }\$/p" "$f" | grep -c "clearSticky"); [ "$n" -eq 0 ] || { echo "$fn still clears: $n"; ok=0; }; done; [ "$ok" = 1 ] && [ "$(grep -c "prevFilterKey" "$f")" -eq 0 ] && [ "$(grep -c 'useScrollAnchor(filterKey)' "$f")" -eq 1 ] && [ "$(grep -c 'sticky.key !== filterKey' "$f")" -ge 1 ] + non-zero exit: an "error TS" line from typecheck; a "still clears" line naming a G4 handler; the old previous-key ref still present; `useScrollAnchor(filterKey)` not exactly once; or no render-time key comparison in the route + corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}" && corepack pnpm --dir web test + non-zero exit: prettier listing files under "Code style issues", any "FAIL" line from the full web suite, or a line beginning "ERROR: Coverage for" (the 70% thresholds) + + + - Tests A and B fail on the pre-change code and pass after it. The SUMMARY quotes the RED run's failing assertion for each. + - Test C passes, and every pre-existing test in `web/app/routes/watchlist.test.tsx` passes unchanged. + - In `web/app/routes/watchlist.tsx`, the bodies of `handleTagsChange`, `handleFlagChange`, and `handleSortChange` contain no `clearSticky` call, and `grep -c "prevFilterKey"` prints 0. + - `grep -c "useScrollAnchor(filterKey)" web/app/routes/watchlist.tsx` prints 1. + - `git diff --name-only -- web/app/lib/useUrlParams.ts web/app/lib/useScrollAnchor.ts web/app/components/watchlist/TagChips.tsx web/package.json web/pnpm-lock.yaml` prints nothing. + - `corepack pnpm --dir web run typecheck` exits 0. + - Before the commit, `corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}"` and the full `corepack pnpm --dir web test` both exit 0 (CLAUDE.md DoD item 6). + + A chip click on a sticky card keeps the card mounted, anchored, and focused, and Back drops stale sticky cards in the same commit as the URL. Committed. + + + + + + + + + Task 2: Carry sticky cards across the route's own tag rewrites, tied to the write that made them (WR-03) + web/app/lib/watchlistView.ts, web/app/lib/watchlistView.test.ts, web/app/routes/watchlist.tsx, web/app/routes/watchlist.test.tsx + + - `web/app/routes/watchlist.tsx` after Task 1, especially the sticky state and its render-time reset, `markSticky`/`clearSticky`, `filterKeyOf`, `writeTagIds`, `replaceSelectedTag`, `applyTagChange`, and the canonical-params effect. + - `web/app/lib/useUrlParams.ts` (`write` calls `mutate` once, synchronously, before `setParams`). + - `web/app/lib/watchlistView.ts` (whole file) and `web/app/lib/watchlistView.test.ts` lines 1-60 (imports and fixture style). + - `web/app/routes/watchlist.test.tsx` lines 1581-1847 (the D-13 describe: helpers, merge/delete tests, and the in-session detach test). + - `.planning/phases/25-find-filter-watchlist-and-history/25-REVIEW.md` WR-03. + + + - Unit tests go in a new `describe("sticky set (G4, WR-03)")` in `web/app/lib/watchlistView.test.ts`, starting from `s0 = { key: "a", loc: "1", ids: new Set([1]), carryTo: null }`. + - Test F: "consumes a carry on the first landing, even one that leaves the filter unchanged". This is the stale-carry sequence: a route-own rewrite, then a write back to the current filter in the same tick, then a G4 action. `s1 = carrySticky(s0, "a", "b")` has `carryTo` "b" (a merge's replace write). `s2 = reconcileSticky(s1, { key: "a", loc: "2", replace: false })` (the write back landed) has `carryTo` null and `loc` "2", and still holds id 1. Then `reconcileSticky(s2, { key: "b", loc: "3", replace: false })` (a G4 action onto the old target) and `reconcileSticky(s2, { key: "b", loc: "3", replace: true })` both have empty `ids`. + - Test G: "applies a carry only to a replace landing on its target". With `c = carrySticky(s0, "a", "b")`, `reconcileSticky(c, { key: "b", loc: "2", replace: true })` keeps id 1 and has `carryTo` null. `reconcileSticky(c, { key: "b", loc: "2", replace: false })` (a G4 push or a Back/Forward pop) and `reconcileSticky(c, { key: "x", loc: "2", replace: true })` both have empty `ids`. + - Test H: "never carries over an unlanded filter change or onto the current filter". `carrySticky(s0, "x", "y").carryTo` is null (a G4 change from the same tick sits under the write). `carrySticky(s0, "a", "a").carryTo` is null. `carrySticky({ ...s0, carryTo: "b" }, "a", "a").carryTo` is null (a write back replaces an older carry). + - Test I: "returns the same state when nothing landed". `reconcileSticky(s0, { key: "a", loc: "1", replace: true })` is `s0` itself, so the render-time guard converges. `reconcileSticky(s0, { key: "a", loc: "2", replace: true })` (a landed `q`-only write) keeps id 1 and has `loc` "2". + - Test D, in the D-13 describe: "keeps a sticky card when Manage tags merges a selected tag away (WR-03)". Set up `mockVocabulary()` and `mockDetachTag` resolving, with `mockRenameTag` (collision with trap) and `mockMergeTag` as in "swaps a merged-away tag for its target in the filter". Start at `/?tags=5` and click `Remove tag rap from Bad Bunny`; `shown()` is Bad Bunny. Then call `openManageTags()` and `mergeRapIntoTrap()`, wait for `search()` to be `?tags=9`, and press Escape to close the dialog. `shown()` is now Bad Bunny, Daddy Yankee, Travis Scott. Then press `Has muted events`, and `shown()` becomes empty: a G4 action still drops the carried card. + - Test E, in the D-13 describe: "keeps a sticky card when Manage tags deletes one of two selected tags (WR-03)". Set up `mockVocabulary()`, `mockDetachTag` resolving, and `mockDeleteTag` resolving `{ carrier_count: 1 }`. Start at `/?tags=5,9` and click `Remove tag rap from Bad Bunny`; Bad Bunny stays. Then call `openManageTags()` and `deleteTagInDialog("rap")`, wait for `search()` to be `?tags=9`, and press Escape. `shown()` is Bad Bunny, Daddy Yankee, Travis Scott. + - Tests D and E fail after Task 1 alone, because the render-time reset drops Bad Bunny. They pass once the carry is in place. Their assertions run after the Escape keystroke, so a pending effect can't make them pass vacuously. They also show that the route passes `replace: true` for its own replace landings. + - All D-13 tests, both Back tests (the existing one and Task 1's probe), and Task 1's tests A and C still pass. + + +Write tests F-I and D-E first. F-H need to fail on an assertion, not on an import error. So first add `StickyState`, +`reconcileSticky`, and `carrySticky` to `watchlistView.ts` in the form plan-checker iteration 1 flagged: reconcile only when the key +changes, keep ids when `carryTo` equals the new key, and carry whenever `to` differs from `prev.key`, ignoring `loc`, `replace`, and +`from`. Run the unit tests: F, G's push case, and H must fail on assertions. Run D and E against Task 1's route: they must fail too. +Record every one of these RED results in the SUMMARY. Then implement (GREEN), per user decision 2026-10-11 (gap planning) on WR-03: + +In `web/app/lib/watchlistView.ts`, which stays pure with no router import: +- Move `EMPTY_IDS` here from the route and export it. +- Export `interface StickyState { key: string; loc: string; ids: ReadonlySet; carryTo: string | null }`. `key` is the + filterKey the set was last reconciled under, `loc` is the router location key it was last reconciled with, and `carryTo` is a + pending carry target. +- Export `reconcileSticky(prev: StickyState, at: { key: string; loc: string; replace: boolean }): StickyState`. Return `prev` itself + when `at.key === prev.key` and `at.loc === prev.loc`, because nothing landed. Otherwise a navigation landed, so return + `{ key: at.key, loc: at.loc, carryTo: null, ids }`. Here `ids` stays `prev.ids` when `at.key === prev.key`, or when `at.replace` is + true and `prev.carryTo === at.key`. In every other case it is `EMPTY_IDS`. +- Export `carrySticky(prev: StickyState, from: string, to: string): StickyState`. The new `carryTo` is `to` when + `from === prev.key && to !== prev.key`, and null otherwise. Return `prev` when `carryTo` doesn't change. +- Put one comment of 1-2 lines on `reconcileSticky` saying a carry only lives until the next landing. Cite WR-03 once. + +In `web/app/routes/watchlist.tsx`: +- Import `NavigationType`, `useLocation`, and `useNavigationType` from `react-router`, and `EMPTY_IDS`, `reconcileSticky`, + `carrySticky`, and `type StickyState` from `~/lib/watchlistView`. Read `const location = useLocation()` and + `const navigationType = useNavigationType()` next to `useUrlParams()`, before the sticky state. +- The sticky state becomes `useState(() => ({ key: filterKey, loc: location.key, ids: EMPTY_IDS, carryTo: null }))`. + Replace Task 1's inline reset with `const current = reconcileSticky(sticky, { key: filterKey, loc: location.key, replace: + navigationType === NavigationType.Replace })`. When `current !== sticky`, call `setSticky(current)` during render. + `stickyIds` stays `current.ids`. +- `markSticky` and `clearSticky` spread `prev`, so `key`, `loc`, and `carryTo` survive an id change. +- Add `writeKeepingSticky(mutate: (next: URLSearchParams) => void, opts: { replace?: boolean })`. It calls `write` with a wrapper + that computes `from = filterKeyOf(parseWatchlistParams(p))`, runs `mutate(p)`, computes `to` the same way, and calls + `setSticky((prev) => carrySticky(prev, from, to))`. `write` invokes its mutator exactly once, synchronously, on the exact params it + hands the router. So `from` shows whether an unlanded filter change from the same tick sits under this write. +- Route the three route-own replace writes through `writeKeepingSticky`, keeping their mutators and `{ replace: true }`: `writeTagIds` + (first-load validation), `replaceSelectedTag` (Manage tags delete/merge, D-13), and the canonical-params effect (`settleTagIds`). + Update that effect's existing eslint-disable reason if it names what the effect reads. +- Leave `handleTagsChange`, `handleFlagChange`, `handleSortChange`, `toggleFilterTag`, and `clearFilters` on plain `write`. These are + the G4 user actions. They push, so their landing must clear the set. Back/Forward never goes through a writer and lands as a pop, so + it never carries. +- Add one short comment (1-2 lines) on `writeKeepingSticky` saying why route-own rewrites keep sticky cards. Cite WR-03 once. + +Run prettier on `web/` before staging. Then run the DoD item 6 gate (the third verify command) and commit with hooks enabled once it +passes. Make no RED-only commit. + + + corepack pnpm --dir web exec vitest run app/lib/watchlistView.test.ts app/components/watchlist/TagChips.test.tsx app/routes/watchlist.test.tsx --coverage.enabled=false + non-zero exit, or "failed" in the vitest summary line + corepack pnpm --dir web run typecheck && f=web/app/routes/watchlist.tsx && v=web/app/lib/watchlistView.ts && [ "$(grep -c 'writeKeepingSticky(' "$f")" -ge 4 ] && [ "$(grep -c 'reconcileSticky(' "$f")" -ge 1 ] && [ "$(grep -c 'NavigationType.Replace' "$f")" -ge 1 ] && [ "$(grep -c 'location.key' "$f")" -ge 2 ] && [ "$(grep -c 'export function reconcileSticky' "$v")" -eq 1 ] && [ "$(grep -c 'export function carrySticky' "$v")" -eq 1 ] && [ "$(grep -c 'carryTo' "$v")" -ge 2 ] && [ "$(grep -c 'react-router' "$v")" -eq 0 ] && ok=1 && for fn in handleTagsChange handleFlagChange handleSortChange toggleFilterTag clearFilters; do n=$(sed -n "/function $fn(/,/^ }\$/p" "$f" | grep -c "writeKeepingSticky"); [ "$n" -eq 0 ] || { echo "$fn carries stickies: $n"; ok=0; }; done; [ "$ok" = 1 ] + non-zero exit: an "error TS" line; fewer than four `writeKeepingSticky(` occurrences (definition plus three call sites); no `reconcileSticky(` call, no `NavigationType.Replace` comparison, or fewer than two `location.key` reads (initial state and reconcile) in the route; `reconcileSticky` or `carrySticky` not exported exactly once from watchlistView.ts, or no `carryTo` there; a router import in watchlistView.ts; or a "carries stickies" line naming a G4 handler + corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}" && corepack pnpm --dir web test + non-zero exit: prettier listing files under "Code style issues", any "FAIL" line from the full web suite, or a line beginning "ERROR: Coverage for" (the 70% thresholds) + + + - Tests F, G (push case), and H fail on assertions against the flagged form of the transitions, and D and E fail on Task 1's route. All of them pass after Task 2. The SUMMARY quotes each RED failure. + - Test F passes: after a carry, a landing on the unchanged filter consumes it (`carryTo` is null), and a later push or replace landing on the old target drops the cards. + - All tests in `web/app/lib/watchlistView.test.ts`, `web/app/routes/watchlist.test.tsx`, and `web/app/components/watchlist/TagChips.test.tsx` pass, including "drops a sticky card when Back restores a filter it does not match" and Task 1's Back probe. + - `grep -c "writeKeepingSticky(" web/app/routes/watchlist.tsx` prints 4 or more, and none of `handleTagsChange`, `handleFlagChange`, `handleSortChange`, `toggleFilterTag`, or `clearFilters` calls it. + - `web/app/lib/watchlistView.ts` exports `reconcileSticky` and `carrySticky` and imports nothing from `react-router`. + - `git diff --name-only -- web/app/lib/useUrlParams.ts web/package.json web/pnpm-lock.yaml` prints nothing. + - Typecheck, `corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}"`, and the full `corepack pnpm --dir web test` all exit 0 before the commit (CLAUDE.md DoD item 6). + + A Manage tags merge or delete of a selected tag, and any canonical rewrite, keeps the user's just-edited cards. Every G4 action and Back still clears them, and no carry outlives the landing it was made for. Committed. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| URL query string → Watchlist view | Untrusted `tags`/`sort`/`muted`/`custom` params, already parsed and canonicalized by `watchlistView.ts`. This plan only changes when sticky state resets | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-25-41 | Denial of service (UI) | Render-time `setSticky` reconcile in `watchlist.tsx` | low | mitigate | The update runs only when `reconcileSticky` returns a new object, and it returns `prev` itself once the key and location key match (Test I), so the component converges after one re-render. Tests A-E exercise every landing path (chip, toggle on/off, Back, merge, delete) without a render loop | +| T-25-42 | Tampering | Sticky carry | low | mitigate | Sticky ids come only from in-card edits and adds, never from the URL. A carry is consumed by the first landing after its write, applies only to a replace landing on its own target, and is never created over an unlanded filter change. So a crafted URL, a G4 push, or a Back/Forward pop can't keep or resurrect cards (unit tests F-H, Test B, the existing Back test, and Test D's G4 tail) | +| T-25-43 | Tampering | Router scroll contract | low | mitigate | `useUrlParams.ts` is unchanged (empty `git diff` in the acceptance criteria). Every write still passes `preventScrollReset: true` | +| T-25-SC | Tampering | npm installs | low | mitigate | No package is added. The acceptance criteria assert that `web/package.json` and `web/pnpm-lock.yaml` are unchanged | + + + +- `corepack pnpm --dir web exec vitest run app/lib/watchlistView.test.ts app/components/watchlist/TagChips.test.tsx app/routes/watchlist.test.tsx --coverage.enabled=false` passes. +- `corepack pnpm --dir web run typecheck` exits 0. +- Before each task commit, `corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}" && corepack pnpm --dir web test` exits 0 (CLAUDE.md DoD item 6, per task). +- The Go gates, the bundle rebuild, and the real-browser re-check run in 25-18. This plan changes no Go file. + + + +The G-25-4 sticky-card path is closed: a chip click on a sticky card keeps it mounted, anchored, and focused. Sticky cards reset in the +same render as every user-driven filter change and Back, never come back when a filter is turned off again, and survive the route's +own Manage tags merge/delete and canonical rewrites, as the user decided on 2026-10-11. A carry never outlives the landing it was made +for, so no G4 action or Back/Forward inherits it. + + +## Artifacts this phase produces + +| Artifact | Kind | Notes | +|----------|------|-------| +| `web/app/routes/watchlist.tsx` | modified | Adds `filterKeyOf(view)` (module function), reads `useLocation()` and `useNavigationType()`, and holds the `sticky` state `{ key, loc, ids, carryTo }`, which is reconciled during render with `reconcileSticky`. Adds `writeKeepingSticky(mutate, opts)`. `markSticky` and `clearSticky` are rewritten as updaters that preserve the other fields. The filterKey effect is removed | +| `web/app/lib/watchlistView.ts` | modified | Adds `EMPTY_IDS` (moved from the route in Task 2), `interface StickyState`, `reconcileSticky(prev, at)`, and `carrySticky(prev, from, to)`, all pure | +| `web/app/lib/watchlistView.test.ts` | modified | Unit tests F (stale-carry sequence), G (replace-only carry), H (no carry over an unlanded change or onto the current key), and I (convergence) | +| `web/app/routes/watchlist.test.tsx` | modified | Tests A (sticky chip click, WR-01), B (Back same-commit probe with `CommitProbe`), C (turning a toggle off doesn't bring a card back), D (merge keeps the sticky card, WR-03), and E (delete keeps the sticky card, WR-03) | + + +Create `.planning/phases/25-find-filter-watchlist-and-history/25-15-SUMMARY.md` when done. Include the RED outputs for tests A, B, D, +E, F, G, and H, the design note on why the render-time reset replaces a purely derived key comparison, and the carry-lifetime note +(location key, replace-only, starting-key check). + diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-15-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-15-SUMMARY.md new file mode 100644 index 00000000..f61fb16e --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-15-SUMMARY.md @@ -0,0 +1,151 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 15 +subsystem: ui +tags: [react-router, sticky-cards, gap-closure, scroll-anchor, tdd] +status: complete +requires: + - phase: 25-find-filter-watchlist-and-history + provides: "useScrollAnchor and the G4 sticky-card contract from plans 25-05 and 25-11" +provides: + - "Sticky set keyed to the filterKey and reconciled during render, so it resets in the same commit as the URL change" + - "Pure reconcileSticky/carrySticky transitions with a carry that lives until the first landing" + - "writeKeepingSticky for the route's own tag rewrites (Manage tags merge/delete, first-load validation, canonicalization)" +affects: [25-18 real-browser re-check of G-25-4, gsd-verify-work] +actuals: + tokens: 52000 + tasks: 2 + commits: 2 +plan_head_before: 7ae9ace5f773fed7465657e176b912b586b18ee3 +tech-stack: + added: [] + patterns: + - "Adjust state while rendering: reconcileSticky returns prev itself when nothing landed, so the render-time setSticky converges" + - "A pending carry is consumed by the first landing (router location key) and applies only to a replace landing on its target" +key-files: + created: [] + modified: + - web/app/routes/watchlist.tsx + - web/app/routes/watchlist.test.tsx + - web/app/lib/watchlistView.ts + - web/app/lib/watchlistView.test.ts +key-decisions: + - "Render-time reset instead of a purely derived `sticky.key === filterKey` check, because a derived check revives a card when a filter is turned off again" + - "Carry lifetime tied to the write: location key consumes it on the first landing, NavigationType.Replace limits it to route-own rewrites, and the write's starting key must equal the reconciled key" + - "A landing that leaves filterKey unchanged keeps the set (G4 clears on filter changes only), per the plan-checker iteration 2 override" +requirements-completed: [WLVW-04, WLVW-06] +coverage: + - id: D1 + description: "A chip click on a sticky card keeps the card mounted, anchored (scrollBy drift correction) and focused" + requirement: WLVW-04 + verification: + - kind: test + ref: "web/app/routes/watchlist.test.tsx#keeps a sticky card's chip anchored and focused when the chip filters (WR-01)" + status: pass + human_judgment: false + - id: D2 + description: "Sticky cards are dropped in the same commit as every filter change, including Back/Forward, and never revived by turning a filter off" + requirement: WLVW-06 + verification: + - kind: test + ref: "web/app/routes/watchlist.test.tsx#drops a sticky card in the same commit as Back's URL change" + status: pass + - kind: test + ref: "web/app/routes/watchlist.test.tsx#does not bring a sticky card back when a toggle is turned off again" + status: pass + human_judgment: false + - id: D3 + description: "The route's own tag rewrites (Manage tags merge/delete, validation, canonicalization) keep sticky cards; no carry outlives its landing" + requirement: WLVW-06 + verification: + - kind: test + ref: "web/app/lib/watchlistView.test.ts#sticky set (G4, WR-03)" + status: pass + - kind: test + ref: "web/app/routes/watchlist.test.tsx#keeps a sticky card when Manage tags merges a selected tag away (WR-03)" + status: pass + - kind: test + ref: "web/app/routes/watchlist.test.tsx#keeps a sticky card when Manage tags deletes one of two selected tags (WR-03)" + status: pass + human_judgment: false + - id: D4 + description: "Scroll position of the clicked sticky card on a real long list in a real browser" + requirement: WLVW-04 + human_judgment: true + rationale: "jsdom stubs getBoundingClientRect and scrollBy; the real-browser re-check is owned by plan 25-18" +duration: 8 min +completed: 2026-10-12 +--- + +# Phase 25 Plan 15: Sticky Cards Keyed to the Filter Summary + +**Sticky-card set keyed to the filterKey and reset during render (closes G-25-4's sticky path, WR-01), with a pure, single-landing carry so the route's own tag rewrites keep sticky cards (WR-03).** + +## Accomplishments + +- A chip click on a sticky card no longer unmounts it: the reset now happens in the same render as the router's transitioned location commit, so `useScrollAnchor`'s captured label stays connected, the drift correction runs once, and focus stays on the label (D-08). +- Back/Forward drops stale sticky cards in the same commit as the URL (no painted frame with a stale card). The post-commit `filterKey` effect and the sync clears in `handleTagsChange`/`handleFlagChange`/`handleSortChange` are gone. +- Manage tags merge or delete of a selected tag, first-load tag validation, and canonicalization keep the user's sticky cards via `writeKeepingSticky`. G4 actions (push) and Back/Forward (pop) still clear them. +- `useUrlParams.ts` is untouched and every write still passes `preventScrollReset: true`. No package changes. + +## Task Commits + +1. Task 1 (tracer): `9057be9` fix(25-15): key sticky cards to the filter and reset them in the URL's render +2. Task 2: `6bb851f` feat(25-15): carry sticky cards across the route's own tag rewrites + +Each task was committed once, after GREEN; no RED-only commit was made (plan commit discipline), so the gate-sequence commits `test(...)`/`feat(...)` do not appear separately. RED evidence is recorded below. + +## RED Evidence + +Task 1 (against the pre-change route): +- Test A `keeps a sticky card's chip anchored and focused when the chip filters (WR-01)`: `expect(label.isConnected).toBe(true)` -> received `false` (the label was unmounted by the first commit). +- Test B `drops a sticky card in the same commit as Back's URL change`: `commits[0]` was `{ search: "?muted=1", shown: ["Bad Bunny", "Bad Two", "Drake"] }`; expected `["Bad Two", "Drake"]` (Bad Bunny still listed, as the old effect clears after the location commit). +- Test C (toggle off again) passed before and after, as planned. + +Task 2 (flagged-form transitions: reconcile only on key change, carry whenever `to !== prev.key`): +- F `consumes a carry on the first landing...`: `expected 'b' to be null` (`s2.carryTo`). +- G `applies a carry only to a replace landing on its target`: `expected [ 1 ] to deeply equal []` for the push landing. +- H `never carries over an unlanded filter change or onto the current filter`: `expected 'y' to be null` for `carrySticky(s0, "x", "y")`. +- I `returns the same state when nothing landed`: `expected '1' to be '2'` (`q.loc`); the identity assertion held. +- D `keeps a sticky card when Manage tags merges a selected tag away (WR-03)`: shown was `['Daddy Yankee', 'Travis Scott']`, expected Bad Bunny first. +- E `keeps a sticky card when Manage tags deletes one of two selected tags (WR-03)`: same received list, Bad Bunny missing. + +## Design Notes + +**Why a render-time reset and not a derived comparison.** The reviewer's sketch derives `ids` only while `sticky.key === filterKey`. That revives a sticky card when the user returns to an earlier filter key (toggle on, then off), which breaks G4. Resetting the state during render (React's "adjust state while rendering" pattern) makes any key change final; `carryTo` is how the route's own rewrites opt out. Test C pins this. + +**Carry lifetime.** A carry belongs to the one write that made it: +- The router location key (`useLocation().key`, fresh per navigation) lets the first landing consume it, even a landing that leaves the filter unchanged. +- `useNavigationType() === NavigationType.Replace` limits it to a replace landing; every route-own rewrite is `{ replace: true }`, G4 writes push, and Back/Forward pop. +- The write's starting filter key must equal the reconciled key (`carrySticky(prev, from, to)`), so a write layered on an unlanded G4 change in the same tick never carries, nor does a write back onto the current key. +The transitions are pure functions in `watchlistView.ts` because same-tick write ordering cannot be driven deterministically through the route in jsdom; unit test F covers the stale-carry sequence. + +## Deviations from Plan + +None - plan executed exactly as written. (The `react-hooks/exhaustive-deps` disable reason on the canonicalization effect was reworded to name `writeKeepingSticky`, as the plan allowed.) + +## Verification + +- `vitest run app/lib/watchlistView.test.ts app/components/watchlist/TagChips.test.tsx app/routes/watchlist.test.tsx`: 163 passed. +- `corepack pnpm --dir web run typecheck`: clean. +- Per task commit: `prettier --check "**/*.{ts,tsx}"` clean and full `corepack pnpm --dir web test`: 31 files, 596 tests passed, coverage thresholds met (590 tests after Task 1). +- Grep gates: no `clearSticky`/`writeKeepingSticky` in the wrong handlers, `prevFilterKey` count 0, `useScrollAnchor(filterKey)` once, four `writeKeepingSticky(` occurrences, `watchlistView.ts` has no router import. +- `git diff --name-only 7ae9ace HEAD` lists only the four plan files (no `useUrlParams.ts`, `package.json`, or `pnpm-lock.yaml`). +- Go gates, the bundle rebuild, and the real-browser re-check are deferred to 25-18 as planned (no Go file changed). + +## Known Stubs + +None. + +## Threat Flags + +None. The new code only changes when client-side sticky state resets; no new endpoint, auth path, or trust boundary (T-25-41..43 mitigated as planned). + +## Next + +Ready for 25-16. + +## Self-Check: PASSED + +- FOUND: web/app/routes/watchlist.tsx, web/app/routes/watchlist.test.tsx, web/app/lib/watchlistView.ts, web/app/lib/watchlistView.test.ts +- FOUND commits: 9057be9, 6bb851f diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-16-PLAN.md b/.planning/phases/25-find-filter-watchlist-and-history/25-16-PLAN.md new file mode 100644 index 00000000..ccc5d587 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-16-PLAN.md @@ -0,0 +1,209 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 16 +type: execute +wave: 1 +depends_on: [] +files_modified: + - web/app/components/watchlist/SearchResultsColumns.tsx + - web/app/components/watchlist/SearchResultsColumns.test.tsx +autonomous: true +gap_closure: true +gap_ids: [G-25-9] +review_ids: [WR-04] +requirements: [WLVW-01] + +estimate: + tokens: 30000 + raw_tokens: 30000 + tasks: 1 + confidence: low + +must_haves: + truths: + - "User decision 2026-10-11 (gap planning), WR-04: on a Deezer search-result row, the explanatory `title` and the width cap `max-w-[40%] shrink-0 lg:max-w-none` sit on a wrapper `` around the disabled `Search MusicBrainz to add` button. The wrapper receives pointer events, so hovering the truncated pill below lg shows the full hint, even though the disabled button has `disabled:pointer-events-none`." + - "The button keeps `variant=\"secondary\"`, `size=\"sm\"`, and `disabled`, gains `w-full`, and keeps its single inner `Search MusicBrainz to add`. It no longer carries the cap or the title. Its accessible name stays exactly `Search MusicBrainz to add`." + - "G-25-9 still holds: below lg the wrapper is capped at 40% of the row and the label truncates, so the artist name keeps the row; from lg up the full label shows. The `Add to Watchlist` and `Already watching` buttons and `web/app/components/ui/button.tsx` are unchanged." + artifacts: + - path: web/app/components/watchlist/SearchResultsColumns.tsx + provides: "`DEEZER_ADD_HINT` module constant; Deezer branch wraps the disabled button in a titled, width-capped span" + contains: "DEEZER_ADD_HINT" + - path: web/app/components/watchlist/SearchResultsColumns.test.tsx + provides: "G-25-9/WR-04 test asserting the wrapper's title and cap, and the button's w-full and unchanged accessible name" + key_links: + - from: "wrapper span title" + to: "native tooltip on hover" + via: "the wrapper has no pointer-events-none, so hover over the disabled (pointer-events-none) button reaches the wrapper" + pattern: "title=\\{DEEZER_ADD_HINT\\}" + prohibitions: + - statement: "MUST NOT edit the shared web/app/components/ui/button.tsx base." + requirement_id: WLVW-01 + category: safety + verification: test + - statement: "MUST NOT change the button's text, its accessible name, or the hint copy." + requirement_id: WLVW-01 + category: safety + verification: test +--- + + +Apply the user's WR-04 decision: the truncated `Search MusicBrainz to add` pill on Deezer rows must have a reachable explanation. + +Review finding (25-REVIEW WR-04): below lg the Deezer button is capped at 40% of the row and its label truncates (G-25-9, plan 25-13). +The only full explanation is the button's `title`. `buttonVariants` includes `disabled:pointer-events-none`, and this button is always +`disabled`, so the native tooltip never appears. + +Locked decision (binding). Cite it as "user decision 2026-10-11 (gap planning)": fix with a wrapper title. Move `title` and the +`max-w-[40%] shrink-0 lg:max-w-none` cap onto a wrapper span that receives pointer events. The Button gets `w-full` and keeps the inner +truncating span. Update SearchResultsColumns.test.tsx to assert the wrapper's title. The accessible name is unchanged. + +Requirement mapping: WR-04 belongs with the search/add surface. It follows 25-13 (G-25-9), which is mapped to WLVW-01, and +25-VERIFICATION records G-25-9 under WLVW-01 as add-flow polish. No other WLVW requirement covers the add-from-search row. + +This is a single-task plan: the fix is one element and its test. Splitting the RED test from the GREEN change into two tasks would only +add a commit boundary inside one TDD cycle. No external API is called. The row stays a disabled hint, and Deezer data handling is +untouched. + +Spec-less probe fallback: visibly skipped for this run. This is gap_closure mode against an executed phase with no SPEC.md, so the +REVIEW finding carries the edge coverage. + +Commit discipline (CLAUDE.md): run `corepack pnpm --dir web exec prettier --write "**/*.{ts,tsx}"` before staging. The commit must +clear CLAUDE.md DoD item 6: `corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}"` and the full `corepack pnpm --dir web test` +(coverage thresholds included). The task's verify runs both, and the commit lands only after GREEN, never on the RED state alone. Go +gates are deferred to 25-18, recorded: this plan changes no Go file. Stage only this plan's two files, because wave 1 shares the main +tree with 25-15 (`watchlist.tsx`, `watchlist.test.tsx`, `watchlistView.ts`, `watchlistView.test.ts`), and `use_worktrees: false` +runs the two plans one after the other. If typecheck, the prettier check, or the full suite fails only in 25-15's files while they +are uncommitted, wait for that plan's commit and re-run. Never edit them. Keep comments to 1-2 lines with one design-doc reference. +Hooks stay on, and no AI attribution trailer. + + + +@~/.claude/gsd-core/workflows/execute-plan.md +@~/.claude/gsd-core/templates/summary.md + + + +@.planning/phases/25-find-filter-watchlist-and-history/25-REVIEW.md +@web/app/components/watchlist/SearchResultsColumns.tsx + + + +Current shapes (verified at planning head `8ccde67`): + +- `web/app/components/watchlist/SearchResultsColumns.tsx` lines 187-216, `SearchResultRow`: `alreadyWatching` renders a disabled + `Already watching` Button with `shrink-0`. `!canAdd` (the Deezer branch, 191-201) renders a ``. +- `web/app/components/watchlist/SearchResultsColumns.test.tsx`: + - The test at lines 167-200 asserts the wrapper is a SPAN with `max-w-[40%] shrink-0 lg:max-w-none`, no `pointer-events-none`, and a title containing "MusicBrainz identity". It also asserts the button is disabled with `w-full`, no `max-w-[40%]`, and `not.toHaveAttribute("title")`, and that `button.children` has length 1 (the truncating label span). + - The test at lines 289-309 asserts the Deezer row's `textContent` equals `` `${deezerArtist.name}${deezerArtist.type}Search MusicBrainz to add` ``. + - The `deezerArtist` fixture is at lines 33-41. + - @testing-library/jest-dom 7.0.1 provides `toHaveAccessibleDescription`. +- `25-UI-SPEC.md` line 188 (inside the page-order code block) starts ` SearchBox "Search artists" + SearchResultsColumns` and ends with `... so it shows on hover (WR-04, user decision 2026-10-11))`. After 25-19 and 25-20, the UI-SPEC diff against `9d3413f` is one removed and two added lines. +- Embedded bundle marker baselines under `internal/webassets/build/client/assets` at `9d3413f` (via `git grep -o -F`): `parentElement` 9, `data-chip-label` 0, `aria-describedby` 3. Minification keeps DOM property names (`parentElement`) and JSX attribute keys (`data-chip-label`, `aria-describedby`). Plans 25-19 and 25-20 don't touch the bundle, so the tree is unchanged between `9d3413f` and the start of this plan. +- `Makefile`: + - The `web` target (line 139) runs a frozen install, a build, and then replaces `internal/webassets/build/client`. + - `SQLC_VERSION := v1.31.1`. + - `sqlc-check` depends on `sqlc-version-check`. + + + + + + Task 1: The disabled Deezer button announces its reason through aria-describedby + web/app/components/watchlist/SearchResultsColumns.tsx, web/app/components/watchlist/SearchResultsColumns.test.tsx + + - `web/app/components/watchlist/SearchResultsColumns.tsx` lines 1-12 and 127-221 (imports, DEEZER_ADD_HINT, SearchResultRow). + - `web/app/components/watchlist/SearchResultsColumns.test.tsx` lines 1-80 (fixtures), 146-248 (Deezer and uncapped-button tests), and 289-309 (row text test). + + + - The disabled `Search MusicBrainz to add` button has the accessible description DEEZER_ADD_HINT, and its accessible name is still `Search MusicBrainz to add`. + - The existing assertions on the button and wrapper still hold: the wrapper is a SPAN with the 40% cap classes and the hint title, and the button has no title, `w-full`, and exactly one child, the truncating label span. + - The description element has class `sr-only`, contains exactly DEEZER_ADD_HINT, sits inside the row, and is not inside the button. + - With two Deezer results, the two buttons' `aria-describedby` values differ, and each resolves via `document.getElementById` to an element whose text is DEEZER_ADD_HINT. + - The `Add to Watchlist` and `Already watching` buttons have no `aria-describedby`. + - The Deezer row's `textContent` is the name, then the type, then `Search MusicBrainz to add`, then DEEZER_ADD_HINT. The sr-only text is part of the row's text content. + + +Per user decision 2026-10-12 (G-25-13, review WR-02: add aria-describedby pointing at an sr-only hint). + +RED first. Update and add the tests from the behavior block, importing `DEEZER_ADD_HINT` from `./SearchResultsColumns`. Run the +first verify command and confirm that the description assertions fail. Then implement in `SearchResultsColumns.tsx`: +- Export `DEEZER_ADD_HINT`, so the test asserts the exact string. +- Import `useId` alongside `useState`, and call `const hintId = useId()` once at the top of `SearchResultRow`. Hooks run + unconditionally, so call it before any branch. +- On the disabled Deezer `Button`, add `aria-describedby={hintId}`. +- Inside the wrapper span, directly after that `Button`, render a span with `id={hintId}` and `className="sr-only"` whose only child + is `{DEEZER_ADD_HINT}` as plain JSX text. +- Keep the wrapper's `title` and classes exactly as they are, and leave the label span inside the button untouched. +- Add nothing visible, and use no other attribute changes. +- Amend the `// WR-04:` comment to say, in at most two lines, that `aria-describedby` gives the button the same hint for screen + readers (G-25-13). + +In the test at lines 167-200, keep every existing assertion and add `expect(button).toHaveAccessibleDescription(DEEZER_ADD_HINT)`. +Add a test with two Deezer artists that checks distinct, resolvable ids. In the uncapped-buttons test, add assertions that `Add to +Watchlist` and `Already watching` have no `aria-describedby`. Update the expected `textContent` in the lines 289-309 test to append +`DEEZER_ADD_HINT`. + +Before staging, run `corepack pnpm --dir web exec prettier --write "**/*.{ts,tsx}"`, then run the remaining verify commands +(CLAUDE.md DoD item 6). Commit once after GREEN, with hooks enabled and no AI attribution, for example `fix(25-21): describe the +disabled Deezer button with its MusicBrainz hint`. + + + corepack pnpm --dir web exec vitest run app/components/watchlist/SearchResultsColumns.test.tsx --coverage.enabled=false + corepack pnpm --dir web run typecheck && grep -qF 'export const DEEZER_ADD_HINT' web/app/components/watchlist/SearchResultsColumns.tsx && grep -qF 'useId()' web/app/components/watchlist/SearchResultsColumns.tsx && grep -qF 'aria-describedby={hintId}' web/app/components/watchlist/SearchResultsColumns.tsx && grep -qF 'toHaveAccessibleDescription(DEEZER_ADD_HINT)' web/app/components/watchlist/SearchResultsColumns.test.tsx && grep -qF 'title={DEEZER_ADD_HINT}' web/app/components/watchlist/SearchResultsColumns.tsx + corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}" && corepack pnpm --dir web test + non-zero exit: a failing SearchResultsColumns test (before the change the description assertion fails); an "error TS" line; the export, `useId()`, `aria-describedby={hintId}`, the accessible-description assertion, or the wrapper title missing; prettier listing files under "Code style issues"; any "FAIL" line; or a line beginning "ERROR: Coverage for" + + + - `expect(button).toHaveAccessibleDescription(DEEZER_ADD_HINT)` passes, and `getByRole("button", { name: "Search MusicBrainz to add" })` still finds the button. + - Two Deezer rows get distinct `aria-describedby` ids that each resolve to the hint text. + - The wrapper keeps `title={DEEZER_ADD_HINT}` and its cap classes, and the button keeps no title and exactly one child. + - Before the commit, `corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}"` and the full `corepack pnpm --dir web test` both exit 0 (CLAUDE.md DoD item 6). + + Screen readers announce why the Deezer button is unavailable, with no visual change. Committed. + + + + Task 2: Record the accessible description on the UI-SPEC page-order SearchResultsColumns line (one line) + .planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md + + - `.planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md` lines 184-200 (page-order code block; line 188 is the SearchResultsColumns note). + + +Make one scoped `Edit` on line 188, the line starting ` SearchBox "Search artists" + SearchResultsColumns`, per user decision +2026-10-12 (G-25-13). Never use a whole-file `Write`. Replace the line's closing text `(WR-04, user decision 2026-10-11))` with exactly: + +`(WR-04, user decision 2026-10-11); the disabled button also carries that hint as its accessible description, through aria-describedby on an sr-only span with a per-row useId id, with no visual change (G-25-13, user decision 2026-10-12))` + +Keep the line's leading whitespace and the rest of the text unchanged, with no wrap. Change nothing else in the UI-SPEC, and leave +25-CONTEXT.md, 25-UAT.md, and 25-VERIFICATION.md alone. Commit the doc alone with hooks enabled and no AI attribution, for example +`docs(25-21): record the Deezer hint's accessible description in the UI-SPEC`. + + + f=.planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md; [ "$(grep -c 'SearchBox "Search artists" + SearchResultsColumns' "$f")" -eq 1 ] && grep -F 'SearchBox "Search artists" + SearchResultsColumns' "$f" | grep -qF 'aria-describedby on an sr-only span with a per-row useId id, with no visual change (G-25-13, user decision 2026-10-12))' && grep -qF 'widened to the nearest surviving card by G-25-11, user decision 2026-10-12' "$f" && grep -q '^| Card chip label | Toggled, card leaves |.*G-25-12, user decision 2026-10-12' "$f" && d=$(git diff -U0 9d3413f -- "$f") && removed=$(printf '%s\n' "$d" | grep -E '^-' | grep -vE '^--- (a/|/dev/null)' | wc -l) && added=$(printf '%s\n' "$d" | grep -E '^\+' | grep -vE '^\+\+\+ (b/|/dev/null)' | wc -l) && echo "removed=$removed added=$added" && test "$removed" -eq 2 && test "$added" -eq 3 && test -z "$(git diff --name-only 9d3413f -- .planning/phases/25-find-filter-watchlist-and-history/25-CONTEXT.md .planning/phases/25-find-filter-watchlist-and-history/25-VERIFICATION.md)" && test -z "$(git diff --name-only 1cb2a7b -- .planning/phases/25-find-filter-watchlist-and-history/25-UAT.md)" + non-zero exit: the page-order line is missing, duplicated, or lacks the G-25-13 text; 25-19's D-12 line or 25-20's focus row is gone; the cumulative UI-SPEC diff against `9d3413f` is not exactly two removed and three added lines; CONTEXT or VERIFICATION changed since `9d3413f`; or UAT changed since `1cb2a7b` (the commit that recorded the 2026-10-12 race waiver) + + + - The single page-order SearchResultsColumns line ends with the G-25-13 accessible-description clause. + - `git diff -U0 9d3413f -- .planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md` shows exactly two removed and three added content lines, from 25-19, 25-20, and this task. + - 25-CONTEXT.md and 25-VERIFICATION.md are unchanged since `9d3413f`, and 25-UAT.md is unchanged since `1cb2a7b`. + + The UI design contract states the Deezer hint's accessible description. Committed. + + + + Task 3: Rebuild the embedded SPA, run the Definition of Done, and confirm the blast radius + Tasks 1-2 and plans 25-19 and 25-20 are committed (`git status --porcelain web/` prints nothing), the dev Postgres answers on localhost:5432 (`docker compose ps postgres` healthy), and `corepack pnpm --dir web install --frozen-lockfile` can resolve from the local store or the registry. + internal/webassets/build/client + + - `.claude/CLAUDE.md` "Definition of Done" (gate order; never `--no-verify`). + - `Makefile` `web` target (line 139), `test-integration` (`COVER_PKGS`), `coverage-gate`, and `sqlc-check` recipes. + - `.planning/phases/25-find-filter-watchlist-and-history/25-18-PLAN.md` Tasks 1 and 3 (the same rebuild, marker method, and gate chain) and `25-18-SUMMARY.md` "Accepted exceptions" (section format only). + - `.planning/phases/25-find-filter-watchlist-and-history/25-UAT.md` ## Execution Waivers (the 2026-10-12 "CI-only race extended" line is the source of this plan's race exception). + + +Rebuild by running the `make web` recipe directly: +- `corepack pnpm --dir web install --frozen-lockfile` +- `corepack pnpm --dir web run build` +- `rm -rf internal/webassets/build/client` +- `mkdir -p internal/webassets/build` +- `cp -r web/build/client internal/webassets/build/client` + +If the install modifies `web/package.json` or `web/pnpm-lock.yaml`, restore both with `git checkout --` and stop. + +Confirm that the bundle carries each fix by comparing marker counts against the `9d3413f` tree (second verify command). Each count +must be strictly greater than its baseline: +- `parentElement` comes from 25-19's `anchorCandidates`. +- `data-chip-label` comes from 25-20's chip label attribute and focus selector. +- `aria-describedby` comes from this plan's Deezer button. + +The vendor chunks can't account for an increase, because the lockfile is frozen and the blast-radius check pins the package files. + +Then run the Definition of Done in order, before committing: +1. `go vet ./...` +2. `golangci-lint run` (from `$HOME/go/bin`) +3. The integration suite with a coverage profile, using the Makefile's `COVER_PKGS` equivalent, without `-race`. This is the Go chain + in the third verify command, which is 25-18's chain with only the log name changed. +4. The coverage gate: `go run ./cmd/coverage-report --mode=total --profile=coverage.out` must print 80 or more. +5. The full `sqlc-check` recipe, including its `sqlc-version-check` guard first. `sqlc version` must equal the Makefile's + `SQLC_VERSION` before `sqlc generate` runs. Then `git diff --exit-code -- internal/db/sqlc/` must be clean. +6. `corepack pnpm --dir web run typecheck`, then `corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}"`, then + `corepack pnpm --dir web test` (70% on all four axes). + +If a gate goes red, fix it in the file of the plan that owns it (25-19, 25-20, or this plan's Task 1), commit that fix separately, +rebuild the bundle again, and re-run. Then confirm the blast radius against `9d3413f` (fifth verify command). + +Commit the refreshed tree with hooks enabled and no AI attribution, for example `build(25-21): rebuild the embedded SPA with the +G-25-11..13 fixes`. The deleted files are superseded hashed assets, which is intended. + +In the SUMMARY, record: +- The `before -> after` marker counts. +- An `Accepted exceptions` heading carrying the line `Accepted exception: CI-only race (user decision 2026-10-12, 25-UAT.md)`. The + source is the 25-UAT.md ## Execution Waivers line "CI-only race extended", which covers 25-19..25-21. The reason: cgo is disabled + locally, no Go source changed in 25-19 to 25-21, and CI's Linux test job runs `-race`. + + + corepack pnpm --dir web install --frozen-lockfile && corepack pnpm --dir web run build && rm -rf internal/webassets/build/client && mkdir -p internal/webassets/build && cp -r web/build/client internal/webassets/build/client && out=$(git status --porcelain internal/webassets/build/client) && test -n "$out" + non-zero exit from the install, build, or copy; or `test -n` failing because the bundle did not change + ok=1; git cat-file -e 9d3413f:internal/webassets/build/client/assets || ok=0; for m in 'parentElement' 'data-chip-label' 'aria-describedby'; do bo=$(git grep -o -F -e "$m" 9d3413f -- internal/webassets/build/client/assets; test $? -le 1) || ok=0; b=$(printf '%s' "$bo" | grep -c .); n=$(grep -r -o -F -e "$m" internal/webassets/build/client/assets | wc -l); echo "$m: $b -> $n"; [ "$n" -gt "$b" ] || ok=0; done; [ "$ok" = 1 ] && go build ./... + non-zero exit: the `9d3413f` baseline tree can't be read (a git error, never counted as zero); a marker's count in the rebuilt assets is not strictly greater than at `9d3413f` (the printed `before -> after` line names it; on the pre-change tree the check fails with `data-chip-label: 0 -> 0`); or the embed doesn't compile + go vet ./... && PATH="$HOME/go/bin:$PATH" golangci-lint run && { TEST_DATABASE_URL='postgres://drop_tracker:drop_tracker@localhost:5432/drop_tracker?sslmode=disable' go test ./... -count=1 -coverprofile=coverage.out -coverpkg="$(go list ./... | grep -vE '(^|/)(internal/db/sqlc|cmd/coverage-report|cmd/migrate|cmd/migration-check)$' | paste -sd, -)" > "${TMPDIR:-/tmp}/gotest-25-21.log" 2>&1; true; } && ! grep -E '^(FAIL|--- FAIL)' "${TMPDIR:-/tmp}/gotest-25-21.log" | grep -v 'internal/testutil' && cov=$(go run ./cmd/coverage-report --mode=total --profile=coverage.out) && echo "Backend coverage: $cov" && awk -v c="$cov" 'BEGIN { exit !(c + 0 >= 80) }' && want=$(sed -n 's/^SQLC_VERSION := //p' Makefile) && got=$(PATH="$HOME/go/bin:$PATH" sqlc version) && test -n "$want" && { [ "$got" = "$want" ] || { echo "sqlc version mismatch: want $want, got $got" >&2; false; }; } && PATH="$HOME/go/bin:$PATH" sqlc generate && git diff --exit-code -- internal/db/sqlc/ + any vet or lint diagnostic; any `FAIL` or `--- FAIL` line in the test log other than internal/testutil's known `go: no such tool "covdata"` failure; coverage below 80; `SQLC_VERSION` missing from the Makefile or a `sqlc version mismatch` (the guard runs before generate); or a diff under internal/db/sqlc/ + corepack pnpm --dir web run typecheck && corepack pnpm --dir web exec prettier --check "**/*.{ts,tsx}" && corepack pnpm --dir web test + non-zero exit, any "error TS" line, prettier listing files under "Code style issues", any "FAIL" line, or a line beginning "ERROR: Coverage for" + out=$(git diff --name-only 9d3413f -- '*.go' queries internal/db internal/events go.mod go.sum web/package.json web/pnpm-lock.yaml web/app/components/ui web/app/lib/useUrlParams.ts web/app/root.tsx .planning/phases/25-find-filter-watchlist-and-history/25-CONTEXT.md .planning/phases/25-find-filter-watchlist-and-history/25-VERIFICATION.md) && test -z "$out" && test -z "$(git diff --name-only 1cb2a7b -- .planning/phases/25-find-filter-watchlist-and-history/25-UAT.md)" && grep -qF 'CI-only race extended (user decision 2026-10-12)' .planning/phases/25-find-filter-watchlist-and-history/25-UAT.md && changed=$(git diff --name-only 9d3413f -- web/app) && extra=$(printf '%s\n' "$changed" | grep -vxE 'web/app/(lib/useScrollAnchor\.ts|lib/useScrollAnchor\.test\.tsx|routes/watchlist\.tsx|routes/watchlist\.test\.tsx|components/watchlist/TagChips\.tsx|components/watchlist/TagChips\.test\.tsx|components/watchlist/SearchResultsColumns\.tsx|components/watchlist/SearchResultsColumns\.test\.tsx)'; true) && echo "unexpected web/app changes: ${extra:-none}" && test -z "$extra" + non-zero exit: a Go source file, the SQL, generated code, the events service, a migration, a dependency file, the vendored ui/ components, useUrlParams, root.tsx, 25-CONTEXT.md, or 25-VERIFICATION.md changed since `9d3413f`; 25-UAT.md changed since `1cb2a7b`, or its 2026-10-12 "CI-only race extended" waiver line is missing; or a web/app file outside the eight gap-plan files changed (the printed paths name it) + + Run the built binary (or `docker compose up` with the rebuilt image) against the dev DB with 50+ artists. On the Watchlist, scrolled far down: (1) at `?tags=A,B`, un-press a chip on a card whose next and previous cards also carry only that tag (G-25-11); (2) repeat (1) with the keyboard (Tab to the chip label, press Enter), then repeat it on a filter where no card survives, and confirm that the mouse click in (1) moved no focus (G-25-12, keyboard only); (3) search `Bad Bunny` and move a screen reader (NVDA or VoiceOver) onto a Deezer row's disabled button, then hover the truncated pill (G-25-13). + (1) The nearest surviving card stays where it was on screen, clamped only when the filtered page is shorter than the window. (2) The focus ring lands on that surviving card's pressed chip label without moving the page. When nothing survives, focus lands on the `Your artists` heading, never on the top of the page. After the mouse click in (1), no focus ring appears on another card or on the heading. (3) The screen reader announces `Search MusicBrainz to add, button, unavailable` followed by the hint, with no visible change to the row, and hovering still shows the native tooltip. + Real-browser layout, focus rings, and assistive-tech output (human_verify_mode: end-of-phase). jsdom stubs layout and has no screen reader. These are the /gsd-verify-work re-test items for G-25-11, G-25-12, and G-25-13. + + + + - `git status --porcelain internal/webassets/build/client` is non-empty before staging, and `go build ./...` exits 0. + - Each of `parentElement`, `data-chip-label`, and `aria-describedby` occurs more often under `internal/webassets/build/client/assets` than in the `9d3413f` tree. + - Backend coverage prints 80 or more, and the web suite reports 70% or more on statements, branches, functions, and lines. + - The sqlc version guard passes before `sqlc generate` runs, and the sqlc diff is clean. + - Both blast-radius checks against `9d3413f` are empty, and 25-UAT.md is unchanged since `1cb2a7b` and still carries the "CI-only race extended" waiver. + - The SUMMARY records `Accepted exception: CI-only race (user decision 2026-10-12, 25-UAT.md)` with its reason, plus the marker counts. + + The Go binary serves G-25-11, G-25-12, and G-25-13, every Definition of Done gate is green, the gap closure touched only its intended surface, and the re-test items are ready for /gsd-verify-work. Committed. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| Third-party catalogue strings (Deezer) → rendered row | Artist name, type, and country render as plain JSX text (unchanged). The new hint is a static constant | +| Package registry → `pnpm install --frozen-lockfile` | Build-time dependency resolution for the bundle | +| Built bundle → go:embed → served SPA | The committed tree is what the binary serves | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-25-56 | Tampering (XSS) | Deezer hint span | low | mitigate | The span's only child is the static `DEEZER_ADD_HINT` constant, rendered as JSX text. No catalogue string feeds the description, and the id comes from React's `useId`, not from a Deezer value | +| T-25-SC | Tampering | npm installs during the rebuild | high | mitigate | No package is added. The rebuild uses `--frozen-lockfile`, and the blast-radius diff asserts that `web/package.json` and `web/pnpm-lock.yaml` are unchanged since `9d3413f` | +| T-25-57 | Tampering | Stale embedded bundle | medium | mitigate | The rebuild is verified on its own. The bundle must change, and each fix's marker (`parentElement`, `data-chip-label`, `aria-describedby`) must occur more often than in the `9d3413f` tree | +| T-25-58 | Elevation / scope creep | Go source, SQL, vendored components, decision and verification docs | low | mitigate | The blast-radius diff against `9d3413f` over `*.go`, queries/, internal/db, internal/events, ui/, useUrlParams.ts, root.tsx, 25-CONTEXT.md, and 25-VERIFICATION.md must be empty, 25-UAT.md must be unchanged since `1cb2a7b`, and web/app changes must stay within the eight gap-plan files | +| T-25-59 | Repudiation | Spec drift | low | mitigate | Task 2's verify fails unless the cumulative UI-SPEC diff against `9d3413f` is exactly two removed and three added lines carrying the G-25-11, G-25-12, and G-25-13 tags | + + + +- `corepack pnpm --dir web exec vitest run app/components/watchlist/SearchResultsColumns.test.tsx --coverage.enabled=false` passes, including `toHaveAccessibleDescription(DEEZER_ADD_HINT)`. +- The UI-SPEC carries the three G-25-11/12/13 lines, with a cumulative diff of two removed and three added lines against `9d3413f`. +- The bundle carries all three markers above their `9d3413f` counts, and `go build ./...` compiles the embed. +- The Go chain, the web chain, and both blast-radius checks exit 0, apart from the documented covdata line. +- After the SUMMARY is written: `grep -qF 'Accepted exception: CI-only race (user decision 2026-10-12, 25-UAT.md)' .planning/phases/25-find-filter-watchlist-and-history/25-21-SUMMARY.md` exits 0. + + + +Screen readers announce the Deezer button's reason with no visual change (G-25-13). The Go binary serves the widened fallback anchor +(G-25-11) and the focus move (G-25-12), the UI design contract states all three decisions, every CLAUDE.md gate is green, and the +three gaps are ready for re-verification. + + + +Create `.planning/phases/25-find-filter-watchlist-and-history/25-21-SUMMARY.md` when done. Its `Accepted exceptions` section must carry +the line `Accepted exception: CI-only race (user decision 2026-10-12, 25-UAT.md)` with the reason, citing the 25-UAT.md ## Execution +Waivers line "CI-only race extended" as its source. Also list the `before -> after` marker counts. + diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-21-SUMMARY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-21-SUMMARY.md new file mode 100644 index 00000000..036f6422 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-21-SUMMARY.md @@ -0,0 +1,150 @@ +--- +phase: 25-find-filter-watchlist-and-history +plan: 21 +subsystem: ui +tags: [react, a11y, aria-describedby, go-embed, gap-closure, ship] + +requires: + - phase: 25-find-filter-watchlist-and-history + provides: "Fallback scroll anchor (25-19) and keyboard focus move (25-20)" +provides: + - "Disabled Deezer button announces DEEZER_ADD_HINT through aria-describedby on a per-row useId sr-only span" + - "25-UI-SPEC page-order SearchResultsColumns note records the accessible description (G-25-13)" + - "Rebuilt internal/webassets/build/client carrying the G-25-11, G-25-12, and G-25-13 fixes" +affects: [25-verify-work] + +actuals: + tokens: 6000 + tasks: 3 + commits: 3 +plan_head_before: 92288414c53a268e16651667f9f1fb2136412eb5 + +tech-stack: + added: [] + patterns: + - "Per-row useId hint id with sr-only description beside a disabled control; wrapper keeps the hover title" + +key-files: + created: [] + modified: + - web/app/components/watchlist/SearchResultsColumns.tsx + - web/app/components/watchlist/SearchResultsColumns.test.tsx + - .planning/phases/25-find-filter-watchlist-and-history/25-UI-SPEC.md + - internal/webassets/build/client + +key-decisions: + - "G-25-13 (user decision 2026-10-12, review WR-02): describe the disabled Deezer button via aria-describedby to an sr-only hint; no visible note, wrapper title and 40% cap kept." + +requirements-completed: [WLVW-01, WLVW-04] + +coverage: + - id: D1 + description: "Disabled Deezer button has accessible description DEEZER_ADD_HINT, name unchanged, per-row unique ids, no description on other buttons" + requirement: WLVW-01 + verification: + - kind: unit + ref: "web/app/components/watchlist/SearchResultsColumns.test.tsx#caps the Deezer hint wrapper, titles it, and truncates the button label (G-25-9, WR-04)" + status: pass + - kind: unit + ref: "web/app/components/watchlist/SearchResultsColumns.test.tsx#describes each Deezer button with its own sr-only hint (G-25-13)" + status: pass + human_judgment: false + - id: D2 + description: "Embedded SPA bundle carries all three gap fixes and the Go embed compiles" + requirement: WLVW-04 + verification: + - kind: other + ref: "marker counts vs 9d3413f (parentElement 9->10, data-chip-label 0->3, aria-describedby 3->4) plus go build ./..." + status: pass + human_judgment: false + - id: D3 + description: "Real screen reader announcement, browser focus ring, and scroll anchoring on the rebuilt binary" + requirement: WLVW-04 + verification: [] + human_judgment: true + rationale: "Real-browser layout, focus rings, and assistive-tech output; jsdom has no screen reader. /gsd-verify-work re-test items for G-25-11, G-25-12, G-25-13." + +duration: 9min +completed: 2026-10-12 +status: complete +--- + +# Phase 25 Plan 21: Deezer hint accessible description and SPA rebuild Summary + +**The disabled `Search MusicBrainz to add` button now announces its reason through `aria-describedby` to a per-row `useId` sr-only span (no visual change), and the embedded SPA is rebuilt so the Go binary serves the G-25-11, G-25-12, and G-25-13 fixes.** + +## Performance + +- **Duration:** about 9 min +- **Completed:** 2026-10-12T22:28Z +- **Tasks:** 3 +- **Files modified:** 4 paths (bundle directory counts as one) + +## Accomplishments +- Closed G-25-13 (review WR-02): `DEEZER_ADD_HINT` is exported, `SearchResultRow` calls `useId()` once, and the disabled button points at an sr-only sibling span. The wrapper title, 40% cap classes, and the button's single truncating label child are untouched. +- 25-UI-SPEC page-order note records the accessible description, tagged `G-25-13, user decision 2026-10-12`; cumulative UI-SPEC diff against `9d3413f` is two removed and three added lines. +- `internal/webassets/build/client` rebuilt from the frozen lockfile; `go build ./...` compiles the embed. + +## Task Commits + +1. **Task 1: Disabled Deezer button announces its reason through aria-describedby** - `69d6f9e` (fix, tracer) +2. **Task 2: UI-SPEC accessible-description note** - `8d24c8a` (docs) +3. **Task 3: Rebuild the embedded SPA** - `5358e5b` (build) + +## RED evidence + +Before the change, `vitest run app/components/watchlist/SearchResultsColumns.test.tsx` gave 3 failed: the `toHaveAccessibleDescription` assertion, the two-row id test (`aria-describedby` null), and the row `textContent` test (the sr-only hint was absent). + +## Marker counts (embedded bundle, `9d3413f` -> now) + +- `parentElement`: 9 -> 10 (25-19) +- `data-chip-label`: 0 -> 3 (25-20) +- `aria-describedby`: 3 -> 4 (25-21) + +## Definition of Done results + +- `go vet ./...`: clean. `golangci-lint run`: 0 issues. +- Integration suite with coverage profile (no `-race`): no `FAIL` lines at all, including `internal/testutil`; backend coverage 90.32% (gate 80). +- `sqlc-check`: `sqlc version` v1.31.1 matched `SQLC_VERSION` before generate; `git diff --exit-code -- internal/db/sqlc/` clean. +- Web: typecheck clean, prettier check clean, 622 tests pass; coverage statements 95.34%, branches 90.84%, functions 96.12%, lines 97.16% (gate 70% on all four). +- Blast radius against `9d3413f`: empty for `*.go`, queries/, internal/db, internal/events, go.mod, go.sum, web/package.json, web/pnpm-lock.yaml, ui/, useUrlParams.ts, root.tsx, 25-CONTEXT.md, 25-VERIFICATION.md. Changed `web/app` files are only the eight gap-plan files. 25-UAT.md unchanged since `1cb2a7b` and still carries the "CI-only race extended" waiver. + +## Accepted exceptions + +Accepted exception: CI-only race (user decision 2026-10-12, 25-UAT.md). Source: the 25-UAT.md ## Execution Waivers line "CI-only race extended", which covers 25-19..25-21. Reason: cgo is disabled locally, no Go source changed in 25-19 to 25-21 (blast-radius check pins every `*.go` file), and CI's Linux test job runs `-race`. + +## Decisions Made +- Followed the locked G-25-13 decision exactly; the review's optional visible touch note was not added (UAT truth says no visual change). + +## Deviations from Plan + +None - plan executed exactly as written. + +## Issues Encountered + +None. + +## Known Stubs + +None. + +## Threat Flags + +None. The hint span's only child is the static constant (T-25-56); no package changed (T-25-SC); all three markers grew in the bundle (T-25-57); blast radius and UI-SPEC diff verified (T-25-58, T-25-59). + +## User Setup Required + +None - no external service configuration required. + +## Next Phase Readiness + +- Ready for `/gsd-verify-work` re-test of G-25-11, G-25-12, and G-25-13 (human-check: built binary against 50+ artists, keyboard chip toggle, screen reader on a Deezer row). + +## Self-Check: PASSED + +- Commits `69d6f9e`, `8d24c8a`, `5358e5b` exist; `git rev-list --count 9228841..HEAD` = 3. +- Modified files and the rebuilt bundle exist; the acceptance criteria for all three tasks re-ran green. + +--- +*Phase: 25-find-filter-watchlist-and-history* +*Completed: 2026-10-12* diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-CONTEXT.md b/.planning/phases/25-find-filter-watchlist-and-history/25-CONTEXT.md index 2b0ecb17..41d2c5cd 100644 --- a/.planning/phases/25-find-filter-watchlist-and-history/25-CONTEXT.md +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-CONTEXT.md @@ -50,7 +50,7 @@ These come from a grill session after discuss and the UI phase. Where they confl - **G1 (supersedes D-14's retention clause and the roadmap's PITFALLS #8 note):** `latest_release_date` **ignores retention**. It is the newest date among all of the artist's stored `new_release` events, at any age. ListWatchlist and GetWatchlistEntry take **no cutoff**, and `watchlist.Service` does not get the retention setting. With the 90-day default, a retention-scoped value would leave most artists at "No releases yet" and collapse the sort. The accepted cost is that the Watchlist can show a date History no longer lists, and the routes stay unlinked (D-13). - **G2 (supersedes D-15's "future dates as-is"):** a date is **upcoming** when it is strictly after UTC `CURRENT_DATE` at its own precision. `2027` and `2026-11` are upcoming on 2026-10-04, but `2026` and `2026-10` are not. Postgres decides this inside the same LATERAL join, so the client never compares dates. - **G3 (extends D-03/D-14):** the payload carries two fields. `latest_release_date` is the newest non-upcoming date, and it alone drives the latest-release sort. `next_release_date` is the **nearest** upcoming date, shown as a muted `Upcoming: {date}` line, and it never affects the sort. Both come from the single `GET /watchlist` query and the shared POST/PATCH projection (Phase 24 D-26). -- **G4 (supersedes UI-SPEC's card-removal focus flow and announcement):** **sticky cards.** An in-card edit never removes its own card from a filtered view, and the card gets no cue. Stickiness clears on a change to search text, sort, tag selection, a toggle, Clear filters, or a chip-click filter, and on reload. "N of M" counts **visible** cards. +- **G4 (supersedes UI-SPEC's card-removal focus flow and announcement):** **sticky cards.** An in-card edit never removes its own card from a filtered view, and the card gets no cue. Stickiness clears on a change to search text, sort, tag selection, a toggle, Clear filters, or a chip-click filter, and on reload. "N of M" counts **visible** cards. Amended 2026-10-11 (WR-03, user decision 2026-10-11): "tag selection" means the user's own tag-filter change. The route's own tag rewrites (a Manage tags merge or delete of a selected tag, the first-load tag-id check, and URL canonicalization) keep sticky cards and carry them to the filter they land on. Back/Forward still clears them. - **G5 (extends D-04):** the name filter adds a fold table after removing accents: `$→s`, `ø→o`, `æ→ae`, `ß→ss`, applied to both the query and the name. `asap` matches A$AP Rocky. - **G6 (D-12 sequencing):** History's move to URL state stays in scope as **its own plan**, separate from the HIST-02 tag filter, so the tag filter can land and be verified on its own. - **G7 (confirms D-11):** both comboboxes stay, with the UI-SPEC's hand-rolled `Combobox` hardening. A backlog todo covers unifying them on base-ui. diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-REVIEW-FIX.md b/.planning/phases/25-find-filter-watchlist-and-history/25-REVIEW-FIX.md new file mode 100644 index 00000000..e1bb59a3 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-REVIEW-FIX.md @@ -0,0 +1,80 @@ +--- +phase: 25-find-filter-watchlist-and-history +fixed_at: 2026-10-10T18:30:00Z +review_path: .planning/phases/25-find-filter-watchlist-and-history/25-REVIEW.md +iteration: 1 +findings_in_scope: 4 +fixed: 4 +skipped: 0 +status: all_fixed +--- + +# Phase 25: Code Review Fix Report + +**Fixed at:** 2026-10-10 +**Source review:** .planning/phases/25-find-filter-watchlist-and-history/25-REVIEW.md +**Iteration:** 1 + +**Summary:** +- Findings in scope: 4 (Critical 0, Warning 4; Info excluded by fix_scope) +- Fixed: 4 +- Skipped: 0 + +**Verification environment:** `workflow.use_worktrees` is `false`, so all edits, commits and gates ran in the main checkout (no worktree). The Postgres from `docker compose` (drop-tracker-postgres-1) was up and `TEST_DATABASE_URL` pointed at it, so DB-backed tests ran rather than skipping. + +## Fixed Issues + +### WR-01: A stale "Load more" response can append rows from the previous filter to the new feed + +**Files modified:** `web/app/routes/history.tsx`, `web/app/routes/history.test.tsx` +**Commit:** 344d69b +**Applied fix:** `handleLoadMore` now captures a feed key (artist, type, tag, reload token) when the request is issued and drops the response, error, and `appendLoading` reset if the key has changed. The page-one effect also resets `appendLoading`. Added a regression test (a held Load more, then a tag change, then a late stale response). It fails without the fix and passes with it. + +### WR-02: Sticky cards survive filter changes made through the URL (Back/Forward) + +**Files modified:** `web/app/routes/watchlist.tsx`, `web/app/routes/watchlist.test.tsx` +**Commit:** 000b090 +**Applied fix:** Added an effect that clears `stickyIds` when the URL-owned filter key (sort, tags, muted, custom) changes. The `q` resync branch (Back/Forward) now also calls `clearSticky()`. `q` is deliberately kept out of the key: its debounced echo write would otherwise wipe a card touched during the 250 ms window. The explicit handler `clearSticky()` calls are kept, so the change is minimal and idempotent. Added a Back-navigation regression test. It fails without the fix and passes with it. +**Not done:** The review also noted that the count announcement is not scheduled for URL-driven changes. I left this out. An effect-driven announcement would overwrite the "Sorted by ..." message that `handleSortChange` sets, and the handlers already schedule announcements for UI-driven changes. + +### WR-03: Placeholder or malformed release dates are shown as a real "Latest release" + +**Files modified:** `queries/watchlist.sql`, `internal/db/sqlc/watchlist.sql.go`, `internal/db/sqlc/querier.go`, `internal/watchlist/service_test.go` +**Commit:** 4bdc558 +**Applied fix:** All four `latest` and `upcoming` LATERAL blocks (`ListWatchlist` and `GetWatchlistEntry`) now require a well-formed `YYYY`, `YYYY-MM` or `YYYY-MM-DD` date (regex) that is `>= '0001'`. sqlc 1.31.1 regenerated the Go code. Added two `TestService_List_LatestRelease` cases (placeholder/malformed dates ignored, and a malformed date never beats a real one). The first fails without the fix (it surfaced `2020-13` and `TBA`) and passes with it. +**Status note:** This is a query-logic change, so it needs a human check that the regex matches the intended accepted formats. + +### WR-04: The "cancels a pending count announcement" test cannot detect the regression it is named for + +**Files modified:** `web/app/routes/watchlist.test.tsx` +**Commit:** 9cc7050 +**Applied fix:** Both sort-announcement tests now use `vi.useFakeTimers({ shouldAdvanceTime: true })` with `userEvent.setup({ advanceTimers })`, and `vi.advanceTimersByTime(1000)` replaces the 700 ms real sleeps. A mutation check (removing the `clearTimeout(countTimer)` line in `handleSortChange`) makes the "cancels" test fail, so it now detects the regression. +**Caveat:** Plain `useFakeTimers` hangs under vitest because RTL's `asyncWrapper` awaits a real `setTimeout`. `shouldAdvanceTime` is therefore required, and the fake clock still tracks wall-clock time while the interactions run. The test can still pass spuriously if the type and sort interactions take over 500 ms, but the 1.4 s of real sleeping is gone. + +## Additional commit + +- befe939: `build(25): rebuild embedded SPA after review fixes`. `web/app` source changed, so I rebuilt with the same steps as the `make web` target (`pnpm install --frozen-lockfile`, `pnpm run build`, copy `web/build/client` to `internal/webassets/build/client`). `make` is not installed here, so I ran the steps by hand. + +## Verification + +| Command | Result | +|---|---| +| `corepack pnpm --dir web exec prettier --write "**/*.{ts,tsx}"` before each web commit; `prettier --check` at the end | clean | +| `cd web && corepack pnpm test` (vitest, coverage thresholds on) | 30 files, 568 tests passed | +| `cd web && corepack pnpm exec tsc --noEmit` | no errors | +| `go vet ./...` | clean | +| `golangci-lint run` | 0 issues | +| `go test ./... -count=1` against Postgres | all packages ok | +| `go test ./internal/webassets ./cmd/server -count=1` after the SPA rebuild | ok | +| `sqlc generate` (v1.31.1) | regenerated; the generated diff was committed with the SQL | + +Not run: +- `make test` and `make sqlc-check`: `make` is not installed on this machine. `go test ./...` and `sqlc generate` ran directly instead. +- `-race` runs: cgo is not enabled here. +- `make coverage-gate`: it needs a `coverage.out` profile, which I did not generate. + +--- + +_Fixed: 2026-10-10_ +_Fixer: Claude (gsd-code-fixer)_ +_Iteration: 1_ diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-REVIEW.md b/.planning/phases/25-find-filter-watchlist-and-history/25-REVIEW.md new file mode 100644 index 00000000..2e652bb7 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-REVIEW.md @@ -0,0 +1,134 @@ +--- +phase: 25-find-filter-watchlist-and-history +reviewed: 2026-10-12T22:34:33Z +depth: standard +files_reviewed: 8 +files_reviewed_list: + - web/app/components/watchlist/SearchResultsColumns.test.tsx + - web/app/components/watchlist/SearchResultsColumns.tsx + - web/app/components/watchlist/TagChips.test.tsx + - web/app/components/watchlist/TagChips.tsx + - web/app/lib/useScrollAnchor.test.tsx + - web/app/lib/useScrollAnchor.ts + - web/app/routes/watchlist.test.tsx + - web/app/routes/watchlist.tsx +findings: + critical: 0 + warning: 2 + info: 6 + total: 8 +status: issues_found +--- + +# Phase 25: Code Review Report (gap closure 25-19 to 25-21) + +**Reviewed:** 2026-10-12T22:34:33Z +**Depth:** standard +**Files Reviewed:** 8 +**Status:** issues_found + +_This report replaces the 2026-10-12T20:25:38Z report (0 critical, 3 warning, 3 info), which is kept in git history at `fc4b2d4`. It covers `fc4b2d4..HEAD` for the eight files above._ + +## Summary + +This round covers 25-19 (G-25-11, every sibling card as a fallback anchor), 25-20 (G-25-12, focus after a keyboard toggle removes its card) and 25-21 (G-25-13, `aria-describedby` on the disabled Deezer button). The four scoped test files pass locally (`vitest run --coverage.enabled=false`, 147 tests). + +Status of the prior warnings: + +| Prior | Verdict | Notes | +|-------|---------|-------| +| WR-01 (fallback checks only the two immediate neighbours) | **Resolved** | `anchorCandidates` walks outward over all sibling cards, next before previous at each distance (`useScrollAnchor.ts:5-19`). The old "no candidate survives" test now drops every card, and new hook and route tests cover runs of leaving cards. A new problem with *distant* survivors is described in WR-02 below. | +| WR-02 (Deezer hint has no accessible description) | **Resolved, with a regression** | The button now has a description through `aria-describedby` (`SearchResultsColumns.tsx:206, 211-213`), and the test asserts `toHaveAccessibleDescription`. The description element is a visible-to-AT `sr-only` span, so the hint is read twice. See WR-01 below. The touch gap is closed by user decision (no visible note). | +| WR-03 (focus falls to `` when the clicked card leaves) | **Resolved** | The keyboard-only move is detected with `detail === 0`, gated on held focus and on focus having fallen to the document, and reported through `useEffectEvent`. It targets the fallback card's pressed chip, otherwise the `tabIndex={-1}` heading. I traced last-card, middle-card, run, no-survivor, mouse and focus-moved-away paths. They behave as the UI-SPEC row says. | + +Prior IN-01 and IN-02 were outside this round's plans and are still open (IN-05 and IN-06 below). Prior IN-03 carries over as IN-07. + +I traced the capture-to-layout-effect timing for the toggle path. The anchor is captured synchronously in the click. The key change, the sticky reset (`reconcileSticky` during render) and the card unmount all land in one commit, so the layout effect sees the final DOM. `useEffectEvent` impls are committed before layout effects run, so `reportFocusLost` always calls the current `focusAfterCardLeaves`. I found no security issues. The new selectors are constant literals, and the hint span renders a static constant. + +## Warnings + +### WR-01: The Deezer hint is read twice in browse mode, because the description element is an `sr-only` span rather than a hidden one + +**File:** `web/app/components/watchlist/SearchResultsColumns.tsx:198-213`; `web/app/components/watchlist/SearchResultsColumns.test.tsx:234-237, 346-348` +**Issue:** `sr-only` hides the span visually but keeps it in the accessibility tree as ordinary text. In browse mode (NVDA, JAWS, VoiceOver virtual cursor), the reader reaches the button and announces "Search MusicBrainz to add, button, unavailable, This project can't yet match…", taking the hint from `aria-describedby`. It then reaches the sibling span and reads the same 130-character sentence again as plain text. + +The wrapper `title` is a third source of the same text. Some readers expose it on the generic ``. The new tests lock the doubled tree in: `toHaveClass("sr-only")` at line 236, and the row `textContent` assertion at line 347 expects the hint to be part of the row's text. +**Fix:** Keep the referenced element out of the tree but still referenceable. Per accname, `aria-describedby` follows a direct reference into hidden content: +```tsx + +``` +`toHaveAccessibleDescription` still passes, because dom-accessibility-api follows direct references into hidden nodes. Replace `toHaveClass("sr-only")` with `toHaveAttribute("hidden")`. The row `textContent` assertion is unchanged, since `textContent` includes hidden nodes. If you want `getByRole` and `toBeVisible` checks to prove it is out of the tree, add `expect(screen.queryByText(DEEZER_ADD_HINT)).not.toBeVisible()`. + +### WR-02: A distant fallback survivor is pinned at its original off-screen offset, so the viewport jumps and the G-25-12 focus target can land off-screen with `preventScroll` + +**File:** `web/app/lib/useScrollAnchor.ts:53-57`; `web/app/routes/watchlist.tsx:197-203` +**Issue:** The correction keeps the chosen candidate at the viewport-relative `top` it had at capture time. For the chip, or an adjacent card, that is near the click point. 25-19 now lets the candidate be any distance away, so it can be a card that was off-screen at capture. + +Worked example: cards are about 166px apart in an 800px viewport, the clicked card is at y=400, and the four cards on each side leave with it (an un-pressed tag that most cards carry, with the 50+ artists used in UAT). The chosen survivor `C+5` was at y≈1230, below the fold. After `scrollBy(0, -1494)` it is at 1230 again. The viewport now shows `C-7..C-11`, cards that were more than 1000px above the user's position. Neither nearest survivor is on screen. That is the G-25-4 "I lost my place" symptom, now in the other direction. + +G-25-12 then focuses `C+5`'s chip with `{ preventScroll: true }` (`watchlist.tsx:202`). A sighted keyboard user's focus is on an element outside the viewport, and the next Tab or Shift+Tab scrolls the page again. + +The route test at `watchlist.test.tsx` "corrects by the nearest survivor's drift when a run of cards leaves (G-25-11)" uses a survivor top of 900, which is already below an 800px fold, so it pins this behaviour without noticing it. +**Fix:** When the anchor is a fallback (`a.el !== captured.el`), aim for the click position instead of the survivor's own old offset. At minimum, clamp the target into the viewport: +```ts +const rect = a.el.getBoundingClientRect() +const want = + a.el === captured.el + ? a.top + : Math.min(Math.max(captured.candidates[0].top, 0), window.innerHeight - rect.height) +const delta = rect.top - want +if (delta !== 0) window.scrollBy(0, delta) +``` +`candidates[0]` is the chip, so its captured top is where the user was looking. Add a route or hook case where the survivor is more than one viewport away, and assert that it ends near the click point. If the UI-SPEC D-12 wording ("where it was on screen") is meant literally for distant survivors, record that decision instead. In either case, the focus target should not stay off-screen: after focusing, call `target.scrollIntoView({ block: "nearest" })` when its rect is outside `[0, innerHeight]`. + +## Info + +### IN-01: A chipless fallback card sends focus to the heading even when a nearer card with chips survives + +**File:** `web/app/routes/watchlist.tsx:197-203`; `web/app/lib/useScrollAnchor.ts:53, 64` +**Issue:** `onFocusLost` receives only the first connected candidate. If that card has no `[data-chip-label]`, focus goes to the heading, which can be far from the click. This can happen when a sticky card (one that fails `q`, `muted` or `custom`) un-presses the last selected tag. `tagIds` becomes empty, the sticky reset removes the clicked card, and the nearest survivor can be an untagged artist. +**Fix:** Pass the ordered list of connected candidates (or let the route walk them) and take the first one that has a chip label, before falling back to the heading. + +### IN-02: The newly focusable heading keeps `outline-none`, so programmatic focus has no visible indicator + +**File:** `web/app/routes/watchlist.tsx:599-604` +**Issue:** `outline-none` dates from 25-04, when the `

` could not take focus. In the no-survivor path, a sighted keyboard user's focus now lands on it with no ring. A `tabIndex={-1}` heading is not in the tab order, so this is not a strict 2.4.7 failure, but the user cannot see where focus went. +**Fix:** Use `focus-visible:ring-[3px] focus-visible:ring-ring/50` (the same ring the chip labels use), or document in the UI-SPEC that this is an intentional choice. + +### IN-03: The `detail === 0` comment overstates the signal + +**File:** `web/app/components/watchlist/TagChips.tsx:198-200` +**Issue:** "detail is 0 only for an Enter or Space activation" is not accurate. A programmatic `.click()` and many assistive-tech default actions also dispatch `detail: 0`. The 25-20 summary already notes this, and the held-focus and focus-fell-to-document guards in the hook make it safe, but the inline comment tells the next reader something false. +**Fix:** Reword it, for example: "`detail` is 0 for keyboard or synthetic activation. The hook's focus guards make that safe (G-25-12)." + +### IN-04: The hint copy is no longer pinned by any test + +**File:** `web/app/components/watchlist/SearchResultsColumns.test.tsx:7, 200, 234` +**Issue:** The new assertions compare against the imported `DEEZER_ADD_HINT`, so a copy change or an accidentally emptied constant still passes. The only literal check is `toContain("MusicBrainz identity")` on the wrapper title (line 190). +**Fix:** Assert the UI-SPEC literal once, for example `expect(DEEZER_ADD_HINT).toBe("This project can't yet match …")`, or use a literal in `toHaveAccessibleDescription`. + +### IN-05: Dead read-order stub in the previous-card route test (carried over from prior IN-01) + +**File:** `web/app/routes/watchlist.test.tsx:2335-2349` +**Issue:** It is unchanged. The label is measured once and is disconnected after the commit, so the `120` branch never runs, and the comment ("key the rect on read order") misdescribes why the test passes. +**Fix:** Drop the stub, or replace it with `expect(label.isConnected).toBe(false)`. + +### IN-06: `clearFilters` hand-derives "will the filter key change" (carried over from prior IN-02) + +**File:** `web/app/routes/watchlist.tsx:531` +**Issue:** It is unchanged. The predicate duplicates `filterKeyOf` minus `sort`/`q`, and there is still no sticky-then-Clear-filters test. +**Fix:** Use `if (filterKeyOf({ ...view, tagIds: [], muted: false, custom: false }) === filterKey) clearSticky()`, or call `clearSticky()` unconditionally. + +### IN-07: Real-browser checks are still outstanding (carried over from prior IN-03) + +**File:** `web/app/lib/useScrollAnchor.test.tsx`; `web/app/routes/watchlist.test.tsx:2392-2600`; `web/app/components/watchlist/SearchResultsColumns.test.tsx:167-239` +**Issue:** Every anchor and focus test stubs `getBoundingClientRect` and `scrollBy`, and jsdom has no screen reader. 25-21 D3 is `human_judgment: true`. The open items are native `overflow-anchor` interplay and page-end clamping, WR-02's distant-survivor case, focus-ring visibility after the move, and how often NVDA/VoiceOver announce the Deezer hint (WR-01). +**Fix:** In `/gsd-verify-work` on the rebuilt binary, run these three checks. First, un-press a tag that removes a long run of cards in a list of 50+ artists. Second, press Enter on the last card's chip and confirm focus is visible and on screen. Third, read a Deezer row in browse mode. + +--- + +_Reviewed: 2026-10-12T22:34:33Z_ +_Reviewer: Claude (gsd-code-reviewer)_ +_Depth: standard_ diff --git a/.planning/phases/25-find-filter-watchlist-and-history/25-SECURITY.md b/.planning/phases/25-find-filter-watchlist-and-history/25-SECURITY.md new file mode 100644 index 00000000..3e8bf650 --- /dev/null +++ b/.planning/phases/25-find-filter-watchlist-and-history/25-SECURITY.md @@ -0,0 +1,133 @@ +--- +phase: "25" +slug: "find-filter-watchlist-and-history" +status: verified +# threats_open = count of OPEN threats at or above workflow.security_block_on severity (the blocking gate) +threats_open: 0 +asvs_level: 1 +created: "2026-10-12" +--- + +# Phase 25 — Security + +> Per-phase security contract: threat register, accepted risks, and audit trail. + +--- + +## Trust Boundaries + +| Boundary | Description | Data Crossing | +|----------|-------------|---------------| +| Postgres events → GET /watchlist JSON | Detected release dates (from MusicBrainz/Deezer) in new response fields, behind the existing auth gate | Catalogue dates, low | +| Browser → GET /events?tag_id | Untrusted query param reaches a SQL predicate | Integer id, low | +| GET /watchlist, /tags, /events, /search → DOM | Release dates, user-authored tag names, `created_at`, third-party catalogue strings rendered in cards, chips, options | Display text, low | +| URL query string → Watchlist/History view state | `q`, `sort`, `tags`, `muted`, `custom`, `artist`, `type`, `tag` are attacker-craftable via a shared link | Untrusted strings, low | +| Browser → history API | Keystrokes can become navigations | UI state, low | +| Card chip click → filter vs server state | Adjacent filter-toggle and detach buttons | Mutation intent, medium | +| Package registry → `pnpm install --frozen-lockfile` | Build-time dependency resolution | Supply chain, high | +| Built bundle → go:embed → served SPA | The committed tree is what the binary serves | Shipped code, medium | + +--- + +## Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation | Status | +|-----------|----------|-----------|----------|-------------|------------|--------| +| T-25-01 | DoS | ListWatchlist laterals | medium | mitigate | Two `LIMIT 1` laterals in one statement (`queries/watchlist.sql:39-93`) | closed | +| T-25-02 | DoS | NULL scan | medium | mitigate | Plain-column laterals; null-not-500 HTTP test (`internal/httpserver` watchlist test ~L2023) | closed | +| T-25-03 | Info Disclosure | latest/next release fields | low | accept | See Accepted Risks AR-25-01 | closed | +| T-25-04 | Tampering | SQL construction | low | accept | See Accepted Risks AR-25-02 | closed | +| T-25-05 | Tampering | `tag_id` → ListEvents/HasOlderEvents | high | mitigate | `parseOptionalPositiveInt64` + `invalid tag_id` (`internal/httpserver/events.go:87-89`); bound `sqlc.narg('tag_id')::bigint` (`queries/events.sql:151,188`) | closed | +| T-25-06 | Info Disclosure | Retention bypass via tag filter | medium | mitigate | Predicate inside retention statements; `TestRetention_TagFilterComposesWithCutoff` (`internal/httpserver/events_test.go:1519`) | closed | +| T-25-07 | Info Disclosure | Error responses | low | mitigate | Fixed messages (`invalid tag_id`, `internal error`) | closed | +| T-25-08 | DoS | EXISTS over artist_tags | low | accept | See Accepted Risks AR-25-03 | closed | +| T-25-09 | Tampering (XSS) | WatchlistRow release lines | medium | mitigate | Plain JSX text; literal-`` route test (`web/app/routes/watchlist.test.tsx`); no `dangerouslySetInnerHTML`/`innerHTML` in `web/app` | closed | +| T-25-10 | Tampering | Date display | low | mitigate | `formatReleaseDate` renders stored precision, no Date/timezone math | closed | +| T-25-11 | Tampering (XSS) | `q` param | medium | mitigate | Controlled input value + JSX text only | closed | +| T-25-12 | DoS | History flooding | low | mitigate | `Q_WRITE_DEBOUNCE_MS = 300` with replace (`web/app/routes/watchlist.tsx:59`) | closed | +| T-25-13 | DoS | Per-keystroke API traffic | low | mitigate | In-memory filter; test asserts `listTags`/`listEvents` not called (`watchlist.test.tsx:1067-1068`) | closed | +| T-25-14 | Repudiation | Invisible add under filter | low | mitigate | Sticky set keyed by POST response id; add + Undo route tests | closed | +| T-25-15 | Tampering | `sort` param | low | mitigate | `isSortKey` allow-list → `DEFAULT_SORT` (`web/app/lib/watchlistView.ts:44,81`) | closed | +| T-25-16 | DoS | Client sort cost | low | accept | See Accepted Risks AR-25-04 | closed | +| T-25-17 | Tampering (XSS) | Tag names in History combobox | medium | mitigate | Plain JSX text / React-escaped `title` | closed | +| T-25-18 | Tampering | `tagId` sent to API | low | mitigate | Ids only from GET /tags; server re-validates (T-25-05) | closed | +| T-25-19 | DoS | Failed GET /tags | low | mitigate | `.catch` → `status: "error"`, no retry (`web/app/routes/history.tsx:173-185`) | closed | +| T-25-20 | DoS | Oversized `tags=` | low | mitigate | `MAX_TAG_TOKENS = 200`, `^[1-9]\d*$` (`watchlistView.ts:63-70`) | closed | +| T-25-21 | Tampering (XSS) | Tag names in toolbar chips | medium | mitigate | JSX text / escaped attributes; no HTML-injection API in `web/app` | closed | +| T-25-22 | Tampering | Filter ↔ server state | low | mitigate | Filter never calls attach/detach/delete | closed | +| T-25-23 | Tampering | Chip label vs × | medium | mitigate | Label calls only `toggleFilterTag` (`TagChips.tsx:200`); tests assert attach/detach not called (`TagChips.test.tsx:554-560`) | closed | +| T-25-24 | Tampering | `muted`/`custom` params | low | mitigate | Only `"1"` honored (`watchlistView.ts:83-84`) | closed | +| T-25-25 | Tampering (XSS) | Chip `aria-label`/`title` | low | mitigate | React-escaped attributes | closed | +| T-25-26 | Tampering | History `artist`/`tag` params | low | mitigate | `ID_PATTERN = /^[1-9]\d*$/` (`web/app/lib/historyParams.ts:10`); server re-validates | closed | +| T-25-27 | Tampering | History `type` param | low | mitigate | Three-value allow-list (`historyParams.ts:5-7`); server `watchlist.EventTypes` (`internal/watchlist/service.go:49`) | closed | +| T-25-28 | Tampering (XSS) | Param values in DOM | low | mitigate | Params parsed to ids/tokens, never rendered | closed | +| T-25-SC | Tampering | npm installs during rebuilds | high | mitigate | No Phase 25 commit touches `web/package.json` or `web/pnpm-lock.yaml`; empty diff vs `461267f`, `8ccde67`, `9d3413f`. Lockfile diff vs `9030bfa` is Phase 24's `7eaa989` (security overrides, patch bumps), merged into the branch base before `627b159` | closed | +| T-25-29 | Tampering | Stale embedded bundle | medium | mitigate | Dedicated rebuild task with marker checks (25-10) | closed | +| T-25-30 | Elevation | Out-of-scope packages | low | mitigate | Empty diff over notifier/detection/discord/musicbrainz/deezer/poller/migrations since `9030bfa` | closed | +| T-25-31 | DoS (UI) | useScrollAnchor effect | low | mitigate | Anchor cleared before measuring; early returns | closed | +| T-25-32 | Tampering | Router scroll contract | low | mitigate | `preventScrollReset: true` on every write (`web/app/lib/useUrlParams.ts:5`); file only touched by 25-04 | closed | +| T-25-33 | Tampering (XSS) | EventCard Detected line | low | mitigate | `formatDetectedDate` → `