From bbd64546b17546b37f97306c1af94ea64ae378fc Mon Sep 17 00:00:00 2001 From: Frank Ray <52075808+FrankRay78@users.noreply.github.com> Date: Sat, 12 Sep 2026 14:49:24 +0200 Subject: [PATCH 1/4] Refs #281: make the review triageable and narrow the category sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four changes to the pre-specification review, all in its own prompt: - An at-a-glance table above the first group gives every gap's number, title and consequence, so the review can be triaged before it is read. Its shape is forced by /speckit.confirmissue, which folds every **N. ** block ending in `> _Answer:_` into the issue body — a bold-numbered index would have been folded as phantom decisions. - Every gap names one of two kinds — changes what gets built, or settles a detail — and says in one clause what a different answer would change. - Gaps are ordered by consequence within each group, then numbered. Requirements still precedes Technical, since that grouping is what /speckit.confirmissue routes folded decisions by. - A 120-word bound per gap replaces "keep each gap tight", which bounded sub-bullets and offered splitting as the remedy — raising the gap count while leaving the word count alone. The category lists become things to consider rather than a sweep to complete: a category with nothing open produces no gap. Yield is unchanged — no gap is dropped, demoted, or capped on merit, and every gap keeps its recommendation and answer slot in the form the downstream parser already reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/commands/speckit.reviewissue.md | 78 +++++++++++++++++++++---- 1 file changed, 68 insertions(+), 10 deletions(-) diff --git a/.claude/commands/speckit.reviewissue.md b/.claude/commands/speckit.reviewissue.md index 2df4bb2..53fb1c7 100644 --- a/.claude/commands/speckit.reviewissue.md +++ b/.claude/commands/speckit.reviewissue.md @@ -84,6 +84,7 @@ Each gap (in either group) must: - be answerable with a short written response (not "go figure it out") - cite concrete evidence from the issue or codebase where relevant +- state its **consequence** — which of the two kinds below it is, and in one clause what a different answer would change - end with a **Recommendation:** line — your best judgement call with a one-sentence *Reason*. This is mandatory, not optional. The author should be able to read the recommendation and either accept it (record a short @@ -91,9 +92,28 @@ Each gap (in either group) must: gap without a recommendation forces the author to originate the answer from scratch, which is exactly the work this command is meant to front-load. -**Number gaps contiguously across both groups** (1, 2, 3, … not 1a, 1b). This keeps `/speckit.confirmissue` parsing simple and lets the author refer to questions by a single number in chat. +**Raise a gap only where the issue leaves something open.** A gap exists because the issue does not determine the answer — someone taking this into SDD would have to invent it or guess. Test every candidate against the issue as it stands: if the body, its acceptance criteria, or its out-of-scope list already settles the point, there is no gap, however squarely a category below invites one. A category you considered and found settled produces **nothing** — no gap, no placeholder, and no commentary bullet announcing that it is fine. -**Requirements gaps** — probe these before any technical question. Cover at least these categories when applicable: +That test is the only thing deciding whether a point becomes a gap. Do **not** ask whether the author would contest your recommendation — that is a judgement about a person rather than about the issue, and a wrong call settles a real decision silently. Every open point stays a numbered gap with its own answer slot, however confident the recommendation is. + +**The review is sized by what the issue leaves open, not by the number of categories below.** The category lists are a checklist of what to *consider*, never a quota to fill. An issue that commits to little should attract a visibly shorter review than one that leaves much open — so if a tightly-scoped issue is producing a long review, the sweep is manufacturing gaps, and the fix is to drop the ones nothing is actually open in. + +**Consequence — every gap says what a different answer would change.** Each gap is exactly one of two kinds: + +- **Changes what gets built** — answering against the recommendation would change something the issue commits to building: a scenario, an acceptance criterion, the scope boundary, a user-visible behaviour. +- **Settles a detail** — what gets built is fixed either way, and the answer settles a detail within it: a name, a path, a value, where something is documented. + +Name the kind on the gap and say in one clause what would change. Where the call is genuinely borderline, take the lower of the two — an inflated marker costs the reader exactly the attention the marker exists to save. + +**Order the gaps, then number them.** Settle the order first; numbers are assigned to the ordered list, never the reverse: + +1. **By group** — all Requirements gaps, then all Technical gaps. This order is load-bearing: `/speckit.confirmissue` routes each folded decision by the group its gap sat under, so consequence ordering operates *within* a group and never moves a gap across the two. +2. **By consequence within the group** — every *Changes what gets built* gap comes before every *Settles a detail* gap in the same group. +3. **Number contiguously across both groups** (1, 2, 3, … not 1a, 1b), following that order. Contiguous numbering keeps `/speckit.confirmissue` parsing simple and lets the author refer to a gap by a single number in chat. + +Numbers are assigned once, when the comment is first composed. A refine run never reorders and never renumbers — see step 6. + +**Requirements gaps** — probe these before any technical question. Consider each category below and raise a gap only where the issue leaves it open: - **User & persona** — who uses this, in what context. The issue may name a feature without naming the person. - **Job-to-be-done** — the user-visible outcome that means "done"; contradictions between the stated outcome and the proposed mechanism. @@ -103,7 +123,7 @@ Each gap (in either group) must: - **Acceptance criteria from outside** — whether existing ACs are observable from outside the implementation by a user or external test; flag project-housekeeping items (project exists, sln updated, test scaffolding) — they belong in `/speckit.tasks`. - **User-visible failure modes** — what the user sees when a dependency is unreachable, slow, or rejects them; fail-open vs fail-closed *from the user's viewpoint*. -**Technical gaps** — surface gaps suggested by Step 2 (codebase grounding) plus any tech shape the issue itself already commits to. Let the author set the depth: they may want extensive tech review or none. +**Technical gaps** — surface gaps suggested by Step 2 (codebase grounding) plus any tech shape the issue itself already commits to. Let the author set the depth: they may want extensive tech review or none. Same test as above — consider each category, raise a gap only where something is genuinely unsettled. - **Where it lives** — existing endpoint/service/flow extended vs. new; who calls whom; which org(s) are involved. - **Integration** — interface contracts with existing components, event/data flow, ordering. @@ -129,15 +149,16 @@ Write what survives as bullets, not questions: ### 4. Draft the comment -Structure the comment body as follows. Each gap gets an inline answer slot -(`> _Answer:_`) so the issue author can respond beneath it in a single edit. +Structure the comment body as follows. A table lists every gap up front so the +author can triage the review before reading into it, and each gap then gets an +inline answer slot (`> _Answer:_`) so they can respond beneath it in a single edit. ```markdown ## Pre-specification review — gaps & clarifications <!-- speckit:review --> -Before taking this into SDD, the following points need answers. Please record responses inline. +Before taking this into SDD, the following points need answers. The table lists every gap and what turns on it — use it to decide where to spend your attention, then record responses inline beneath each gap. > If an answer slot says `not sure`, `idk`, `tbd`, `more options`, `help me`, or similar hedge (anything that means "I want help, not a decision"), re-run `/speckit.reviewissue #N` and that question will be re-framed with extra options, a worked example, and a revised recommendation. Iterate as many times as you need. > @@ -145,9 +166,18 @@ Before taking this into SDD, the following points need answers. Please record re > > When all answers are concrete, run `/speckit.confirmissue #N` to fold them into the issue body as **Confirmed decisions**. That deletes this comment — the decisions are the record from then on, and you revise one by editing its bullet. +### Gaps at a glance + +| # | Gap | If answered against the recommendation | +|---|---|---| +| 1 | <short title> | Changes what gets built — <what would change, ≤12 words> | +| 2 | <short title> | Settles a detail — <what would change, ≤12 words> | +| 3 | <short title> | Settles a detail — <what would change, ≤12 words> | + ### Requirements gaps **1. <short title>** +_Changes what gets built:_ <what a different answer would change, one clause> <concrete framing of the gap, including any evidence from issue/codebase> - <sub-question 1> - <sub-question 2> @@ -157,6 +187,7 @@ Before taking this into SDD, the following points need answers. Please record re > _Answer:_ **2. <short title>** +_Settles a detail:_ <what a different answer would change, one clause> ... > _**Recommendation:**_ ... Reason: ... @@ -168,6 +199,7 @@ Before taking this into SDD, the following points need answers. Please record re > Omit this section entirely if no technical gaps were identified — do not emit an empty heading. Gap numbering continues from the Requirements section (3, 4, …), not restarting at 1. **N. <short title>** +_<consequence kind>:_ <what a different answer would change, one clause> ... > _**Recommendation:**_ ... Reason: ... @@ -182,8 +214,32 @@ Before taking this into SDD, the following points need answers. Please record re - ... ``` -Keep each gap tight. If a gap has more than ~3 sub-bullets, consider whether -it is actually two gaps. +**The at-a-glance table.** One row per gap, in the same order as the gaps +themselves, spanning both groups — it is the whole set in one view, which is +what makes the review triageable without reading it. Titles in the `Gap` column +match each gap's own title verbatim, so a row and its gap are unmistakably the +same thing. Always emit the table, even for a single gap: the author should +never have to check whether it is there. The one exception is a review that +raises no gaps at all — omit the table along with the groups, since there is +nothing to triage. + +Two rules on its shape, and the first is load-bearing: + +- **Never use the `**N. <title>**` form in the table, and never put a `> _Answer:_` line above the first group.** `/speckit.confirmissue` parses a gap as a `**N. <title>**` block ending in `> _Answer:_`, and folds every one it finds into the issue body. A row that imitates that shape would be folded as a phantom decision the author never made. Table cells carry a bare number and plain text, which matches nothing the parser looks for. +- **The table replaces nothing.** Every gap keeps its own consequence line, framing, recommendation and answer slot below; the table is a view of them, not a substitute. + +**Length bound — 120 words per gap.** Count everything from the `**N. <title>**` +line through to its `> _Answer:_` slot: the consequence line, the framing, every +sub-bullet, and the recommendation with its reason. The bound applies to the gap +as a whole rather than to any one part of it, and no gap may exceed it. + +**Splitting is not how you meet the bound.** A 200-word gap broken into two +100-word gaps satisfies nothing — the reader faces the same prose and one more +decision. Cut instead: drop the restatement of what the issue already says, keep +the evidence that makes the gap specific, and let the recommendation carry the +detail rather than the framing. Split only where the gap is genuinely two +independent questions needing two separate answers — and then each half must +meet the bound on its own. **Recommendation quality bar:** the recommendation must be a concrete, actionable default (a value, a library, a field name, an HTTP status, an @@ -251,7 +307,9 @@ Re-framing rules (hedging case only): - **Expand the question body** with 2–4 concrete options laid out as a sub-list, each with a one-line trade-off. Add a worked example or a pointer to a comparable existing pattern in the codebase (read the codebase again if needed — surface defaults they may not have known existed: existing constants, sibling service patterns, port allocations, etc.). - **Revise the `> _**Recommendation:**_` line** if the new framing changes your call. Keep the `Reason:` cite tied to evidence. - **After ~2 hedging iterations on the same question** with no commitment, add a final option *"This may be out of scope for the current issue — answer `out of scope: <reason>` to drop it"* and call it out in the recommendation. Do not edit the gap out yourself — leave that to the author + `/speckit.confirmissue`. -- **Do not touch any other gap.** Substantive, out-of-scope, and empty answers must come through byte-for-byte. The Commentary section is also untouched. +- **Keep the at-a-glance table in step with the gap you re-framed.** If the re-framing changes that gap's title or its consequence, update its row — the title cell, the consequence cell, or both — and nothing else in the table. Never add, remove, reorder or renumber rows: the table mirrors the posted gap order, which was fixed when the comment was first composed. +- **Do not retrofit the new shape onto an older comment.** A comment posted before the at-a-glance table and the consequence line existed has no row to update and no kind to restate — leave it that way. A refine run re-frames the hedging gap and nothing else; it never adds a table to a comment that has none. +- **Do not touch any other gap.** Substantive, out-of-scope, and empty answers must come through byte-for-byte, and so must every table row but the one you changed. The Commentary section is also untouched. If no gap qualifies for re-framing, **make no edit** and report that in chat (the author either still has un-answered questions, or is ready for `/speckit.confirmissue`). @@ -286,7 +344,7 @@ Keep your own chat response short. Tailor it to the run mode: **First run:** - confirm the issue reviewed (number + title) -- state how many gaps were raised, and whether the review carries commentary +- state how many gaps were raised, split by consequence (e.g. *5 gaps — 2 change what gets built, 3 settle a detail*), and whether the review carries commentary - return the comment URL **Refine run:** From c5f4eedeed54f243c4a484d72f4f438f1ef717a9 Mon Sep 17 00:00:00 2001 From: Frank Ray <52075808+FrankRay78@users.noreply.github.com> Date: Sat, 12 Sep 2026 14:49:31 +0200 Subject: [PATCH 2/4] Refs #281: record the triageable-review decisions as a CIR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six decisions a future maintainer would reasonably question, and the constraint none of them make obvious: the up-front view is a table because /speckit.confirmissue folds anything shaped like a numbered gap into the issue body, so the obvious bold-numbered index would have posted phantom decisions the author never made. Also records why consequence is two values rather than a severity scale, why the sweep is narrowed at source rather than capped, why splitting is not how a gap meets the length bound, and where 120 came from — the four gaps of the review on #278 ran 184 to 317 words. Four residuals, including the third consumer of this comment (/speckit.draftissue), enumerated here because reviews of this same section missed it on both preceding issues. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .../2026-09-12-triageable-review-comment.md | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 docs/change-intent-records/2026-09-12-triageable-review-comment.md diff --git a/docs/change-intent-records/2026-09-12-triageable-review-comment.md b/docs/change-intent-records/2026-09-12-triageable-review-comment.md new file mode 100644 index 0000000..5ecfd54 --- /dev/null +++ b/docs/change-intent-records/2026-09-12-triageable-review-comment.md @@ -0,0 +1,40 @@ +# A Triageable Pre-Specification Review + +**Intent:** Let the author of an issue spend attention on a `/speckit.reviewissue` comment in proportion to what is at stake, and stop the command's fourteen-category sweep inflating the review beyond what the issue actually leaves open — without dropping, demoting or capping a single gap on merit. + +**Behaviour:** + +- Given a review raising several gaps, When the author opens the comment, Then an at-a-glance table gives every gap's number, title and consequence before any gap body is read. +- Given gaps that differ in consequence, When the comment is composed, Then within each group the gaps that change what the issue commits to building are numbered ahead of those that settle a detail, and each gap names its own kind. +- Given an issue that leaves little open, When the review runs, Then it raises gaps only where something is genuinely unsettled, and is visibly shorter than the review of an issue that leaves much open. +- Given a gap of any consequence, When it is composed, Then it still carries a recommendation with a one-sentence reason and an inline answer slot, in the form `/speckit.confirmissue` already parses. + +**Constraints:** + +- `/speckit.confirmissue`'s prompt is out of scope, so its parser is fixed: it folds every `**N. <title>**` block ending in a `> _Answer:_` line into the issue body. Anything added above the gaps must match neither shape. +- The Requirements-then-Technical group order is load-bearing for that command's routing — it files each folded decision by the group its gap sat under. Consequence ordering therefore operates strictly *within* a group. +- The author refers to gaps by number in chat, and folding strips the numbers, so ordering must be settled before numbering and a posted review is never renumbered. +- Prompt-only change. Per Constitution Principle I's configuration-and-tooling carve-out there is no xUnit test and no bespoke checker; the evidence is the command's own output before and after, recorded in the PR body. + +**Decisions:** + +1. **Consequence is two values, not a severity scale.** *Changes what gets built* versus *Settles a detail*, decided by a question about the artefact — would a different answer change something the issue commits to building? Rejected: a three-tier priority, which offers a middle bucket everything drifts into and still does not say *what* would change. Borderline calls take the lower kind, because an inflated marker costs the reader precisely the attention the marker exists to save. + +2. **The up-front view is a table, and a downstream parser chose its shape.** The obvious index mirrors the gaps — a bold-numbered list — and that is exactly the shape `/speckit.confirmissue` folds, so every row would have landed in the issue body as a phantom decision the author never made. A table row carries a bare number and plain text and matches nothing the parser looks for. Also rejected: a collapsed `<details>` block, which leaves the reader an action to perform before they can see the set, which is the behaviour being removed. + +3. **The table is always emitted, even for a single gap.** Rejected: a threshold below which it is omitted. A threshold makes the reader establish whether the table is missing or merely absent before trusting it, and gives the refine run a mode in which a row may or may not exist. + +4. **The sweep is narrowed at source by an open-ness test, not by a cap.** A category considered and found settled produces nothing. Rejected: a numeric cap on gaps, which forces out a genuine gap for arriving last; and the superseded #278 proposal of demoting the gaps the reviewer predicts the author would not contest, which asks the model to judge a person rather than the issue and settles a real decision silently when it judges wrong. + +5. **The length bound is per-gap words, and splitting is explicitly not the remedy.** The rule it replaces bounded sub-bullets and offered "consider whether it is actually two gaps" as the fix — which raises the gap count while leaving the word count where it was. 120 words was chosen against real output: the four gaps of the review on #278 ran 184, 192, 196 and 317 words, so the bound bites on every one of them while still leaving room for framing, cited evidence, two or three sub-questions and a reasoned recommendation. + +6. **A refine run does not retrofit the new shape.** A comment posted before the table and the consequence line existed keeps neither. Re-framing a hedging gap updates that gap's row if it has one, and nothing else. + +**Residual risks:** + +- **The consequence call is an unverified model judgement.** #281's own open questions name the fallback if it proves badly calibrated: order by consequence without publishing the marker, which keeps the benefit that survives a wrong call and drops the one that does not. +- **The 120-word bound and the open-ness test are enforced by instruction, not by a gate** — against this repo's precedent of preferring a gate (CIR [`2026-09-07-automated-prespec-review`](2026-09-07-automated-prespec-review.md), decision 5). `/speckit.reviewissue` composes prose, and there is no exit code that decides whether a gap was manufactured. +- **`/speckit.draftissue` is a third consumer of this comment.** It ingests the whole body as brief material in migration mode and reads it as prose rather than parsing its shape, so the table breaks nothing — but each gap title now reaches it twice. Enumerated here deliberately: the same consumer was missed by reviews of this section on the two preceding issues (`docs/study/279.md`, `docs/study/280.md`). +- **Nothing verifies that a recommendation is a good default, or that its *Reason* survives being checked against the file it cites.** A shorter review makes each recommendation carry more weight, so this change raises the cost of that gap without addressing it. #281 records it as separate and more consequential than length. + +**Date:** 2026-09-12 From 211726c5f24e08db4c20698a755532197d29e1c2 Mon Sep 17 00:00:00 2001 From: Frank Ray <52075808+FrankRay78@users.noreply.github.com> Date: Sat, 12 Sep 2026 14:49:55 +0200 Subject: [PATCH 3/4] Refs #281: soft-wrap the prose added to the review prompt One line per paragraph, per CLAUDE.md. The four paragraphs added around the comment template were hard-wrapped to match the older text beside them, which reflows a whole block on a one-word edit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/commands/speckit.reviewissue.md | 28 +++++-------------------- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/.claude/commands/speckit.reviewissue.md b/.claude/commands/speckit.reviewissue.md index 53fb1c7..c755edd 100644 --- a/.claude/commands/speckit.reviewissue.md +++ b/.claude/commands/speckit.reviewissue.md @@ -149,9 +149,7 @@ Write what survives as bullets, not questions: ### 4. Draft the comment -Structure the comment body as follows. A table lists every gap up front so the -author can triage the review before reading into it, and each gap then gets an -inline answer slot (`> _Answer:_`) so they can respond beneath it in a single edit. +Structure the comment body as follows. A table lists every gap up front so the author can triage the review before reading into it, and each gap then gets an inline answer slot (`> _Answer:_`) so they can respond beneath it in a single edit. ```markdown ## Pre-specification review — gaps & clarifications @@ -214,32 +212,16 @@ _<consequence kind>:_ <what a different answer would change, one clause> - ... ``` -**The at-a-glance table.** One row per gap, in the same order as the gaps -themselves, spanning both groups — it is the whole set in one view, which is -what makes the review triageable without reading it. Titles in the `Gap` column -match each gap's own title verbatim, so a row and its gap are unmistakably the -same thing. Always emit the table, even for a single gap: the author should -never have to check whether it is there. The one exception is a review that -raises no gaps at all — omit the table along with the groups, since there is -nothing to triage. +**The at-a-glance table.** One row per gap, in the same order as the gaps themselves, spanning both groups — it is the whole set in one view, which is what makes the review triageable without reading it. Titles in the `Gap` column match each gap's own title verbatim, so a row and its gap are unmistakably the same thing. Always emit the table, even for a single gap: the author should never have to check whether it is there. The one exception is a review that raises no gaps at all — omit the table along with the groups, since there is nothing to triage. Two rules on its shape, and the first is load-bearing: - **Never use the `**N. <title>**` form in the table, and never put a `> _Answer:_` line above the first group.** `/speckit.confirmissue` parses a gap as a `**N. <title>**` block ending in `> _Answer:_`, and folds every one it finds into the issue body. A row that imitates that shape would be folded as a phantom decision the author never made. Table cells carry a bare number and plain text, which matches nothing the parser looks for. - **The table replaces nothing.** Every gap keeps its own consequence line, framing, recommendation and answer slot below; the table is a view of them, not a substitute. -**Length bound — 120 words per gap.** Count everything from the `**N. <title>**` -line through to its `> _Answer:_` slot: the consequence line, the framing, every -sub-bullet, and the recommendation with its reason. The bound applies to the gap -as a whole rather than to any one part of it, and no gap may exceed it. - -**Splitting is not how you meet the bound.** A 200-word gap broken into two -100-word gaps satisfies nothing — the reader faces the same prose and one more -decision. Cut instead: drop the restatement of what the issue already says, keep -the evidence that makes the gap specific, and let the recommendation carry the -detail rather than the framing. Split only where the gap is genuinely two -independent questions needing two separate answers — and then each half must -meet the bound on its own. +**Length bound — 120 words per gap.** Count everything from the `**N. <title>**` line through to its `> _Answer:_` slot: the consequence line, the framing, every sub-bullet, and the recommendation with its reason. The bound applies to the gap as a whole rather than to any one part of it, and no gap may exceed it. + +**Splitting is not how you meet the bound.** A 200-word gap broken into two 100-word gaps satisfies nothing — the reader faces the same prose and one more decision. Cut instead: drop the restatement of what the issue already says, keep the evidence that makes the gap specific, and let the recommendation carry the detail rather than the framing. Split only where the gap is genuinely two independent questions needing two separate answers — and then each half must meet the bound on its own. **Recommendation quality bar:** the recommendation must be a concrete, actionable default (a value, a library, a field name, an HTTP status, an From 3bfda8cf079906d0aeb71915b2e7fa5bace68b62 Mon Sep 17 00:00:00 2001 From: Frank Ray <52075808+FrankRay78@users.noreply.github.com> Date: Sat, 12 Sep 2026 15:13:21 +0200 Subject: [PATCH 4/4] fix: apply /verify review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four corrections where the new text asserted something untrue or contradicted a rule elsewhere in the file: - The 120-word bound was stated as an unscoped absolute, contradicting step 6's mandate that a re-framed gap gain 2-4 options with trade-offs and a worked example. Scoped to first composition. - A refine run was told to update the table row but never the gap's own consequence line, so the up-front view could drift from the gap it describes — the one thing AC10 exists to prevent. - "This order is load-bearing" was false. /speckit.confirmissue routes by the heading a gap sat under, not by the sequence of the two sections; swap them and its output is byte-identical. Reattributed to grouping. - The phantom-decision mechanism was wrong. That command hard-stops in step 2 when a parsed gap looks unanswered, so a table row in gap shape either blocks the fold or takes the first real gap's answer slot. Also: narrowed an absolute that excluded step 1's existing-comments rule; dropped the zero-gap table exception, which had no template support (the empty-group path belongs to #278's successor); dropped an unsupported claim that contiguous numbering helps the parser, which strips numbers; named "Settles a detail" rather than "the lower of the two", since the design denies there is a scale; softened "the sweep is manufacturing gaps" to a likelihood, per #281's own open question; and fixed the example table to show the consequence column restarting at the group boundary. Added a pre-post self-check step. Composing to a bound is not meeting one: the first draft written under this prompt overshot on 3 of 3 gaps unaided, which no instruction caught. CIR: the 184/192/196/317 figure was wrong — the counting script ran past the answer slot to end-of-file, adding 94 words of trailing heading and commentary. Two independent counts put the fourth gap at 223. Corrected, with the counting method stated. Also split the gating residual, which conflated the two judgement-only criteria with nine that are mechanically decidable, and recorded why a hook cannot reach them: a first run pipes the body straight to gh, leaving no committed artefact to check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/commands/speckit.reviewissue.md | 29 +++++++++---------- .../2026-09-12-triageable-review-comment.md | 17 ++++++----- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/.claude/commands/speckit.reviewissue.md b/.claude/commands/speckit.reviewissue.md index c755edd..c79eef0 100644 --- a/.claude/commands/speckit.reviewissue.md +++ b/.claude/commands/speckit.reviewissue.md @@ -92,24 +92,24 @@ Each gap (in either group) must: gap without a recommendation forces the author to originate the answer from scratch, which is exactly the work this command is meant to front-load. -**Raise a gap only where the issue leaves something open.** A gap exists because the issue does not determine the answer — someone taking this into SDD would have to invent it or guess. Test every candidate against the issue as it stands: if the body, its acceptance criteria, or its out-of-scope list already settles the point, there is no gap, however squarely a category below invites one. A category you considered and found settled produces **nothing** — no gap, no placeholder, and no commentary bullet announcing that it is fine. +**Raise a gap only where the issue leaves something open.** A gap exists because the issue does not determine the answer — someone taking this into SDD would have to invent it or guess. Test every candidate against the issue as it stands: if the body, its acceptance criteria, its out-of-scope list, or an existing non-review comment (step 1) already settles the point, there is no gap, however squarely a category below invites one. A category you considered and found settled produces **nothing** — no gap, no placeholder, and no commentary bullet announcing that it is fine. -That test is the only thing deciding whether a point becomes a gap. Do **not** ask whether the author would contest your recommendation — that is a judgement about a person rather than about the issue, and a wrong call settles a real decision silently. Every open point stays a numbered gap with its own answer slot, however confident the recommendation is. +Do **not** additionally ask whether the author would contest your recommendation — that is a judgement about a person rather than about the issue, and a wrong call settles a real decision silently. Every open point stays a numbered gap with its own answer slot, however confident the recommendation is. -**The review is sized by what the issue leaves open, not by the number of categories below.** The category lists are a checklist of what to *consider*, never a quota to fill. An issue that commits to little should attract a visibly shorter review than one that leaves much open — so if a tightly-scoped issue is producing a long review, the sweep is manufacturing gaps, and the fix is to drop the ones nothing is actually open in. +**The review is sized by what the issue leaves open, not by the number of categories below.** The category lists are a checklist of what to *consider*, never a quota to fill. An issue that commits to little should attract a visibly shorter review than one that leaves much open — so if a tightly-scoped issue is producing a long review, the sweep is likely manufacturing gaps; re-test each against the issue and drop the ones nothing is actually open in. **Consequence — every gap says what a different answer would change.** Each gap is exactly one of two kinds: - **Changes what gets built** — answering against the recommendation would change something the issue commits to building: a scenario, an acceptance criterion, the scope boundary, a user-visible behaviour. - **Settles a detail** — what gets built is fixed either way, and the answer settles a detail within it: a name, a path, a value, where something is documented. -Name the kind on the gap and say in one clause what would change. Where the call is genuinely borderline, take the lower of the two — an inflated marker costs the reader exactly the attention the marker exists to save. +Name the kind on the gap and say in one clause what would change. Where the call is genuinely borderline, take *Settles a detail* — an inflated marker costs the reader exactly the attention the marker exists to save. **Order the gaps, then number them.** Settle the order first; numbers are assigned to the ordered list, never the reverse: -1. **By group** — all Requirements gaps, then all Technical gaps. This order is load-bearing: `/speckit.confirmissue` routes each folded decision by the group its gap sat under, so consequence ordering operates *within* a group and never moves a gap across the two. +1. **By group** — all Requirements gaps, then all Technical gaps, because requirements are probed before any technical question. What is load-bearing here is the *grouping*, not the sequence: `/speckit.confirmissue` routes each folded decision by the heading its gap sat under (its step 4), so consequence ordering operates *within* a group and never moves a gap across the two. 2. **By consequence within the group** — every *Changes what gets built* gap comes before every *Settles a detail* gap in the same group. -3. **Number contiguously across both groups** (1, 2, 3, … not 1a, 1b), following that order. Contiguous numbering keeps `/speckit.confirmissue` parsing simple and lets the author refer to a gap by a single number in chat. +3. **Number contiguously across both groups** (1, 2, 3, … not 1a, 1b), following that order. Contiguous numbering lets the author refer to a gap by a single number in chat. Numbers are assigned once, when the comment is first composed. A refine run never reorders and never renumbers — see step 6. @@ -170,7 +170,7 @@ Before taking this into SDD, the following points need answers. The table lists |---|---|---| | 1 | <short title> | Changes what gets built — <what would change, ≤12 words> | | 2 | <short title> | Settles a detail — <what would change, ≤12 words> | -| 3 | <short title> | Settles a detail — <what would change, ≤12 words> | +| 3 | <short title> | Changes what gets built — <what would change, ≤12 words> | ### Requirements gaps @@ -212,17 +212,16 @@ _<consequence kind>:_ <what a different answer would change, one clause> - ... ``` -**The at-a-glance table.** One row per gap, in the same order as the gaps themselves, spanning both groups — it is the whole set in one view, which is what makes the review triageable without reading it. Titles in the `Gap` column match each gap's own title verbatim, so a row and its gap are unmistakably the same thing. Always emit the table, even for a single gap: the author should never have to check whether it is there. The one exception is a review that raises no gaps at all — omit the table along with the groups, since there is nothing to triage. +**The at-a-glance table.** One row per gap, in the same order as the gaps themselves, spanning both groups — so the consequence column is not sorted globally: it restarts at *Changes what gets built* where the Technical group begins. Titles in the `Gap` column match each gap's own title verbatim, and the consequence cell is a compression of the gap's own consequence line, so a row and its gap are unmistakably the same thing and never say different ones. Always emit the table, even for a single gap: the author should never have to check whether it is there. -Two rules on its shape, and the first is load-bearing: +**Never use the `**N. <title>**` form in the table, and never put a `> _Answer:_` line above the first group.** `/speckit.confirmissue` parses every `**N. <title>**` block in the comment as a gap, ending at its `> _Answer:_` line (its step 2). A row imitating that shape carries no answer slot of its own, so it either hard-stops the fold — step 2 refuses to fold anything while a parsed gap looks unanswered — or takes the first real gap's answer slot as its own and corrupts the decisions that do land. Table cells carry a bare number and plain text, which matches nothing the parser looks for. -- **Never use the `**N. <title>**` form in the table, and never put a `> _Answer:_` line above the first group.** `/speckit.confirmissue` parses a gap as a `**N. <title>**` block ending in `> _Answer:_`, and folds every one it finds into the issue body. A row that imitates that shape would be folded as a phantom decision the author never made. Table cells carry a bare number and plain text, which matches nothing the parser looks for. -- **The table replaces nothing.** Every gap keeps its own consequence line, framing, recommendation and answer slot below; the table is a view of them, not a substitute. - -**Length bound — 120 words per gap.** Count everything from the `**N. <title>**` line through to its `> _Answer:_` slot: the consequence line, the framing, every sub-bullet, and the recommendation with its reason. The bound applies to the gap as a whole rather than to any one part of it, and no gap may exceed it. +**Length bound — 120 words per gap.** Count everything from the `**N. <title>**` line through to its `> _Answer:_` slot: the consequence line, the framing, every sub-bullet, and the recommendation with its reason. Count whitespace-separated words of the prose, taking a markdown link as its link text rather than its URL. The bound applies to the gap as a whole rather than to any one part of it, and to the comment as first composed — a refine run's expansion (step 6) may exceed it, where keeping the re-framing tight is the goal rather than the ceiling. **Splitting is not how you meet the bound.** A 200-word gap broken into two 100-word gaps satisfies nothing — the reader faces the same prose and one more decision. Cut instead: drop the restatement of what the issue already says, keep the evidence that makes the gap specific, and let the recommendation carry the detail rather than the framing. Split only where the gap is genuinely two independent questions needing two separate answers — and then each half must meet the bound on its own. +**Check the draft before posting.** Composing to a bound is not the same as meeting one — count, do not estimate. Before step 5 posts, verify against the draft: one table row per gap, its title matching the gap verbatim and its consequence cell agreeing with the gap's consequence line; every gap carrying a consequence line, a recommendation with its one-sentence reason, and an answer slot; every gap within the bound, counted rather than judged; *Changes what gets built* ahead of *Settles a detail* within each group; and no line above the first group heading matching either `**N. <title>**` or `> _Answer:_`. Fix what fails and re-check. This applies again to a refine run's edit (step 6), minus the bound. + **Recommendation quality bar:** the recommendation must be a concrete, actionable default (a value, a library, a field name, an HTTP status, an "in/out of scope" call) — not a meta-suggestion like "consider X". If you @@ -289,8 +288,8 @@ Re-framing rules (hedging case only): - **Expand the question body** with 2–4 concrete options laid out as a sub-list, each with a one-line trade-off. Add a worked example or a pointer to a comparable existing pattern in the codebase (read the codebase again if needed — surface defaults they may not have known existed: existing constants, sibling service patterns, port allocations, etc.). - **Revise the `> _**Recommendation:**_` line** if the new framing changes your call. Keep the `Reason:` cite tied to evidence. - **After ~2 hedging iterations on the same question** with no commitment, add a final option *"This may be out of scope for the current issue — answer `out of scope: <reason>` to drop it"* and call it out in the recommendation. Do not edit the gap out yourself — leave that to the author + `/speckit.confirmissue`. -- **Keep the at-a-glance table in step with the gap you re-framed.** If the re-framing changes that gap's title or its consequence, update its row — the title cell, the consequence cell, or both — and nothing else in the table. Never add, remove, reorder or renumber rows: the table mirrors the posted gap order, which was fixed when the comment was first composed. -- **Do not retrofit the new shape onto an older comment.** A comment posted before the at-a-glance table and the consequence line existed has no row to update and no kind to restate — leave it that way. A refine run re-frames the hedging gap and nothing else; it never adds a table to a comment that has none. +- **Keep the gap's consequence line and its table row in step with the re-framing.** If the new framing changes that gap's title or its consequence, update both places that state it — the `_<kind>:_` line on the gap body, and its row's title and consequence cells — and nothing else in the table. Never add, remove, reorder or renumber rows: the table mirrors the posted gap order, fixed when the comment was first composed. A consequence that changes after posting can therefore leave a *Changes what gets built* gap sitting below a *Settles a detail* one; that is the accepted cost of never renumbering a review the author already refers to by number. +- **Do not retrofit the table onto an older comment.** A comment posted before the at-a-glance table and the consequence line existed has no row to update and no kind to restate — leave it that way. A refine run re-frames the hedging gap and nothing else; it never adds a table to a comment that has none. - **Do not touch any other gap.** Substantive, out-of-scope, and empty answers must come through byte-for-byte, and so must every table row but the one you changed. The Commentary section is also untouched. If no gap qualifies for re-framing, **make no edit** and report that in chat (the author either still has un-answered questions, or is ready for `/speckit.confirmissue`). diff --git a/docs/change-intent-records/2026-09-12-triageable-review-comment.md b/docs/change-intent-records/2026-09-12-triageable-review-comment.md index 5ecfd54..aa95d93 100644 --- a/docs/change-intent-records/2026-09-12-triageable-review-comment.md +++ b/docs/change-intent-records/2026-09-12-triageable-review-comment.md @@ -12,29 +12,30 @@ **Constraints:** - `/speckit.confirmissue`'s prompt is out of scope, so its parser is fixed: it folds every `**N. <title>**` block ending in a `> _Answer:_` line into the issue body. Anything added above the gaps must match neither shape. -- The Requirements-then-Technical group order is load-bearing for that command's routing — it files each folded decision by the group its gap sat under. Consequence ordering therefore operates strictly *within* a group. +- What that command's routing depends on is the *grouping* — it files each folded decision by the heading its gap sat under — not the sequence of the two sections, which is byte-identical in its output either way. Consequence ordering therefore operates strictly *within* a group. Requirements-before-Technical is retained on its own merits (requirements are probed first, and #281 puts the groups' order out of scope), not because the parser needs it. - The author refers to gaps by number in chat, and folding strips the numbers, so ordering must be settled before numbering and a posted review is never renumbered. -- Prompt-only change. Per Constitution Principle I's configuration-and-tooling carve-out there is no xUnit test and no bespoke checker; the evidence is the command's own output before and after, recorded in the PR body. +- Prompt-only change, so Principle I's configuration-and-tooling carve-out governs: no xUnit test, and explicitly no bespoke checker standing in for a tool. The evidence is the command's own output before and after, recorded in the PR body. The carve-out's second clause — make the check repeatable — is discharged by a self-check inside the command rather than by CI, for the reason under *Known residuals*. **Decisions:** 1. **Consequence is two values, not a severity scale.** *Changes what gets built* versus *Settles a detail*, decided by a question about the artefact — would a different answer change something the issue commits to building? Rejected: a three-tier priority, which offers a middle bucket everything drifts into and still does not say *what* would change. Borderline calls take the lower kind, because an inflated marker costs the reader precisely the attention the marker exists to save. -2. **The up-front view is a table, and a downstream parser chose its shape.** The obvious index mirrors the gaps — a bold-numbered list — and that is exactly the shape `/speckit.confirmissue` folds, so every row would have landed in the issue body as a phantom decision the author never made. A table row carries a bare number and plain text and matches nothing the parser looks for. Also rejected: a collapsed `<details>` block, which leaves the reader an action to perform before they can see the set, which is the behaviour being removed. +2. **The up-front view is a table, and a downstream parser chose its shape.** The obvious index mirrors the gaps — a bold-numbered list — and that is exactly the shape `/speckit.confirmissue` parses as a gap. The damage is not a tidy extra bullet: a row has no answer slot of its own, so that command's step 2 either hard-stops the whole fold as unanswered, or resolves the row's end to the first real gap's answer slot and corrupts the decisions that do land. A table row carries a bare number and plain text and matches nothing the parser looks for. Also rejected: a collapsed `<details>` block, which leaves the reader an action to perform before they can see the set, which is the behaviour being removed. 3. **The table is always emitted, even for a single gap.** Rejected: a threshold below which it is omitted. A threshold makes the reader establish whether the table is missing or merely absent before trusting it, and gives the refine run a mode in which a row may or may not exist. 4. **The sweep is narrowed at source by an open-ness test, not by a cap.** A category considered and found settled produces nothing. Rejected: a numeric cap on gaps, which forces out a genuine gap for arriving last; and the superseded #278 proposal of demoting the gaps the reviewer predicts the author would not contest, which asks the model to judge a person rather than the issue and settles a real decision silently when it judges wrong. -5. **The length bound is per-gap words, and splitting is explicitly not the remedy.** The rule it replaces bounded sub-bullets and offered "consider whether it is actually two gaps" as the fix — which raises the gap count while leaving the word count where it was. 120 words was chosen against real output: the four gaps of the review on #278 ran 184, 192, 196 and 317 words, so the bound bites on every one of them while still leaving room for framing, cited evidence, two or three sub-questions and a reasoned recommendation. +5. **The length bound is per-gap words, and splitting is explicitly not the remedy.** The rule it replaces bounded sub-bullets and offered "consider whether it is actually two gaps" as the fix — which raises the gap count while leaving the word count where it was. 120 words was chosen against real output: counted from each `**N. <title>**` line through to its own `> _Answer:_` slot, the four gaps of the review on #278 ran 184, 192, 196 and 223 words, so the bound bites on every one of them while still leaving room for framing, cited evidence, two or three sub-questions and a reasoned recommendation. (An earlier draft of this record said 317 for the last gap; that was a counting script running past the answer slot to the end of the comment. The spread is 184–223 and the conclusion is unchanged.) -6. **A refine run does not retrofit the new shape.** A comment posted before the table and the consequence line existed keeps neither. Re-framing a hedging gap updates that gap's row if it has one, and nothing else. +6. **A refine run does not retrofit the shape, and keeps both statements of a consequence in step.** A comment posted before the table and the consequence line existed keeps neither. Re-framing a hedging gap updates that gap's consequence line *and* its row — the two places a consequence is stated — and nothing else. Because rows are never reordered and numbers never reassigned, a consequence that changes after posting can leave the order no longer sorted; that is the accepted cost of not renumbering a review the author already cites by number. -**Residual risks:** +**Known residuals:** - **The consequence call is an unverified model judgement.** #281's own open questions name the fallback if it proves badly calibrated: order by consequence without publishing the marker, which keeps the benefit that survives a wrong call and drops the one that does not. -- **The 120-word bound and the open-ness test are enforced by instruction, not by a gate** — against this repo's precedent of preferring a gate (CIR [`2026-09-07-automated-prespec-review`](2026-09-07-automated-prespec-review.md), decision 5). `/speckit.reviewissue` composes prose, and there is no exit code that decides whether a gap was manufactured. -- **`/speckit.draftissue` is a third consumer of this comment.** It ingests the whole body as brief material in migration mode and reads it as prose rather than parsing its shape, so the table breaks nothing — but each gap title now reaches it twice. Enumerated here deliberately: the same consumer was missed by reviews of this section on the two preceding issues (`docs/study/279.md`, `docs/study/280.md`). +- **Enforcement is a self-check inside the command, not a gate** — against this repo's precedent of preferring a gate (CIR [`2026-09-07-automated-prespec-review`](2026-09-07-automated-prespec-review.md), decision 5). The two halves differ and should not be conflated. Whether a gap was manufactured, and whether a review is proportionate, are judgements no exit code decides. But the table's shape, the parser-collision rule, the word bound, the per-gap consequence line and the ordering *are* mechanically decidable, and [`.claude/hooks/traceability-gate.sh`](../../.claude/hooks/traceability-gate.sh) is this repo's precedent for gating a markdown-shape invariant. What blocks a gate here is that the invariant lives in a GitHub comment composed at runtime: a first run pipes the body straight into `gh issue comment`, so there is no committed artefact for a hook or a CI step to read. Making it gateable means having the command always write the body to a file first and checking that — a change to how the command posts, which #281 does not scope. Until then the check is repeatable but self-administered, which is weaker than the precedent. +- **`/speckit.draftissue` is a third consumer of this comment.** It ingests the whole body as brief material in migration mode and reads it as prose rather than parsing its shape, so the table breaks nothing — but each gap title now reaches it twice. Enumerated here deliberately: the same consumer was missed on each of the two preceding issues — by a review of the Commentary section's own description (`docs/study/279.md`), then by the enumeration of consumers for the comment's deletion (`docs/study/280.md`). +- **The two kinds are framed around built behaviour, and some gaps are about neither.** A gap asking which release a change ships under, or what its RED step is, concerns a process artefact rather than something built, and neither kind fits it cleanly. A controlled trial hit this on two of five gaps; the "when borderline, take *Settles a detail*" tie-break absorbed it without a wrong call, which is the tie-break doing its job rather than evidence that the taxonomy is complete. - **Nothing verifies that a recommendation is a good default, or that its *Reason* survives being checked against the file it cites.** A shorter review makes each recommendation carry more weight, so this change raises the cost of that gap without addressing it. #281 records it as separate and more consequential than length. **Date:** 2026-09-12