feat(test-plan): add a form-semantics axis, qualify dedup, define the PR set

[erickgonzalez]

Proposed Changes

Three gaps found by reviewing a real run of the skill. Two further findings from the same review are
deliberately not here — see Not in this PR.

1 · A tenth coverage axis: Form & state semantics. The existing nine axes are platform concerns
(permissions, hosts, languages, version state, cache, workflow, push publish, persistence, UI/UX),
and the UI/UX checklist is purely presentational — rendering, keyboard, focus, screen reader, empty
states, responsive, i18n, theme, copy. It asks whether a control looks right; nothing asked whether
the data survives. The new axis prompts for every option of a bounded choice, numeric boundaries,
derived fields, child-entity CRUD inside a parent form, optional-set-vs-unset, which gesture persists
what, leaving with unsaved work, and locked states. On a form-heavy change the matrix previously
never asked.

2 · A mocked unit spec no longer retires a manual case. §5 dropped any axis a PR-added test
covered, with no caveat. PR #37064 added 162 mockReturnValue calls — four mocking
$hasUnsavedChanges — and zero e2e specs, so the unsaved-changes guard was retired from the manual
plan on the strength of a spec that builds a fake component. A mock proves component logic, not a
persistence round-trip, a real route change, or a status transition.

3 · A run resolves a PR set as well as an issue set. The skill was single-PR by construction:
§5 diffed one <pr> and §2 resolved issues but never PRs. Spec-driven work lands as several PRs
against the same issues, and the skill can legitimately be handed issues alone. §2 now defines the
input contract for all three shapes; coverage runs over the union of the set's diffs; the Summary
names the PRs covered and any deliberately left out; unmerged PRs are excluded and said to be.

Checklist

• Tests — n/a (skill instructions); every claim below verified against the cited PR/issue
• Translations — n/a
• Security Implications Contemplated — no change to tool permissions or input handling

Additional Info

Not in this PR, from the same review:

• Behavior in the diff that no AC names should force a coverage re-walk. Valid gap, but the
  proposed trigger was "when the diff contradicts the description". It doesn't fit the case that
  prompted it: Experiments Portlet — Screen 2: Create/Update (/experiments/new + /:id/configuration) #37003 specified autosave with no Save button, and feat(experiments): create/update experiment screen (/experiments/new + /:id/configuration) #37064 shipped autosave
  (AUTOSAVE_DEBOUNCE_MS = 500, 158 PATCH lines, replaceUrl) plus Save Draft and a route guard.
  Nothing was contradicted — it was additive — so a contradiction-keyed rule would never fire. Needs
  reframing around unmapped shipped behavior before it's worth encoding.
• Reinstating a coverage index. The per-issue index was removed deliberately; the AC-3: /
  US1-AS3: citation convention already carries traceability in case names. Open question, not a
  defect.

Unrelated: this also regenerates CATALOG.md, which was already stale on main by one count
line after speckit-docs-converge landed. Not part of this change, but skill-lint gates every PR
touching .claude/skills/**.

SKILL.md is now 445 lines — still under the ~500 convention, but close enough that the next
addition should move into references/.

This PR fixes: #37225

[claude]

Claude finished @erickgonzalez's task in 1m 26s —— View job

---

Review

Documentation-only change to the dot-test-plan skill (three markdown files). I reviewed the committed diff (origin/main...HEAD) and verified the substantive factual claims against the codebase.

Note on environment: the working tree in my checkout carries an uncommitted revert of this PR (16 insertions / 98 deletions — the exact inverse), so Read/grep of the files show the old text. I reviewed the committed state via git diff, which is correct. Flagging only so nobody is confused by a stray diff — it is not part of the PR.

Claims I verified

• ✅ darkModeSelector pin — core-web/libs/ui/src/lib/theme/providers.ts:31 reads darkModeSelector: options?.darkModeSelector ?? false. Accurate.
• ✅ No prefers-color-scheme / data-theme usage — zero matches across core-web. Accurate.
• ✅ "The single .dark selector in the repo belongs to the standalone block-editor app config" — exactly one match, core-web/apps/dotcms-block-editor/src/app/app.config.ts. Accurate.
• ✅ Axis count is internally consistent at ten across all three files (README intro + reference link, coverage-matrix.md header, and the ten table rows: Permissions, Sites/hosts, Languages, Content version state, Cache, Workflow, Push Publish, Persistence, UI/UX, + Form & state semantics).
• ✅ PR-set contract is consistent between SKILL.md §2 (the table + the "not once per PR, not once per issue" walk rule), SKILL.md §5/§6, coverage-matrix.md, and the README "Multi-issue consolidation" section. The mocked-spec caveat now appears in both SKILL.md §5 and the coverage-matrix.md dedup footer, and they no longer contradict (the drift called out in your own comment is resolved).

New Issues

No issues found.

The change is coherent, the dedup/mocked-spec caveat is now stated identically in both places, and the "does not exist in dotCMS" carve-out is backed by real code references rather than assertion. The self-caught third drift instance (coverage-matrix.md:102) is a good catch — that was the one that mattered, since it was a substantive contradiction on this PR's own change rather than stale prose.

One forward-looking note you already raised in the description stands on its own merit: at 445 lines SKILL.md is close enough to the ~500 convention that the next addition should move into references/. Not a blocker for this PR.

· issue-37225-form-semantics-and-pr-set

[erickgonzalez]

Both valid — fixed in 2488f01, along with a third instance of the same drift that the review didn't catch.

🟡 "Nine product-surface axes" — fixed

Right, and right about why it matters rather than being cosmetic. This is an instruction file a model
reads to decide which axes to walk; a header undercounting its own table invites treating the tenth
as an afterthought, which defeats the point of adding it. Now Ten coverage axes., aligned with the
README's wording.

🟡 "the union of everything the PR changed" — fixed

Now the union of everything the PR set changed — not once per PR, and not once per issue. The
explicit "not once per PR" is there because that is the wrong reading the new multi-PR contract makes
newly available.

A third one, same drift, one line further down

coverage-matrix.md:102 also said "a test the PR itself added" — singular, and it restated the
dedup rule without the mocked-spec caveat this PR adds to SKILL.md §5. So the two files
disagreed on when an axis may be dropped, which is worse than the two you flagged: not stale prose
but a substantive contradiction on this PR's own change. Now:

An axis or mandatory case already covered by a test one of the PRs itself added gets no manual
case … This holds only when that test exercises the same path a person would: a spec that mocks the
store or builds a fake component does not retire the case (SKILL.md §5).

Root cause

All three are the same mistake: I edited this file to add a row and a checklist and never re-read the
prose around them. The three substantive changes were verified against the cited PRs; the file's own
narration was not. Worth noting for anyone reviewing a future skill PR — a reference file's
introduction is easy to leave describing the state before the edit.

Verified consistent across all four files now: ten axis rows, ten in the matrix header, ten in the
README, and both the matrix and SKILL.md §6 walking the PR set. skill-lint clean, fences balanced.