pal-e-docs

Review: Players list view: SSR table at /players

review-1092-2026-04-25 Doc

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Scope is well-written and traceable. One [LABEL] fix and several [SCOPE] dependency-sequencing flags before this can move backlog -> todo. No structural rewrite needed.

Template Completeness (template-issue-feature)

  • [x] Type: Feature
  • [x] Lineage
  • [x] Repo: forgejo_admin/westside-admin
  • [x] User Story (references story-westside-admin-admin-row-crud)
  • [x] Context (clear motivation, locked decision called out)
  • [x] File Targets (Create + Modify + Do NOT create)
  • [x] Acceptance Criteria (9 testable items)
  • [x] Test Expectations (integration + unit + manual)
  • [x] Constraints (scopedDb, no client fetch, no Tailwind, allowlist inline)
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:admin-row-crud label - Story note story-westside-admin-admin-row-crud verified, entry exists on project-westside-admin user-stories section
  • [x] arch:page-server label - generic component label, not a backing-note expectation. Backing arch artifact is arch-dataflow-westside-admin Flow 1 (referenced in Related). No arch-page-server note exists and none is expected (page-server is a SvelteKit primitive, not a domain-level component).
  • [x] Forgejo issue forgejo_admin/westside-admin#4 - open, well-formed
  • [x] Sensitive-column handling traces to arch-domain-westside-admin Key Decisions (contract_token explicitly listed there)

File Targets

All listed files are new (the repo currently contains only README.md). Path conventions are correct for SvelteKit adapter-node. Paths cannot be pre-verified against existing code because scaffolding ticket #6 (arch:sveltekit-server) has not landed.
  • [x] src/routes/players/+page.server.ts - standard SvelteKit load + actions location
  • [x] src/routes/players/+page.svelte - standard
  • [x] src/lib/components/EnumBadge.svelte, DateCell.svelte, JsonbCell.svelte - standard $lib pattern, server-safe (no $lib/server import needed)
  • [x] src/routes/+layout.svelte - exists conceptually after scaffolding ticket #6
  • [!] All paths are valid conditional on #6 (sveltekit-server scaffolding) landing first. This is correctly captured in Lineage.

Repo Placement

OK. Single repo: forgejo_admin/westside-admin. No cross-repo work.

Dependencies

Lineage names them ("scaffolding + Drizzle + auth + tenant helper"). Mapped to board items:
  • #1087 (issue #6) arch:sveltekit-server - scaffolding (BLOCKER)
  • #1089 (issue #1) arch:scopeddb-helper - Drizzle setup (BLOCKER)
  • #1091 (issue #3) arch:scopeddb-helper - tenant helper (BLOCKER)
  • #1090 (issue #2) arch:hooks-server - auth (BLOCKER, #1093 also depends)
  • #1096 (pal-e-platform #301) arch:keycloak - Keycloak realm/client (BLOCKER for live auth)
  • #1097 (pal-e-platform #302) arch:postgres-grants - cross-ns DB access (BLOCKER for live data)
None of these are in in_progress; all are siblings in backlog. The dependencies are correctly stated in Lineage but the issue body does not list specific Forgejo issue numbers. [BODY] recommendation: enumerate blocker issue numbers under Lineage so the dev agent can verify upstream merges before starting.

Acceptance Criteria

9 criteria, all testable. Each maps to either an integration test (rows, pagination, filter, auth gates) or a render assertion (badges, dates, jsonb collapse, sensitive-column exclusion). Test Expectations section names real vitest-shaped commands. Manual visual check is appropriate for v1. Allowlist enforcement at +page.server.ts level is correct (server-side, before serialization).

Blast Radius

Limited - first feature in a fresh repo. No sibling services to mirror. One forward-looking concern: the inline column allowlist is intentional per Constraints ("refactor when 2nd table arrives"). Good - prevents premature abstraction. No downstream consumers of this route yet.

Decomposition Assessment (5-minute rule)

  • 5 new files + 1 modify = 6 file targets, single repo
  • 9 AC - at the threshold but tightly coupled (single route + 3 cell components)
  • Estimated agent time: 15-25 min for a competent agent (3 small Svelte components + 1 server load + 1 page render + nav link)
This is at the upper edge of the 5-minute rule but is a single coherent slice. Decomposition would fragment a tight seam (server load -> page render -> cell components are mutually dependent). Recommendation: keep as one ticket, accept ~20 min agent run. No decomposition needed.

Recommendation

  • [BODY] Add explicit blocker issue numbers in Lineage section: "Blocked by forgejo_admin/westside-admin#6 (scaffolding), #1, #3 (scopedDb), #2 (hooks/auth), and pal-e-platform#301 (Keycloak), #302 (postgres-grants)." This lets the dev agent verify upstream PRs are merged before starting.
  • [SCOPE] Confirm sequencing: this ticket cannot move past next_up until all 6 blockers are merged. Ava should hold this in todo after refinement and not promote to next_up until the dependency chain clears.
  • [BODY] Optional: name the test runner explicitly (likely npm test / vitest) once scaffolding ticket #6 establishes it.
After the [BODY] addition, this ticket is structurally READY. The verdict is NEEDS_REFINEMENT only on dependency-traceability hygiene, not on scope quality.