Skip to content

feat(review): pattern column on the close-out acceptance-criteria rollup - #3904

Merged
kyle-sexton merged 2 commits into
mainfrom
feat/3824-pattern-column
Sep 7, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
feat/3824-pattern-column

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Closes #3824

What changed

Close-out mode's acceptance-criteria rollup already anchors the pass on the container's criteria,
already gives every criterion a verdict of delivered / partial / missing / unverifiable,
and already renders unconditionally. What it lacked was any awareness of a criterion's requirement
pattern, so the shape of a tagged set (every unwanted-behaviour criterion unverifiable while
the event-driven ones were all delivered) was discarded.

The rollup now gains one conditional column naming each criterion's requirement pattern when
any criterion retrieved in Step 2 opens with a bracketed EARS tag.

  • The cell carries one of exactly five names, ubiquitous, event-driven, state-driven,
    unwanted-behaviour, optional-feature, the vocabulary the writing side (planning/interview+prd: unwanted-behaviour coverage prompt always on, EARS format opt-in via convention doc #3821 / feat(planning): always-on unwanted-behaviour coverage prompt, EARS tags under the convention #3882) emits.
    A bracket holding anything else leaves the cell empty rather than echoing raw text.
  • Detection is a leading bracket holding one of those names, nothing more. No flag, no lever, no
    convention key, and no resolution ladder. A checklist marker (- [ ]) is named explicitly as the
    bracket that is not a tag, because the repo's own Agent Brief template emits criteria that way.
  • Every criterion still gets a row. A partially tagged set shows its untagged rows with an
    empty pattern cell rather than dropping them, which is the invisibility the rollup exists to
    remove. A set carrying no tag renders exactly as it does today, with no extra column.
  • Step 2's extraction bullet now says to keep the criterion line whole, so the tag survives the
    read.

What deliberately did not change

  • No second verdict vocabulary. The existing four values stay; no met, no not-met.
  • The blocking rule is untouched. Only a missing or wrong finding keeps the container open.
    That rule is co-owned by the work-items container lifecycle and changing it is a container-level
    decision, not a side effect of adding a column. plugins/work-items/ is not in this diff.

Verification

A fresh-context verifier that did not write the change confirmed all nine checked points with no
CRITICALs: no second verdict vocabulary, blocking rule unchanged, plugins/work-items/ untouched,
one row per criterion on a partially tagged set, the five names verbatim (including British
-behaviour), unchanged rendering with no tags, no flag/lever/convention key, scope held to one
column, and house mechanics (em-dash purge on the README, changelog order, version parity). Its one
non-blocking tightening (the checklist-marker ambiguity) is applied as the second commit.

Gates run locally: check-changelog-parity.sh --check-bump main (pass),
check-purged-em-dashes.sh (pass), validate-plugin-contracts.mjs (pass),
markdownlint-cli2 on the changed markdown (0 issues).

Review plugin bumped 0.26.19 to 0.26.20.

Note: #3821 / #3882 (the writing side that emits the tags) is still open, so this reader lands
before its writer. The vocabulary is pinned to that PR's stated contract.

🤖 Generated with Claude Code

https://claude.ai/code/session_012jiwedVq2GxuzN7siXQbr4

kyle-sexton and others added 2 commits September 6, 2026 23:48
Close-out mode's acceptance-criteria rollup already gives every criterion a
verdict of delivered, partial, missing or unverifiable, and already renders
unconditionally. What it lacked was any awareness of a criterion's requirement
pattern, so a tagged set's shape (every unwanted-behaviour criterion
unverifiable while the event-driven ones all delivered) was discarded.

The rollup now gains one column naming each criterion's pattern when any
criterion retrieved in Step 2 opens with a bracketed EARS tag. The cell carries
one of exactly five names, ubiquitous, event-driven, state-driven,
unwanted-behaviour, optional-feature, matching what the planning surfaces emit;
a bracket holding anything else leaves the cell empty. Detection is that
bracket's presence, so no flag, lever, or convention key is read and no
resolution ladder is restated. Every criterion still gets a row, so a partially
tagged set shows untagged rows with an empty pattern cell, and an untagged set
renders exactly as before with no extra column.

Step 2's extraction bullet now says to keep the criterion line whole so the tag
survives the read. The verdict vocabulary and the blocking rule (missing or
wrong keeps the container open) are unchanged.

Closes #3824

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012jiwedVq2GxuzN7siXQbr4
The detection sentence said "that bracket's presence", which read in isolation
makes a bare `[` the trigger. The repo's own Agent Brief template emits
acceptance criteria as `- [ ] <criterion>`, so a checkbox-shaped set could be
read as tagged and render an empty pattern column on every row.

Detection is now stated as a leading bracket holding one of the five names, and
the checklist marker is named as the case it excludes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012jiwedVq2GxuzN7siXQbr4
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@kyle-sexton
kyle-sexton enabled auto-merge (squash) September 7, 2026 03:52
@claude

claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 20s —— View job


/review:code-review — code-review lane

  • Gather context (PR diff, files changed, custom instructions)
  • Run /review:code-review per custom instructions
  • Post findings as inline PR comments (none — no blocking findings)
  • Post summary comment

Skip-gate check: PR is open, non-trivial, no prior applicable review found → proceeded with full review.

Scope: All 4 changed files (plugins/review/skills/quality-gate/context/close-out.md, plugins/review/README.md, plugins/review/CHANGELOG.md, plugins/review/.claude-plugin/plugin.json). No claude-security-review findings in scope for this lane.

Findings

None that meet the high-signal bar (a careful senior reviewer would block or flag). No inline comments posted.

What I checked

  • The commit history already closes the one real ambiguity I was tracking down. Commit 1's detection rule was "that bracket's presence" for triggering the column, which would have made a bare checklist marker (- [ ]) — the format the repo's own Agent Brief template emits — misfire and add an empty column. Commit 2 (b1eb52e) tightens this to "a leading bracket holding one of those five names," which correctly excludes - [ ] from triggering the column at all, not just from populating the cell. Verified by diffing the two commits directly.
  • Internal consistency: the five pattern names (ubiquitous, event-driven, state-driven, unwanted-behaviour, optional-feature) are listed identically and in the same order in close-out.md, CHANGELOG.md, and the PR body.
  • Scope claims verified against the diff: no second verdict vocabulary was introduced (still delivered/partial/missing/unverifiable), the blocking rule at close-out.md#L394-396 is untouched, and plugins/work-items/ does not appear in the diff.
  • Step 2 → Step 6 dependency holds together: Step 2's extraction bullet (close-out.md line 96-98) now keeps the criterion line whole instead of stripping it to prose, which is exactly what Step 6's bracket-detection logic needs to see the tag. No other file in this skill (spec.md) depends on criteria text being pre-stripped of a leading bracket, so this doesn't regress anything downstream.
  • "Every criterion still gets a row" is stated unconditionally for the partially-tagged case, matching the PR's stated intent to avoid re-introducing the invisibility problem the rollup exists to fix.
  • Version/changelog hygiene: plugin.json 0.26.19 → 0.26.20 matches the new top CHANGELOG.md entry; versioning convention here is a flat patch bump per release regardless of Added/Changed, consistent with the plugin's prior history.
  • Author-claimed verification (changelog-parity script, em-dash purge, markdown lint, plugin-contract validation) is noted as author-claimed and unverified — I did not independently run those gates.

Strengths

  • The PR is unusually precise about what it deliberately did not change (no second verdict vocabulary, blocking rule untouched, work-items/ untouched), which made scope verification straightforward.
  • The self-correction between the two commits (tightening the checklist-marker ambiguity) shows the adversarial-verification step described in the PR body actually caught a real edge case rather than rubber-stamping.

@claude

claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 3s —— View job


Security review (/review:security-review)

  • Gather context (PR diff vs origin/main, changed files)
  • Invoke /review:security-review and follow its instructions
  • Post findings

Skip gate triggered: this change has no security-relevant surface.

All four changed files are documentation/config, not executable logic:

  • plugins/review/skills/quality-gate/context/close-out.md — adds instructional prose telling the close-out reviewer to keep an acceptance-criterion line whole and to render an optional "requirement pattern" column, restricted to a closed vocabulary of exactly five literal names (ubiquitous, event-driven, state-driven, unwanted-behaviour, optional-feature). Anything else in the bracket, including untrusted/attacker-influenced text from an issue body, is explicitly discarded rather than echoed into the rendered table — this is itself a sound mitigation against a criterion body smuggling arbitrary text into the rollup output, and the PR doesn't weaken it.
  • plugins/review/README.md — one sentence describing the new column.
  • plugins/review/CHANGELOG.md — changelog entry.
  • plugins/review/.claude-plugin/plugin.json — version bump only.

No code execution paths, no injection surfaces (command/SQL/path/template), no auth/access-control logic, no secret or token handling, and no GitHub Actions workflow changes are touched by this diff. Nothing here falls in this lane's scope.

No security findings.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Last security-reviewed head: b1eb52ef2ecd2aba1ab820de1b46c19e99d242ab. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

This PR body does not yet satisfy the issue-linkage contract:

  • Missing a "## Summary" section. Describe what this PR changes and why, in a sentence or two.
  • Missing a "## Fix" section. State the concrete change and how it addresses the problem.
  • Missing a "## Related" section. List related PRs, ADRs, or decision-log entries this PR does not close.

Edit the body and this comment updates itself on the next run.

@kyle-sexton
kyle-sexton merged commit d8f7d0e into main Sep 7, 2026
18 checks passed
@kyle-sexton
kyle-sexton deleted the feat/3824-pattern-column branch September 7, 2026 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

review/quality-gate close-out: pattern column on the acceptance-criteria rollup

1 participant