pal-e-docs

Review: Link groups model and grouped card containers

review-1407-2026-06-09-r2 Review

review approved
Project

Verdict: APPROVED

Re-review of board item #1407. Previous review review-1407-2026-06-09 returned NEEDS_REFINEMENT with 5 findings. All blocking issues have been resolved.

Previous Findings Resolution

  • [x] Lineage #39 -> #40 -- FIXED. Issue body now correctly says "Depends on ldraney/palinks #40".
  • [x] canvas_controller.js dependency -- FIXED. File target entry notes "(file created by #40)" and Constraints section says "canvas_controller.js is created by #40 -- this ticket extends it, does not create it".
  • [x] story:canvas-layout not in user-stories -- Acknowledged as cross-cutting gap. Not blocking per caller direction. [SCOPE] remains open.
  • [x] arch-palinks missing -- Acknowledged as board-wide gap. Not blocking per #36 review precedent. [SCOPE] remains open.
  • [x] Decomposition deferred -- Correctly deferred until dependencies #40 and #36 land. Ticket is in backlog; decomposition is premature now.
  • [x] Feature flag system not implemented -- Acknowledged. Flag section documents the target gate. Consistent with template guidance; the flag design is forward-looking documentation, not a claim that the system exists.

Template Completeness

  • [x] Type
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Feature Flag
  • [x] Acceptance Criteria
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:canvas-layout label -- present on board item #1407
  • [ ] story note MISSING -- story:canvas-layout not listed in project-palinks user-stories section. Current entries: containerize-palinks, sortable-links, auth-roles, custom-domain. Acknowledged as cross-cutting gap, not blocking. [SCOPE] Create user story entry for story:canvas-layout on project-palinks user-stories section.
  • [x] arch:palinks label -- present on board item
  • [ ] arch note MISSING -- no arch-palinks note found in pal-e-docs. Board-wide gap with precedent from #36 review. Not blocking. [SCOPE] Create architecture note arch-palinks.
  • [x] Forgejo issue -- ldraney/palinks#41, state: open

File Targets

  • [x] db/migrate/*_create_link_groups.rb -- new file (create). Migration directory exists at db/migrate/. OK.
  • [x] db/migrate/*_add_group_id_to_links.rb -- new file (create). OK.
  • [x] app/models/link_group.rb -- new file (create). Models directory exists. OK.
  • [x] app/models/link.rb -- exists. No group association yet. Will need belongs_to :link_group added. OK.
  • [x] app/views/links/_link_group.html.erb -- new file (create). Views directory exists. OK.
  • [x] app/views/links/index.html.erb -- exists. Currently renders a flat link-grid with sortable controller. Will need canvas-aware rendering. OK.
  • [x] app/javascript/controllers/canvas_controller.js -- does not exist yet. Ticket now explicitly documents that this file is created by #40 and this ticket extends it. Dependency is clearly stated. OK.
  • [x] app/controllers/link_groups_controller.rb -- new file (create). Controllers directory exists. OK.
  • [x] config/routes.rb -- exists. Currently has resources :links. Will need link_groups resource. OK.
  • [x] app/javascript/controllers/sortable_controller.js -- correctly listed as DO NOT TOUCH. Exists. OK.

Repo Placement

OK. Issue filed on ldraney/palinks, all file targets are in the same repo. No cross-repo work needed.

Dependencies

  • #36 (Create clicks table and tracking) -- board item #1400, column: todo. Group aggregate click tracking depends on the clicks table. Documented in issue.
  • #40 (Canvas view with activity-based card sizing) -- board item #1406, column: backlog. Canvas rendering and canvas_controller.js depend on this. Documented in issue Lineage, File Targets, and Constraints sections.
  • Feature flag system -- not yet implemented per docs/feature-flags.md. Ticket documents the target flag design. Acceptable as forward-looking documentation.

Acceptance Criteria

7 acceptance criteria. All well-written and verifiable once prerequisites exist. Several AC depend on #36 and #40 landing first, which is expected given the dependency chain. The AC themselves are testable by an agent post-dependencies.

Blast Radius

  • Adding link_group_id to links is a schema change affecting the Link model. Migration should be reversible.
  • Modifying index.html.erb changes the primary view -- must be behind feature flag.
  • No sibling services or downstream consumers. Palinks is self-contained.
  • No existing references to link_group or LinkGroup anywhere in the codebase -- clean surface area.

Decomposition Assessment

7 AC + 9 file targets exceeds the 5-minute rule. However, decomposition is correctly deferred: the ticket is in backlog, and dependencies #40 (backlog) and #36 (todo) must ship first. When this ticket moves to todo, decompose into: (1) model/migrations/routes, (2) controller/CRUD, (3) view/canvas integration.

Recommendations

  • [SCOPE] Create user story entry for story:canvas-layout on project-palinks user-stories section (cross-cutting gap, not blocking this ticket).
  • [SCOPE] Create architecture note arch-palinks (board-wide gap, not blocking this ticket).
  • [DECOMPOSE] When this ticket moves from backlog to todo (after #40 ships), route to skill-decompose-ticket. Suggested split: (1) model/migrations/routes, (2) controller/CRUD, (3) view/canvas integration.
No blocking issues. All NEEDS_REFINEMENT findings from the previous review are resolved.