pal-e-docs

Review: #22 testing-strategy + local-dev-setup (decomp:1800)

review-1805-2026-07-05 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Board item 1805 (board-iss), Forgejo issue ldraney/intelligentstaffingsystems#22, 2 points, decomposition child of #19 (item 1800, tracking-only per review-1800-2026-07-05). Scope quality is excellent — every file target, spec reference, and factual claim verified against origin/main (670c515), and the ticket correctly owns plan open decision 1 (test framework wording). The single gap is the missing arch-docs backing note behind the arch:docs label — a board-wide gap shared by all decomp:1800 siblings, fixable with one note.

Template Completeness (Feature template)

  • [x] Type — Feature (routes to template-issue-feature)
  • [x] Lineage — child of #19, cites review-1800-2026-07-05 and the spec doc
  • [x] Repo — ldraney/intelligentstaffingsystems
  • [x] User Story — developer-facing, docs-before-first-PR intent stated
  • [x] Context — names both docs, the landscaping-assistant read-only reference, the net-new Keycloak stubbing section, and open decision 1 ownership
  • [x] File Targets — 2 create targets plus explicit do-not-touch (sibling ownership)
  • [x] Feature Flag — "none"; correct (docs-only; repo has no docs/feature-flags.md — that is a Tier 2 item)
  • [x] Acceptance Criteria — 5 checkboxes
  • [x] Test Expectations — manual render + link check, run command N/A (appropriate for docs-only)
  • [x] Constraints — framework decision must match #3's scaffold; no README contradiction
  • [x] Checklist
  • [x] Related — project slug + parent
All 12 sections present. No template gaps.

Traceability

  • [x] story:project-setup label — present on board item 1805
  • [x] story note verified — project-iss user-stories section (created 2026-07-05 21:52, after the parent review flagged it) lists key project-setup with success metric "Repo, docs, infra, and CI exist; sprints can dispatch" — docs work fits squarely. Parent review's [SCOPE] on the missing project page is resolved.
  • [x] arch:docs label — present on board item 1805 (and now on parent 1800 and all siblings — consistent family labeling)
  • [ ] arch note MISSING — search_notes("arch-docs") returns empty; list_notes(project="iss") shows only project-iss, review-1800, and board-iss (no arch note under any name). [SCOPE] Create architecture note arch-docs for component docs. Same gap was flagged for westside-basketball (reviews 1673/1674/1676) and claude-custom (review-1526) — the note was never created in any project.
  • [x] Forgejo issue — https://forgejo.tail5b443a.ts.net/ldraney/intelligentstaffingsystems/issues/22, open, label sprint:0 (board item carries the full label set — normal; board labels are authoritative)

File Targets

Verified against origin/main (commit 670c515).
  • [x] docs/testing-strategy.md (create) — does not exist on main; content spec in issue matches docs/adoption-plan.md line 29 nearly verbatim (endpoint-first, no browser/system tests with the Hotwire "server IS the frontend" rationale, "What gates a PR" table, net-new per-role Keycloak session stubbing section, owns the lint/test claims from architecture.md's CI diagram)
  • [x] docs/local-dev-setup.md (create) — does not exist on main; plan line 13 spec (prereqs/first-time/daily-commands, trigger "now") matches
  • [x] docs/adoption-plan.md (spec reference) — exists on main; open decision 1 verified at line 39: "tickets say rails test (Minitest default); landscaping docs say 'specs' (RSpec). testing-strategy.md decides; CI diagram stays framework-neutral until then" — exactly what the issue Context claims this ticket owns
  • [x] ~/landscaping-assistant/docs/testing-strategy.md (read-only reference) — exists (7.0k); headings confirm Philosophy / Why No System Tests / The Hotwire Testing Insight / What gates a PR / Test Layers — everything the adaptation needs
  • [x] ~/landscaping-assistant/docs/local-dev-setup.md (read-only reference) — exists (2.9k); contains all five sections AC 4 names (Prerequisites / First-Time Setup / Running the App / Key Files / Common Commands)
  • [x] Supporting claims — port 9999 (README.md:105), ~/rails-base copy prereq (README.md:86,93), "the 3 roles" (docs/user-stories.md:5 "Three roles govern access, enforced in Keycloak and Rails"), RuboCop + Brakeman gate claims (docs/architecture.md:317-318 CI diagram) — all verified on main
  • [x] Do-not-touch — "all other docs" correctly assigns security.md to #20, pipeline.md to #21, user-stories/architecture amendments to #23, README/plan-status to closer #24

Repo Placement

OK. Single-repo docs work; issue filed on the repo it modifies. No cross-repo concerns.

Dependencies

Blocked by: nothing. Spec doc is committed on main; nothing on the board is in_progress; siblings are order-independent.
Blocks: #24 (item 1807, the closer) carries blocker:siblings and cannot run until this lands — documented via labels. Downstream, this ticket's framework decision feeds #3 (bootstrap, sprint:1): the plan's "before first PR" trigger makes testing-strategy.md a gate for the first code PR, and the issue's Constraints section documents the match-what-#3-scaffolds rule. Adequately documented.
Soft cross-link with #21: AC 5 requires both docs to link pipeline.md in their Related sections, but pipeline.md is created by sibling #21 and does not exist yet. If #22 merges first, that link is a forward reference (dead until #21 lands). Not a blocker — siblings are expected to land in parallel and closer #24 (blocker:siblings) is positioned to catch any unresolved link — but the "internal links resolve" manual check should tolerate the forward reference at PR time.

Acceptance Criteria

5 criteria, all objectively verifiable by an agent: AC 1 (system-test exclusion + rationale) and AC 2 (per-role stubbing, 3 roles) are grep-able doc content; AC 3 (framework stated + RuboCop/Brakeman gate claims reconciled) is checkable against architecture.md:317-318; AC 4 names the exact five sections (all present in the landscaping reference) plus the port-9999 consistency check; AC 5 is a link check with the forward-reference caveat above. Test Expectations honestly say manual / N/A — no fake test commands. No missing criteria.

Blast Radius

Docs-only; no code or config. Two propagation effects: (1) this doc settles plan open decision 1 — the framework wording binds #3's CI scaffold and every future test-running ticket, which is exactly why the constraint pins it to Rails-default Minitest unless reasoned otherwise; (2) the "what gates a PR" table overlaps sibling #21's pipeline.md territory — the plan assigns gate-claim ownership to testing-strategy.md and AC 3's "reconciles" wording handles it. Not touching README avoids the CLAUDE.md-symlink propagation the parent review warned about.

Decomposition Assessment

No decomposition needed. 2 file targets, 1 repo (rule: >3 across >2 — not tripped); 5 acceptance criteria (rule: >5 — not tripped, at threshold); estimated agent work under 5 minutes (adapt one 7k and one 2.9k reference doc against explicit per-doc specs). This IS a decomposition child — the split did its job.

Recommendation

  • [SCOPE] Create architecture note arch-docs for component docs — the arch:docs label has no backing note in pal-e-docs. Board-wide gap: all five decomp:1800 siblings (#20-#24) carry arch:docs, so one note clears the traceability leg for the whole family. Suggested content: in-repo documentation standards, docs/ tree structure, cross-reference conventions, the adoption-plan tier/trigger model.
No [BODY] or [LABEL] fixes needed — the issue body and board labels are accurate as written. On re-review after the arch-docs note exists, this ticket is READY as-is.