Review: Playground overhaul 4/4: Cleanup + component docs

review-384-2026-03-25 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Lineage — present (references westside-app#84, parent note westside-playground-overhaul)
  • [x] Repo — present (forgejo_admin/westside-playground)
  • [x] User Story — present (story:WS-S26)
  • [x] Context — present (final cleanup after sub-tickets 1-3)
  • [x] File Targets — present (3 files to remove, 24 pages to verify)
  • [x] Acceptance Criteria — present (6 items)
  • [x] Test Expectations — present (3 items)
  • [x] Constraints — present (dependency on sub-tickets 1-3)
  • [x] Checklist — present (5 items)
  • [x] Related — present
  • [x] Type — present (Feature)
All required template sections are present.

File Targets

  • [x] tryouts.html — verified: exists at ~/westside-playground/tryouts.html, confirmed stale (references cancelled tryouts)
  • [x] westside-index.html — verified: exists, confirmed stale (self-referencing nav links, old copy)
  • [x] original-index.html — verified: exists, confirmed stale (references old west-side-basketball Stripe link)
  • [ ] "All 24 pages" — ISSUE: page count is wrong. Currently 18 HTML files exist. Sub-ticket #86 defines 8 public pages, #87 defines 14 authenticated pages = 22 total. Not 24. After sub-tickets complete and stale files are removed, the count will depend on whether signin.html and success.html survive or get replaced.

Repo Placement

MISMATCH. The issue body says Repo: forgejo_admin/westside-playground but the Forgejo issue is filed on forgejo_admin/westside-app (#88). All 4 overhaul sub-tickets (#85-#88) share this mismatch — they are filed on westside-app but the work targets westside-playground. This is not a blocker (the Repo field in the body is correct for agents), but it creates confusion in Forgejo's issue tracker.

Dependencies

Board item labels: depends:ws-86+ws-87. All three dependencies are currently open:
  • #85 (CSS/JS unification) — open, board column: todo
  • #86 (Public pages) — open, board column: todo, depends on #85
  • #87 (Authenticated pages) — open, board column: todo, depends on #85
Dependencies are correctly documented. The ticket explicitly states it depends on sub-tickets 1-3. The board labels only list depends:ws-86+ws-87 but should also include ws-85 since the Constraints section says "Depends on sub-tickets 1, 2, and 3 all being complete." The transitive dependency through #86/#87 covers it, but explicit is better.

Acceptance Criteria

  • [x] "Stale files removed (3 files)" — testable: ls and verify absence
  • [x] "Every page has complete component doc comment" — testable: grep @route
  • [x] "All nav links work across all 24 pages" — testable: grep for href, cross-reference file list
  • [ ] "grep for broken hrefs returns zero" — ISSUE: underspecified. What counts as "broken"? Need a concrete command (e.g., extract all href values from all HTML files, verify each target exists as a file or valid anchor)
  • [ ] "ls *.html matches exactly the 24 expected pages" — ISSUE: the 24-page list is not provided. The ticket should enumerate the exact expected filenames so the agent can verify the list, not guess.
  • [x] "Lucas final phone walkthrough" — non-automatable, correctly flagged as manual gate

Blast Radius

  • index.html links to tryouts.html: Two links in index.html (lines 48, 57) point to tryouts.html. When tryouts.html is removed, these become broken links. Sub-ticket #86 should handle this (updating index.html), but this ticket's verification step must confirm those links were removed or redirected.
  • shared/app.css has tryouts styles: Line 1969+ of shared/app.css contains a "Page: tryouts.html" CSS section. This becomes dead CSS after removal. Sub-ticket #85 (CSS unification) should handle cleanup, but this ticket should verify.
  • signin.html orphaning: index.html links to signin.html. Sub-ticket #86 creates login.html as replacement. signin.html is not listed as a stale file to remove in this ticket, but it should be — or its fate should be documented.
  • success.html orphaning: success.html (registration success page) is not listed in either sub-ticket #86 or #87. It has no component doc comments. Its fate is undocumented — should it be removed, kept, or updated?
  • README.md references stale files: The playground README references tryouts.html and signin.html in its route table. Needs updating.
  • westside-app impact: Minimal — only one comment reference to "playground" found in westside-app SvelteKit code. No functional coupling.

Recommendation

Four issues to resolve before this ticket is READY:
  • Fix page count: Change "24 pages" to the correct number. Enumerate the exact expected filenames in the acceptance criteria so the agent has a verifiable checklist. Based on sub-tickets #86 (8 pages) + #87 (14 pages) = 22, but signin.html and success.html need disposition.
  • Account for signin.html and success.html: These two files are not in any sub-ticket's scope. Either add them to the stale removal list (if login.html replaces signin.html), or add them to sub-ticket #86/#87 page lists. success.html (registration confirmation) likely belongs in the public pages list.
  • Specify the broken-href grep command: The acceptance criterion "grep for broken hrefs returns zero" needs a concrete test command or script the agent can run.
  • Consider filing on correct repo: All 4 overhaul issues are filed on westside-app but target westside-playground. Not a blocker (the body's Repo field is correct), but worth noting for Forgejo hygiene.