diff --git a/docs/plans/pr-description-rebuild/artifacts/decision-log.md b/docs/plans/pr-description-rebuild/artifacts/decision-log.md new file mode 100644 index 00000000..9190cdca --- /dev/null +++ b/docs/plans/pr-description-rebuild/artifacts/decision-log.md @@ -0,0 +1,419 @@ +# Decision Log: Verified PR Descriptions + +Decisions behind [../feature-specification.md](../feature-specification.md). Full decisions carry rationale, evidence, and rejected alternatives. Trivial decisions are short, but each carries a heading so the spec can link to it. + +Evidence trust classes follow [the evidence rule](../../../../han-core/references/evidence-rule.md): `codebase` (read from this repository), `web` (external source, cited by ID from `docs/research/effective-pull-request-descriptions.md`), `provided` (stated by the user). + +The empirical spine of this feature (A13, A24, A27, A28) is a set of 2026 arXiv preprints that have not been peer-reviewed, and A13's effect sizes are observational and associative rather than causal. Every decision resting on them carries that caveat inline rather than laundering the numbers into certainty. The two decisions most exposed to it, [D2](#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for) and [D3](#d3-the-lean-core-is-kept-and-gains-a-feedback-ask), have matching entries in the spec's Open Items. + +## Full decisions + +### D2: The gate shows each claim with its evidence, and blocks on what the skill cannot vouch for + +- **Outcome:** Before the description is final, the skill lists every assertion it makes and shows the evidence recorded for each. Three kinds of item block: a claim the skill could not evidence, a claim about absence, and the statement of intent (with the feedback ask). Each must be individually disposed of, with no bulk path over them. Every other claim may be accepted together once the blocking items are settled. The gate does not label a claim "supported" or otherwise vouch for it; it shows the claim and the evidence and lets the engineer judge. +- **Rationale:** The research hands the rebuild this gap explicitly and declines to close it. The first draft of this spec closed it badly: it offered a "confirm the list as it stands" option, which is one action discharging every obligation on the screen. All three reviewers independently found that this reproduces the exact failure the gate exists to prevent. Making the careless path cost one keystroke while the careful path costs N is a choice architecture that guarantees the careless path, and it gets worse precisely as the diff gets larger, which is where the fabrication risk is highest. Blocking only on the items the skill genuinely cannot vouch for costs nothing on a clean change and cannot be skipped on a dirty one. + + Dropping the affirmative "supported" label came from the same review and is the sharper half. [T1](feature-technical-notes.md#t1-claim-provenance-is-captured-at-authoring-time) establishes that only the authoring pass can honestly say *I could not evidence this*. It does not establish that the same pass can honestly say *this one is evidenced*, and that is the identical self-assessment T1 rejects. A "supported" badge on a fabricated claim is worse than no badge: it tells the engineer which rows to skip, and a plausible-looking hunk under a fabricated claim is exactly what hides there. +- **Evidence:** + - `web` — A25: GitHub's own documentation admits a "known risk" of hallucination in generated summaries and asks for careful human review of every one. First-party vendor documentation, and an admission against interest. + - `web` — A28: measured tendency for AI descriptions to claim functionality absent from the diff. Independent of A25. *2026 preprint, not peer-reviewed.* + - `web` — A27: ~61% of AI-authored PRs get no recorded human review. *2026 preprint, single-source, and shares a dataset with A24; carried as a risk signal, not a measurement of this design. See the spec's Open Items.* + - `web` — the research's own V5 finding: the gate "must be structurally enforced ... or the option inherits A27's failure mode." + - `codebase` — `han-github/skills/update-pr-description/SKILL.md` Step 6.3: today's gate is a yes/no on finished prose, immediately before `gh pr edit --body`. + - `provided` — the user chose the claim-by-claim gate over two weaker options. +- **Rejected alternatives:** + - *A single "confirm the list as it stands" option.* What the first draft specified. Rejected on unanimous review: it makes waving through exactly one action, structurally identical to the yes/no gate being replaced, and merely longer. + - *Confirm the intent sentence only.* Closes the intent hole and leaves A28's measured failure untouched. Rejected because the claim hole is the one with direct empirical support. + - *Keep the yes/no gate.* Rejected as the named failure mode. + - *Label evidenced claims "supported".* Rejected: T1 authorizes only the negative marker, and the affirmative one manufactures automation bias. +- **Linked technical notes:** [T1](feature-technical-notes.md#t1-claim-provenance-is-captured-at-authoring-time) +- **Driven by findings:** F1, F2 +- **Referenced in spec:** Outcome, Primary Flow, The Gate, Open Items + +### D3: The lean core is kept, and gains a feedback ask + +- **Outcome:** The description keeps its existing shape — a one-sentence bolded summary, a behavior-changes section when runtime behavior changes, a reading-order guide only on a large change — and adds one line stating what kind of feedback the engineer wants. The engineer supplies that line at the gate, and leaving it blank omits the section. When a repository template is in use, the line goes into the template's own section for it when one exists, and is appended after the template's sections when none does — the same treatment the reading-order guide already gets. No issue link and no testing note are added. +- **Rationale:** The user's original instruction was to keep the lean core untouched, and the feedback ask was deferred on the stated cost that it "adds a question to every run." The UX review observed that [D2](#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for) had already spent that cost: the gate now stops and takes free text from the engineer on every run regardless. The deferral's own reopening trigger, written into the first draft of this spec, was therefore satisfied at spec time rather than at some future date. Since the item is the single largest measured effect on merge odds in the entire research base, and it now rides on an interaction already being paid for, keeping it deferred would have been deferring it for a reason that no longer existed. + + The template case was left open by that reasoning and is settled here: a template with no home for the line does not get a new structure invented for it, it gets the line appended, because the conformance rules already prescribe exactly that for the one other section the skill adds of its own accord. + + This is the weakest-supported commitment in the spec, and it is recorded as such. A13 is a single observational preprint; the effect is associative, not causal; and it was measured on open-source pull requests, not on a solo or small-team workflow. The spec carries a matching open item that would drop the ask if engineers leave it blank in practice. +- **Evidence:** + - `web` — A13: stating the desired feedback type shows the largest single effect of any description element (odds ratio 1.65–1.72, i.e. 64–72% higher merge odds), despite 16.2% prevalence. *2026 preprint, not peer-reviewed; single-source for the figures; observational, so associative rather than causal.* + - `codebase` — `han-github/skills/update-pr-description/references/template.md`: the current lean default, kept. + - `codebase` — `han-github/skills/update-pr-description/references/template-conformance.md` section 5: a section the skill adds of its own accord fills the template's equivalent when one exists, and is appended after the template's sections when none does. + - `provided` — the user chose to keep the lean core, then chose to fold the feedback ask into the gate once its cost basis changed. +- **Rejected alternatives:** + - *Adopt the research's full recommended core (issue link, feedback ask, testing note).* Rejected: each added section is a new question or a new fabrication surface on every run. Only the item whose cost had already been absorbed was taken. + - *Ask the feedback question as its own interaction.* Rejected: it is one field on a stop the engineer is already making. A separate question is a separate interruption for no gain. + - *Keep the feedback ask deferred.* Rejected: the reason for deferring it no longer held. + - *Omit the feedback line when a repository template has no home for it.* Rejected: it makes the ask silently conditional on template shape, and the conformance rules already answer the question. +- **Linked technical notes:** — +- **Driven by findings:** F4 +- **Referenced in spec:** Outcome, The Gate, Deferred (YAGNI), Open Items + +### D4: The size fact is delivered at the gate, not shouted past the engineer + +- **Outcome:** The skill measures the change and tells the engineer when it is large enough that reviewer defect-finding suffers, and it does so at the gate rather than early in the run. Size is counted as added and deleted lines in significant files. "Significant" carries the definition the skill already applies to its reading-order guide — code files count, documentation and configuration do not — extended to name lockfiles, generated code, and vendored dependencies explicitly, which the existing definition does not. It warns; it never blocks. +- **Rationale:** Change size is the most corroborated finding in the research, across three independent sources over two decades, and it deserves to be said. But the first draft said it in the form guaranteed to be ignored: a non-blocking statement fired at step two of a seven-step flow, immediately before the longest silent stretch of the run, then buried under everything that followed. In a scrolling terminal, that is not a message, it is a message-shaped absence. Attaching the fact to the gate costs nothing and puts it where the engineer is already stopped and already reading. + + The unit matters as much as the placement. The first draft said "four hundred lines" without saying lines of what, which would have fired a lecture about splitting the change at every dependency bump with a large lockfile. A warning that fires on changes nobody can split is a warning engineers train themselves to ignore, and the research names irreducibly-large changes as the known gap in this lever. The existing "significant files" definition covers documentation and configuration only; naming lockfiles, generated code, and vendored dependencies is an extension of it rather than a straight reuse, and it is recorded here as one. +- **Evidence:** + - `web` — A18: defect-finding drops sharply past 200–400 lines reviewed in one sitting. Vendor-published summary of a widely-cited Cisco study; predates modern PR workflows. + - `web` — A20: more files in a change correlates with a lower proportion of useful review comments. Independent of A18. + - `web` — A23: Google converges on the same order of magnitude for the same reason. Independent of both. + - `web` — the research's O4 trade-offs: the lever "offers no guidance" for irreducibly large changes (migrations, generated code, vendored dependencies). + - `codebase` — `han-github/skills/update-pr-description/SKILL.md` Step 3 defines "significant" files for the reading-order threshold as code files, with documentation and configuration excluded by default. This decision extends that list; it does not restate it. + - `provided` — the user chose "warn, then continue." +- **Rejected alternatives:** + - *Fire the warning early, as its own message.* What the first draft specified. Rejected on review: it is unactionable at that moment by the decision's own logic, and it teaches the engineer to skim the skill's non-interactive output, which is the exact habit that later lets them skim the gate. + - *Block on an oversized change.* Rejected: the work is already done, so blocking punishes a decision the engineer cannot cheaply reverse. + - *Warn, and scale the description's structure with size.* Rejected: it adds a second threshold to tune, and no evidence gives one. + - *Say nothing.* Rejected: it discards the best-corroborated finding in the research. +- **Linked technical notes:** — +- **Driven by findings:** F5, F6 +- **Referenced in spec:** Primary Flow, User Interactions, Deferred (YAGNI) + +### D7: The engineer's verdict is binding, and re-rendering does not re-draft + +- **Outcome:** A claim the engineer corrects is used as they wrote it. A claim they reject is removed. The description is re-rendered from the assertions that survive — the surviving assertions are re-joined into readable prose, and no new assertion is introduced. The skill does not re-draft over a correction to improve its wording. The re-rendered description is shown to the engineer before anything is published, so the text a reviewer will read is text a human has seen. +- **Rationale:** The gate is worth having only if the engineer's verdict binds. Any behavior that lets a rejected claim survive, or that "improves" a correction back toward the draft's wording, reintroduces the unverified text the gate exists to catch, and the engineer would not see it a second time. + + The reviewers pressed on the difference between re-rendering and re-drafting, which the first draft left as a bare word. It is a real distinction and it is narrow: re-rendering may rearrange and re-join what survives so the prose reads as English, because deleting one assertion from a paragraph otherwise leaves a dangling connective. It may not assert anything that was not already verified. The test is whether a reader of the re-rendered description could learn a fact the engineer never approved. If yes, it is a re-draft. + + Showing the re-rendered text back is the safety net on that rule, and it closes a hole this synthesis found: [D11](#d11-nothing-rewrites-the-draft-between-authoring-and-the-gate) rejects a post-gate readability pass on the grounds that it produces "final text no human ever saw," and re-rendering is itself a change to the words after approval. The rules are consistent only if the engineer sees the result, so the spec now says they do. +- **Evidence:** + - `web` — A26: it is inappropriate to hand a reviewer text the author has not personally validated. A correction the tool overrides is text the author did not validate, and so is a re-rendering the author never saw. + - `provided` — implied by the user's selection of the claim-by-claim gate; the gate is meaningless without a binding verdict. +- **Rejected alternatives:** + - *Re-draft after corrections, to smooth the prose.* Rejected: a re-draft can reintroduce an unverified claim. + - *Purely mechanical deletion, with no re-joining.* Rejected on review: it produces orphaned fragments and dangling connectives, and a description that reads as broken invites the engineer to fix it by hand outside the gate. + - *Re-render without showing the result.* Rejected in synthesis: it is the same objection D11 raises against a post-gate rewrite, and the fix costs one display. +- **Linked technical notes:** [T1](feature-technical-notes.md#t1-claim-provenance-is-captured-at-authoring-time) +- **Driven by findings:** F7 +- **Referenced in spec:** Primary Flow, Alternate Flows and States + +### D10: Evidence is the diff, or a cited repository file + +- **Outcome:** A claim is evidenced when the skill can point at the part of the diff it was written from, *or* at the repository file it was written from. Only a claim the skill can point at nothing for is marked unevidenced and blocks. +- **Rationale:** The first draft defined evidence as the diff alone while simultaneously granting the drafting pass permission to read surrounding source to understand a change the diff does not explain. Two reviewers found the contradiction independently. A claim like "the flag defaults to off," where the default lives in an unchanged file, is true, well-evidenced, and not in the diff. Under the first draft it would have been marked unevidenced and blocked, identically to a hallucination. + + That is not a cosmetic bug. False positives on the unevidenced marker are more corrosive than false negatives: if a third of the blocking items turn out to be true-but-out-of-diff, the engineer learns the marker is noise and starts clearing it reflexively, which is the wave-through habit arriving through the back door. The marker only works if it is rare and it means something. +- **Evidence:** + - `codebase` — `han-github/skills/update-pr-description/SKILL.md` Step 4 instructs the authoring agent to "Read additional source files ... when the diff alone does not explain the change," while the first draft's gate rule recognized only the diff. + - `web` — A27: the failure mode is a gate people stop genuinely performing; a marker that cries wolf is a direct route there. *2026 preprint; see D2's caveat.* +- **Rejected alternatives:** + - *Forbid claims resting on anything but the diff.* Rejected: it would forbid true and useful claims a reviewer needs, and the drafting pass reads surrounding source precisely because the diff alone often does not explain a change. + - *A third "supported by repository context" category, distinct from diff-supported.* Rejected under the simpler-version test: the engineer's job at the gate is to look at the evidence, and the evidence's provenance is visible in what they are shown. A third label adds a taxonomy without adding a decision. +- **Linked technical notes:** — +- **Driven by findings:** F8 +- **Referenced in spec:** Edge Cases and Failure Modes + +### D11: Nothing rewrites the draft between authoring and the gate + +- **Outcome:** The pass that writes the description writes it to the readability standard and to the template's required shape, and records each claim's evidence, in the same act. No pass runs over the draft afterward to improve it, correct its structure, or bring it into conformance. The text shown at the gate is the text that was authored against the evidence. +- **Rationale:** The current skill dispatches a readability editor after authoring, which rewrites the draft while preserving its facts. Under the new gate that pass becomes actively harmful, and the danger is easy to miss: a rewrite that preserves every fact still restates every claim in new words. The text shown at the gate would no longer be the text whose evidence was recorded. That is precisely the reconstruction [T1](feature-technical-notes.md#t1-claim-provenance-is-captured-at-authoring-time) rules out, arriving through a pass nobody thought of as generative. + + The scope of this rule is wider than the first draft's, and the widening is a synthesis finding rather than a reviewer one. The readability editor is not the only pass that rewrites the draft after it is authored: the skill also runs a readability self-check and a structural verification step, both of which end with an instruction to fix what they find. Every one of those is the same hazard wearing different clothes. Naming only the readability editor would have left two live rewrite paths in a skill whose gate depends on there being none. The rule is therefore stated over any pass that touches the words, and conformance is something the authoring pass satisfies rather than something a later pass repairs. +- **Evidence:** + - `codebase` — `han-github/skills/update-pr-description/SKILL.md` Step 4 dispatches `han-core:readability-editor` after authoring and applies its rewrite as the working description. + - `codebase` — `han-github/skills/update-pr-description/SKILL.md` Step 4 also runs a readability self-check that ends "fix any failure before finalizing," and Step 5 verifies structure and ends "Fix any issues directly before proceeding to Step 6." Two further rewrite paths over authored text. + - `codebase` — `han-github/references/readability-rule.md`: the standard, which the repository applies broadly and which this decision keeps. + - `provided` — the user chose to fold readability into authoring over the two alternatives. +- **Rejected alternatives:** + - *Run the readability pass after the gate.* Rejected: it rewords claims the engineer had just approved, producing final text no human ever saw. + - *Drop the readability standard.* Rejected: it abandons a house convention for no reason; the standard is not what caused the problem, the second pass was. + - *Keep the pass where it is.* Rejected: it silently guts the gate. + - *Name only the readability editor and leave the verification passes alone.* Rejected in synthesis: they rewrite authored text too, and the gate cannot tell the difference between a rewrite it authorized and one it did not. +- **Linked technical notes:** [T1](feature-technical-notes.md#t1-claim-provenance-is-captured-at-authoring-time) +- **Driven by findings:** F9 +- **Referenced in spec:** Primary Flow + +### D12: A claim is one independently verifiable assertion, and the gate covers every assertion the reviewer will read + +- **Outcome:** The unit of the gate is one independently verifiable assertion, not one sentence. A sentence carrying two assertions yields two items, so the engineer can reject one and keep the other. The gate covers the summary sentence, the behavior-changes prose, any prose filled into a repository template's sections, any note saying a template's section does not apply, and every checklist box the draft proposes to check. The reading-order guide is navigational, asserts nothing about the change, and is not gated. +- **Rationale:** All three reviewers found that the first draft never defined "claim," and that the gate's entire weight rests on that word. Left undefined, an implementer could satisfy the spec with three coarse claims each evidenced by the whole diff, which verifies nothing, or with twenty-five atomic ones, which nobody reads. The skill's own summary template makes the problem concrete: `This PR , so that ` fuses an evidenced claim and an unprovable intent in one sentence, and the `` slot alone can bundle two facts. + + Scoping the gate to the lean core is what keeps it small. The description is two to five short paragraphs by construction, so the assertion count stays in a range a person will actually read. That is a prediction, not a measurement, and it is carried as an open item. + + Checklist boxes are in scope because a checked box is an assertion made to the reviewer in the engineer's name. Leaving it out would break the gate's promise on the one item type that most looks like an attestation. The "not applicable" notes the conformance rules generate are in scope for the same reason, and this synthesis added them: the reviewers raised the question and the first resolution answered it only for the post-rejection case. A note saying a section does not apply is an assertion of absence written in the engineer's name, and it now blocks under [D15](#d15-absence-claims-are-structurally-unprovable-like-intent) like any other. +- **Evidence:** + - `codebase` — `han-github/skills/update-pr-description/references/template.md`: the summary sentence's fused shape, `**This PR , so that .**` + - `codebase` — `han-github/skills/update-pr-description/references/template-conformance.md` section 4: boxes are checked on the engineer's behalf and constrained to what the diff proves; section 6: an unfillable section gets "a short honest note" written for it. + - `web` — A26: text handed to a reviewer that the author did not validate. A checked box and an unprompted "not applicable" are both such text. +- **Rejected alternatives:** + - *One claim per sentence.* Rejected: a sentence bundling a true and a false assertion forces the engineer to reject both or accept both. + - *One claim per clause.* Rejected under the simpler-version test: it inflates the item count without adding decisions, and an unreadably long gate is a gate that gets skimmed. + - *Exclude checklist boxes from the gate.* Rejected: it breaks the gate's promise where the promise matters most. + - *Gate the reading-order guide.* Rejected: it points at where to read, and asserts nothing that can be true or false about the change. +- **Linked technical notes:** [T1](feature-technical-notes.md#t1-claim-provenance-is-captured-at-authoring-time) +- **Driven by findings:** F10, F11 +- **Referenced in spec:** Primary Flow, Edge Cases and Failure Modes, Open Items + +### D13: The gate fails closed + +- **Outcome:** If the skill cannot obtain an answer from a human at the gate, that silence is not approval. The description is delivered marked unverified, and nothing is published. +- **Rationale:** This is not a hypothetical, though the precise scope of the hazard matters and the first draft overstated it. The repository's own skill-building guidance documents that the interactive question mechanism returns empty answers, with the user never seeing the question, when the skill lists it among its auto-approved tools — and that a parent skill's auto-approval rules stack onto every child skill it invokes. The first condition is one this skill controls and must avoid. The second is one it cannot control at all: a skill that calls this one can silently disable its gate. + + A gate whose entire value is that a human answered it, running on a mechanism that can silently answer nothing whenever a caller is misconfigured, would publish unverified claims to a public pull request under the engineer's name with no human error involved at all. Failing closed costs a run; failing open costs the feature. The cost of failing closed is real and is recorded in the spec's Open Items: a skill invoked from a misconfigured parent will never publish, and nobody has yet run it that way. +- **Evidence:** + - `codebase` — `han-plugin-builder/skills/guidance/references/skill-building-guidance/allowed-tools-AskUserQuestion.md`: the tool "returns immediately with empty answers. The user never sees the question," when it is listed in `allowed-tools`; and "If a parent skill has `AskUserQuestion` in its `allowed-tools`, every child skill's AskUserQuestion calls will also silently fail." The upstream bug was closed in March 2026 with a fix noted as upcoming, and the document records it still reproducing afterward. + - `codebase` — `han-github/skills/update-pr-description/SKILL.md` frontmatter does not list the question tool in `allowed-tools`, so the failure is a caller-side risk here rather than a present defect. The rule exists because the skill cannot see its callers. + - `web` — A27: the documented outcome of gates that do not actually get performed. *2026 preprint; see D2's caveat.* +- **Rejected alternatives:** + - *Treat no answer as approval.* Rejected: it is the feature's failure mode, reached with zero human involvement. + - *Leave it unspecified.* Rejected: unspecified defaults resolve to whatever is easiest, which here is failing open. + - *Rely on the skill's own configuration being correct and say nothing.* Rejected in synthesis: the documented propagation path runs from a parent this skill cannot inspect. +- **Linked technical notes:** — +- **Driven by findings:** F3 +- **Referenced in spec:** The Gate, Open Items + +### D14: Replacing an existing description is disclosed before it happens + +- **Outcome:** When the pull request already has a description, it is replaced rather than merged into. Before asking to publish, the skill says what is about to be lost. +- **Rationale:** The skill is called `update-pr-description`, so running it against a pull request that already has one is not an edge case, it is the name. Two reviewers flagged the same irony: the gate next door goes to great lengths to protect the reviewer from text the engineer did not validate, while the publish step silently destroys the text the engineer most certainly did — a hand-written description, reviewer notes, a ticket link. The engineer approves *claims* at the gate; they are never asked to approve a *deletion*. + + Disclosure is the proportionate fix. Merging into an existing description was considered and rejected: it would mean the skill reasoning about which human sentences to keep, which is a larger feature and a new fabrication surface. Saying plainly what is about to go, and letting the engineer decline, converts an undisclosed destructive action into a disclosed one at the cost of one sentence. + + This interacts with the deferred issue link: a re-run deletes a link the engineer added by hand and does not put one back. Disclosure makes that visible. It does not solve it, and the spec says so. +- **Evidence:** + - `codebase` — `han-github/skills/update-pr-description/SKILL.md` Step 6.3 runs `gh pr edit --body`, a whole-body replacement, on a yes/no with no disclosure of the prior content. + - `web` — A26: the text an author personally validated is the valuable text. This flow destroyed exactly that. +- **Rejected alternatives:** + - *Merge the generated description into the existing one.* Rejected: it requires the skill to judge which human sentences survive, which is a bigger feature and a new place to fabricate. + - *Replace silently.* Rejected: it is a destructive write to a system of record, performed without the engineer knowing what was there. +- **Linked technical notes:** — +- **Driven by findings:** F13 +- **Referenced in spec:** Primary Flow, Edge Cases and Failure Modes, Out of Scope, Deferred (YAGNI) + +### D15: Absence claims are structurally unprovable, like intent + +- **Outcome:** A claim that nothing changed — no behavior change for existing callers, no impact on a caller, a template section that does not apply — is a blocking gate item. The engineer vouches for it or it is dropped. It is not treated as an ordinary unevidenced claim. +- **Rationale:** A diff can prove what changed and can never prove that nothing else did. The first draft recognized this for the intent sentence and gave it a dedicated path, then left absence claims to the generic unevidenced bucket, which has them fire on every no-behavior-change pull request by design. A marker that fires routinely, for a reason that is structural rather than suspicious, is a marker the engineer learns to clear without reading. Naming absence as its own always-unprovable category keeps the unevidenced marker meaning what it says: *the skill wrote something it could not point at*. + + The "not applicable" note a template forces is the same shape of claim and is routed the same way, which is how [D12](#d12-a-claim-is-one-independently-verifiable-assertion-and-the-gate-covers-every-assertion-the-reviewer-will-read) and [D8](#d8-repository-template-conformance-is-carried-forward-and-its-fill-passes-through-the-gate) stay consistent with each other. +- **Evidence:** + - `codebase` — `han-github/skills/update-pr-description/SKILL.md` Step 4 conditions the behavior-changes section on whether runtime behavior changed, so the "nothing changed" judgment is one the skill routinely makes. + - `codebase` — `han-github/skills/update-pr-description/references/template-conformance.md` section 6 has the skill write "Not applicable" notes on the engineer's behalf today. + - `web` — A27, via the same reasoning as [D10](#d10-evidence-is-the-diff-or-a-cited-repository-file): a marker that cries wolf trains the wave-through habit. +- **Rejected alternatives:** + - *Leave absence claims in the generic unevidenced bucket.* Rejected: it dilutes the one signal the gate depends on. + - *Let absence claims pass unblocked.* Rejected: an unchallenged "no behavior changes for existing callers" is among the most consequential things a description can get wrong. +- **Linked technical notes:** — +- **Driven by findings:** F14 +- **Referenced in spec:** The Gate, Edge Cases and Failure Modes + +### D16: A section emptied by rejections returns to the engineer + +- **Outcome:** When the engineer rejects every claim in a section a repository template requires, the empty section is handed back to them: they write it, or mark it explicitly not applicable. The skill does not compose an honest-sounding note on their behalf. +- **Rationale:** The template conformance rules carried forward under [D8](#d8-repository-template-conformance-is-carried-forward-and-its-fill-passes-through-the-gate) require every template section to stay present, filled with content or an honest "not applicable" note, never silently dropped. [D7](#d7-the-engineers-verdict-is-binding-and-re-rendering-does-not-re-draft) forbids the skill from drafting new text after the gate. An edge-case reviewer found these two rules collide exactly when a rejection empties a mandatory section, and that the spec named no winner, so an implementer would have silently violated one of them. + + Returning the section to the engineer honors both. The template keeps its section, the skill invents nothing, and the one person who can say what belongs there says it. +- **Evidence:** + - `codebase` — `han-github/skills/update-pr-description/references/template-conformance.md` section 6: every template section stays, filled or honestly noted. + - `codebase` — this log's [D7](#d7-the-engineers-verdict-is-binding-and-re-rendering-does-not-re-draft): no re-drafting after the gate. +- **Rejected alternatives:** + - *Auto-fill "Not applicable."* Rejected: it is an assertion to the reviewer that the engineer never made, which is the thing the gate exists to prevent. + - *Leave the section present and empty.* Rejected: it violates the template contract and reads as an oversight. +- **Linked technical notes:** — +- **Driven by findings:** F12 +- **Referenced in spec:** Edge Cases and Failure Modes, User Interactions + +### D17: The engineer may state intent before the draft is written + +- **Outcome:** The engineer can state why they made the change when they invoke the skill. The draft is then written against that intent rather than guessing at it. It stays optional, and the intent sentence still blocks at the gate, because the draft renders that intent in its own words. +- **Rationale:** The gate catches invented claims. Preventing them is cheaper than catching them, and the largest single source of invention is the skill guessing at a "why" that lives only in the engineer's head. A draft written against a stated intent invents less, which makes the gate shorter, which makes the gate more likely to be read. Error prevention compounds with the gate rather than competing with it. + + It costs nothing to add: the skill already accepts optional context as an argument. This decision gives that argument a job. + + Why the intent sentence still blocks even when the engineer supplied it: the draft writes *against* their intent, not verbatim from it, so the sentence a reviewer reads is the skill's rendering of what they said. [D21](#d21-editing-the-draft-directly-verifies-what-was-edited-and-nothing-else) exempts text the engineer wrote from further verification; it does not exempt text the skill wrote from text the engineer wrote. Confirming a rendering is one keystroke and it is the right one. +- **Evidence:** + - `codebase` — `han-github/skills/update-pr-description/SKILL.md` frontmatter already declares `argument-hint: "[optional context about the PR]"`, and no step in the skill body consumes it. + - `web` — A5: the "why" is the thing source code cannot reveal, and the thing the description exists to carry. +- **Rejected alternatives:** + - *Require the intent up front.* Rejected: it puts a mandatory question before the engineer has seen anything, and the gate already blocks on intent, so the requirement would be redundant as well as annoying. + - *Skip the intent gate item when the engineer supplied intent up front.* Rejected in synthesis: the skill rewrote their sentence, and the rewrite is what ships. +- **Linked technical notes:** — +- **Driven by findings:** F17 +- **Referenced in spec:** Actors and Triggers, Primary Flow, The Gate, Edge Cases and Failure Modes + +### D18: An incomplete read of the diff is disclosed as its own condition + +- **Outcome:** When the skill could not read the whole diff, it says so at the gate, and says which parts it did not read. This is a separate fact from the size warning. +- **Rationale:** The first draft answered "the diff is too large to read in full" by pointing at the size warning, which two reviewers correctly called a non-answer. The size warning is about a *reviewer's* defect-finding falling off past a few hundred lines. It says nothing about the *skill's* own blindness, and the two conditions are unrelated: a two-thousand-line lockfile diff is large but perfectly readable, while a genuinely truncated read can happen without the engineer ever knowing. + + This is the gate's one silent failure surface. The gate can only show the claims that exist; it cannot show a claim about a file the skill never opened. An engineer who verifies every item on the screen would reasonably believe they verified the change, when they verified a description of an unknown subset of it. A gate that cannot be complete has to say that it is not. +- **Evidence:** + - `codebase` — `han-github/skills/update-pr-description/SKILL.md` injects the full diff into its context unconditionally and interpolates it into the authoring prompt, with no size guard and no truncation signal. + - `web` — A25: GitHub's own summarizer excludes files with more than 400 combined changed lines, evidence that a real generator hits real limits and must decide what to say about them. +- **Rejected alternatives:** + - *Fold it into the size warning.* Rejected: the two facts are unrelated, and folding them means the skill never actually says the thing that matters. + - *Say nothing.* Rejected: it is the one way the gate can be silently, invisibly wrong. +- **Linked technical notes:** — +- **Driven by findings:** F15 +- **Referenced in spec:** Primary Flow, Edge Cases and Failure Modes + +### D21: Editing the draft directly verifies what was edited, and nothing else + +- **Outcome:** The engineer can edit the draft directly at the gate. Text they wrote is text they verified, and needs no further evidence. Editing is not an exit: any claim they did not touch is still theirs to dispose of, and the blocking items still block. +- **Rationale:** The first draft offered direct editing as a third option beside confirm and correct, and said nothing about what it did to the gate's state. As written it was a one-keystroke path from twenty unread claims to publish. The option is legitimate and worth keeping, for the reason [D7](#d7-the-engineers-verdict-is-binding-and-re-rendering-does-not-re-draft) already gives: text the engineer authored is text the engineer validated. It just cannot double as a bypass for the text they did not author. +- **Evidence:** + - `web` — A26: the author must personally validate what the reviewer reads. Direct authorship is the strongest form of that validation, which is exactly why it does not extend to text they did not write. +- **Rejected alternatives:** + - *Treat a direct edit as full verification of the description.* Rejected: it verifies the sentences they touched, and says nothing about the rest. + - *Forbid direct editing.* Rejected: it is the fastest correct path when the draft is broadly wrong, and blocking it pushes the engineer out of the skill entirely. +- **Linked technical notes:** — +- **Driven by findings:** F16 +- **Referenced in spec:** The Gate + +## Trivial decisions + +Short decisions, each with a heading so the spec's inline citations resolve. + +### D1: Rebuild in place + +The existing `/update-pr-description` skill is rebuilt rather than a second skill added alongside it. Considered a parallel skill; rejected because two skills for one job is a routing problem, not a feature. + +- **Driven by findings:** — +- **Referenced in spec:** — (the rebuild is the spec's premise, not a behavior it states) + +### D5: Authoring is a writing pass, not a repurposed critique pass + +The description is authored by a pass whose native output is finished prose, so the skill carries no retry path for a critique agent reverting to producing a review report. Considered leaving the agent choice to implementation planning, which is its ordinary home; settled here on the user's explicit instruction, and subsumed by [D11](#d11-nothing-rewrites-the-draft-between-authoring-and-the-gate), which folds readability and conformance into the same pass. This decision is deliberately not surfaced in the spec: it is a pipeline mechanic, and the spec states behavior. + +- **Evidence:** `codebase` — `han-github/skills/update-pr-description/SKILL.md` Step 4 dispatches `han-core:junior-developer` to author the description and carries a documented discard-and-re-issue retry path for when it returns a review report instead; `han-core/agents/junior-developer.md` declares an adversarial-collaboration critique purpose whose native output is a review report. +- **Driven by findings:** F9, F18 +- **Referenced in spec:** — (routed out of the spec by F18) + +### D6: The gate runs whether or not a pull request exists + +The claim-by-claim check runs before the description is presented as final, including when there is no PR to publish to and the engineer will paste the text by hand. + +- **Driven by findings:** — +- **Referenced in spec:** Alternate Flows and States + +### D8: Repository template conformance is carried forward, and its fill passes through the gate + +The existing rules for discovering a template, preserving its section order, checking only diff-provable boxes, and appending a section the template has no home for are kept (`han-github/skills/update-pr-description/references/template-conformance.md`); no evidence surfaced for changing them. What changes is that the fill those rules produce — section prose, checked boxes, "not applicable" notes — is not final text any more. It is a set of proposals that pass through the gate like every other assertion ([D12](#d12-a-claim-is-one-independently-verifiable-assertion-and-the-gate-covers-every-assertion-the-reviewer-will-read), [D15](#d15-absence-claims-are-structurally-unprovable-like-intent)). + +- **Driven by findings:** F11, F12 +- **Referenced in spec:** Outcome, Primary Flow, Edge Cases and Failure Modes + +### D9: The reading-order guide keeps its existing threshold + +"What to look at first" still appears only past roughly eight to ten significant code files, and it is not a gate item, because it points at where to read rather than asserting anything about the change ([D12](#d12-a-claim-is-one-independently-verifiable-assertion-and-the-gate-covers-every-assertion-the-reviewer-will-read)). + +- **Driven by findings:** — +- **Referenced in spec:** Primary Flow (the one part of the description the gate does not cover). The threshold itself rides inside the lean core, which the spec's Outcome carries via [D3](#d3-the-lean-core-is-kept-and-gains-a-feedback-ask). + +### D19: A re-run re-drafts and re-gates + +Running the skill again on the same branch re-drafts from the current diff and re-gates from scratch; verified claims are not carried across runs, because the diff they were verified against has changed. + +- **Driven by findings:** F22 +- **Referenced in spec:** Alternate Flows and States, Deferred (YAGNI) + +### D20: The verified description is written to a file + +The deliverable is a file the engineer can take away, not a block of markdown they must select out of terminal scrollback, whether or not the skill publishes it. + +- **Driven by findings:** F21 +- **Referenced in spec:** Primary Flow + +### D22: The existing branch-state and template-discovery guards are carried forward + +The skill's current guards are kept unchanged: it stops when the branch has no commits or no file changes, it asks for the default branch when it cannot determine one, and it asks which template to conform to when the repository offers several. They are cited here so no behavior in the spec rests on nothing. + +- **Evidence:** `codebase` — `han-github/skills/update-pr-description/SKILL.md` Step 1 (branch-state validation and the default-branch question) and Step 2.3 (the multiple-template question, including the "None" option). +- **Driven by findings:** — (added in synthesis to close an uncited-commitment gap) +- **Referenced in spec:** Primary Flow, Alternate Flows and States, Edge Cases and Failure Modes +- **Amended in review:** [D29](#d29-github-access-is-needed-only-to-publish) adds the guard this entry missed. The skill's `gh` CLI prerequisite check is a hard stop, and it is deliberately not carried forward. + +## Decisions added in review + +These decisions were added during `iterative-plan-review` round R1. Each cites the finding that forced it, recorded in [review-findings.md](review-findings.md). + +### D23: An adversarial pass refutes each claim against its evidence before the gate + +- **Outcome:** After the description is authored and before the gate is assembled, a separate pass reads every claim beside the evidence recorded for it and tries to refute it. It answers one question per claim: does this evidence actually support this claim? A claim it refutes is demoted into the blocking tier and shown to the engineer with the challenge that refuted it. The pass reads only. It may attach a challenge and move a claim between tiers, and it may not touch a single word of the description. +- **Rationale:** [T1](feature-technical-notes.md#t1-claim-provenance-is-captured-at-authoring-time) proves the authoring pass can honestly report the *absence* of evidence but cannot certify its *presence*. That left the measured failure mode wide open, because a fabricated claim usually arrives with a confident pointer at a real hunk rather than with nothing attached, so it was never marked unevidenced and never blocked. Three reviewers found the hole independently. The fix has to come from a pass that did not write the text and is pointed at knocking it down, because that is the only pass with no motive to accept it. The read-only constraint is what keeps [D11](#d11-nothing-rewrites-the-draft-between-authoring-and-the-gate) and T1 standing: a refuting pass allowed to soften a sentence it doubts would make the gate's text a reconstruction again. +- **Evidence:** `provided` — the user directed that an adversarial validator run across all items before the list is presented to the engineer. `codebase` — the hole it closes is recorded as F25 and F26 in [review-findings.md](review-findings.md), each traced to spec text and to D2's own rationale. +- **Rejected alternatives:** + - *Let the authoring pass self-check its citations.* Rejected: it is the same generative act asking itself for a second opinion, which is precisely the reconstruction T1 rules out. + - *Make the refuting pass advisory only.* Rejected: a challenge that changes no tier leaves a refuted claim clearable by the same one-keystroke bulk action, which is the failure being fixed. + - *Let a refuted claim block unconditionally.* Rejected as heavier than the evidence supports. Demotion into the blocking tier already forces an individual decision, and the engineer keeps the final word, which is the principle the whole gate rests on. +- **Linked technical notes:** [T2](feature-technical-notes.md#t2-refutation-is-a-separate-pass-that-reads-the-claims-and-never-touches-the-words) +- **Driven by findings:** F25, F26 +- **Referenced in spec:** Outcome, Primary Flow, The Gate, Open Items + +### D24: The gate is ordered, grouped, counted, and honest about what it does not know + +- **Outcome:** The gate opens by stating how many items it holds and how many of them block. Blocking items come first, grouped by kind. The non-blocking tier follows, ordered weakest-evidence-locus first, with each item showing what the skill pointed at when it wrote the claim. Each tier is named for what it structurally is rather than for what it costs, and the bulk action on the non-blocking tier is worded as an act of authorship. The gate states plainly that it shows whether evidence exists, not whether that evidence supports the claim. No disposition is pre-selected. The size fact and any incomplete-read fact are stated once as a preamble and are not items. +- **Rationale:** [D2](#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for) removed the "supported" badge because it would teach the engineer to skip exactly the rows a fabricated claim hides in, and then re-created the badge structurally: one tier costing N actions beside one tier costing one action says *the machine is sure about the second* more loudly than any label. Removing the word did not remove the vouch. The answer is not to delete the cheap tier, which would tax every clean run and buy fatigue, but to stop the tier from carrying a verdict it has not earned. Naming the tiers honestly, showing the evidence locus, and putting the weakest evidence where attention is freshest gives the engineer orientation without certification, which is D2's actual principle. The counts and the ordering exist because an engineer who cannot see how much gate remains starts optimizing for exit, and a wave-through that arrives as time management is still a wave-through. +- **Evidence:** `provided` — the user chose this resolution over both abolishing the bulk path and documenting the hole as accepted risk. `codebase` — F25, F42, F43, F46 in [review-findings.md](review-findings.md). +- **Rejected alternatives:** + - *Dispose of every claim individually.* Rejected: the interaction cost scales with claim count on every run including clean ones, which is the fatigue path to rubber-stamping. D2's "costs nothing on a clean change" property is worth keeping. + - *Document the hole in Open Items and change nothing.* Rejected: the spec's Outcome would then overclaim what the gate closes. +- **Linked technical notes:** [T2](feature-technical-notes.md#t2-refutation-is-a-separate-pass-that-reads-the-claims-and-never-touches-the-words) +- **Driven by findings:** F25, F42, F43, F46 +- **Referenced in spec:** Primary Flow, The Gate + +### D25: The reading-order guide points, and characterizes nothing + +- **Outcome:** The guide's bullets name files and areas in a suggested reading order. They do not characterize anything as a risk, a tradeoff, or a decision. With that content rule, the guide asserts nothing that can be true or false about the change, and its exemption from the gate under [D9](#d9-the-reading-order-guide-keeps-its-existing-threshold) is true by construction rather than by assertion. +- **Rationale:** D9 and D12 exempted the guide because it "asserts nothing about the change." The skill's own content rule refutes that: it defines the bullets as pointers to "a decision, tradeoff, or risk the reviewer should weight." Naming a risk in a file is a falsifiable claim, and the section fires only past eight to ten significant files, which is where fabrication risk is highest. The exemption and the content rule could not both stand. Narrowing the content is the cheaper repair: it keeps the gate from growing on exactly the changes where it is already largest, and a reading-order guide that points is still a reading-order guide. +- **Evidence:** `codebase` — `han-github/skills/update-pr-description/references/template.md:24` and `han-github/skills/update-pr-description/SKILL.md:110`, both quoted in F27. `provided` — the user chose narrowing over gating the bullets or cutting the section. +- **Rejected alternatives:** + - *Gate the guide's bullets.* Rejected: it grows the gate on large changes, where the item count is already the design's biggest open risk. + - *Drop the section.* Rejected: it costs nothing on the runs where it does not fire, and no evidence says the guide is unwanted. +- **Driven by findings:** F27 +- **Referenced in spec:** Primary Flow, The Gate + +### D26: A template section the change does not reach is one judgment, not many + +- **Outcome:** An absence the description *asserts to the reviewer*, such as a claim that nothing changed for existing callers, keeps blocking individually. A template section the change simply does not reach is different: the engineer is shown the set of unreached sections and vouches for the set in one act, pulling out any individual section that deserves its own decision. +- **Rationale:** Under [D8](#d8-repository-template-conformance-is-carried-forward-and-its-fill-passes-through-the-gate) and [D15](#d15-absence-claims-are-structurally-unprovable-like-intent) as first written, a one-line docs fix against an eight-section corporate template produced roughly eight blocking items before a single substantive claim appeared. The smallest, safest change drew the longest and most ceremonious gate, which is the precise training regime for the wave-through reflex. D15's own rationale names the hazard: "a marker that fires routinely, for a reason that is structural rather than suspicious, is a marker the engineer learns to clear without reading." Conflating "the diff proves nothing changed here" with "this section plainly does not apply to a docs fix" dilutes the signal D15 exists to protect. They are different judgments and they now cost differently. +- **Evidence:** `codebase` — F28 in [review-findings.md](review-findings.md), tracing the item-count blowup through D8's carry-forward of the conformance rules. `provided` — the user chose set-disposal over keeping each note blocking. +- **Driven by findings:** F28 +- **Referenced in spec:** The Gate, Edge Cases and Failure Modes, Open Items + +### D27: The engineer can always leave, and an unverified run does not hand them paste-ready prose + +- **Outcome:** The engineer can abandon the gate at any point. Abandoning yields what a fail-closed run yields, and so does a fail-closed run: the claims delivered as an un-assembled list, each beside its evidence and its standing, with the unevidenced and refuted ones named, and nothing published. An unverified run never produces a description the engineer can paste into GitHub without assembling it themselves. +- **Rationale:** Two holes closed by one rule. First, the only committed exit from a started gate was to complete it, which is a trap in exactly the scenario where the skill performs worst, and the rational response to a trap is to stop entering it. Second, [D13](#d13-the-gate-fails-closed) failed closed to nothing on the no-pull-request path: "nothing is published" is a no-op when there is nothing to publish to, and the engineer received the same paste-ready file a successful run produces. Making the unverified artifact structurally unpasteable is what gives the fail-closed rule teeth where it previously had none, and it is what makes the abandon path safe to offer. An exit the engineer can always take is what makes the block on the way to *publication* legitimate rather than coercive. +- **Evidence:** `codebase` — F34 and F41 in [review-findings.md](review-findings.md). +- **Driven by findings:** F34, F41 +- **Referenced in spec:** The Gate, User Interactions, Alternate Flows and States + +### D28: The skill's files live outside the working tree, and the description it replaces is kept + +- **Outcome:** A file the skill writes lands outside the repository's working tree, so it can never be committed into the change it describes, and a re-run replaces the prior run's file. The description is written to a file whenever the skill does not publish it. Before an existing pull request description is replaced, that description is written to a file too, and the disclosure names it. +- **Rationale:** Three problems, one place. The spec's Coordinations declared every repository file read-only while Primary Flow wrote one, and it never said where; a description file dropped in the working tree is an untracked file an engineer can accidentally commit into the very pull request being described. [D20](#d20-the-verified-description-is-written-to-a-file) wrote the file "whatever happens next," including on a successful publish where the engineer already has the text on GitHub, which is a completeness argument rather than an evidenced need. And publishing destroyed the existing description irreversibly, with disclosure but no recovery, while D20 had already established the mechanism that makes it recoverable: a terminal is a bad place to keep text you might need, and the description about to be deleted is exactly such text. Preserving it is not merging it, which stays out of scope as a fabrication surface. +- **Evidence:** `codebase` — F32, F33, F45 in [review-findings.md](review-findings.md). F21 in [team-findings.md](team-findings.md) established the need the file serves, which is specific to the paths where the engineer moves text by hand. +- **Driven by findings:** F32, F33, F45 +- **Referenced in spec:** Primary Flow, Coordinations, Out of Scope, Deferred (YAGNI) + +### D29: GitHub access is needed only to publish + +- **Outcome:** The skill needs version control and a branch. GitHub access is needed only for the publish step. When the GitHub CLI is missing, or present but unauthenticated, the run degrades to the no-pull-request flow: the gate runs and the file is delivered. The current skill's hard stop on a missing CLI is deliberately not carried forward. +- **Rationale:** The current skill stops dead when the CLI is absent, because a skill whose only deliverable is a GitHub write has nothing to offer without it. That stopped being true. [D6](#d6-the-gate-runs-whether-or-not-a-pull-request-exists) runs the gate with no pull request, and [D28](#d28-the-skills-files-live-outside-the-working-tree-and-the-description-it-replaces-is-kept) makes a file the deliverable, so the skill now does most of its job with no GitHub access at all. Keeping the stop would refuse to do work the design explicitly says is worth doing. [D22](#d22-the-existing-branch-state-and-template-discovery-guards-are-carried-forward) claimed to enumerate the guards so that no behavior rested on nothing, and missed this one entirely. +- **Evidence:** `codebase` — `han-github/skills/update-pr-description/SKILL.md:12-18`, the hard stop, quoted in F29. +- **Driven by findings:** F29 +- **Referenced in spec:** Primary Flow, Alternate Flows and States, Coordinations + +### D30: The unit of direct editing is an item, not the draft + +- **Outcome:** The engineer edits an item: a claim, a section's prose, or the intent sentence. The edited text becomes that item's disposition, verified by authorship. The skill never re-derives the item set from edited prose. +- **Rationale:** [D21](#d21-editing-the-draft-directly-verifies-what-was-edited-and-nothing-else) says "any claim they did not touch is still theirs to dispose of." To know which claims a free-form edit of the whole draft touched, something must read the edited prose and re-derive the claim-to-evidence pairing from it, and that is exactly the reconstruction [T1](feature-technical-notes.md#t1-claim-provenance-is-captured-at-authoring-time) rules out as structurally dishonest. The spec was committing to a rule whose most obvious implementation silently guts the feature. Editing per item keeps the pairing intact, because the item's identity never has to be recovered from the words. +- **Evidence:** `codebase` — F31 in [review-findings.md](review-findings.md), tracing the collision to T1's own text. +- **Linked technical notes:** [T2](feature-technical-notes.md#t2-refutation-is-a-separate-pass-that-reads-the-claims-and-never-touches-the-words) +- **Driven by findings:** F31 +- **Referenced in spec:** The Gate + +### D31: A wholly rejected draft is re-authored and re-gated from the top + +- **Outcome:** When the engineer rejects every claim, the skill offers to author a fresh draft with their corrections as input. That draft is a new authoring pass, and it produces a new gate, entered from the top. It is offered once and it is declinable, and declining takes the abandon path in [D27](#d27-the-engineer-can-always-leave-and-an-unverified-run-does-not-hand-them-paste-ready-prose). +- **Rationale:** The spec offered the re-draft while citing [D17](#d17-the-engineer-may-state-intent-before-the-draft-is-written), which says nothing about re-drafting. A second draft is a fresh set of unverified claims, and nothing said it re-gates. Read literally against [D7](#d7-the-engineers-verdict-is-binding-and-re-rendering-does-not-re-draft) and [D11](#d11-nothing-rewrites-the-draft-between-authoring-and-the-gate), which both forbid generative passes over approved material, the row was a loophole through which an ungated draft could reach the pull request. Giving the behavior its own decision closes it. +- **Evidence:** `codebase` — F36 in [review-findings.md](review-findings.md). +- **Driven by findings:** F36 +- **Referenced in spec:** Edge Cases and Failure Modes, Alternate Flows and States diff --git a/docs/plans/pr-description-rebuild/artifacts/feature-technical-notes.md b/docs/plans/pr-description-rebuild/artifacts/feature-technical-notes.md new file mode 100644 index 00000000..1977338b --- /dev/null +++ b/docs/plans/pr-description-rebuild/artifacts/feature-technical-notes.md @@ -0,0 +1,37 @@ +# Feature Technical Notes: Verified PR Descriptions + +Load-bearing mechanics. A note lives here only because a behavior in [../feature-specification.md](../feature-specification.md) is correct only because of the mechanic, and the mechanic is not discoverable from existing code in this repository. + +## T1: Claim provenance is captured at authoring time + +- **Context:** The gate ([D2](decision-log.md#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for)) commits the skill to showing each claim beside the evidence it was written from, and to marking as unevidenced any claim it could not point at anything for. The gate's entire value rests on that marking being honest. + +- **Technical detail:** The pass that writes the description must emit each assertion already bound to the evidence it rests on, as a single act, and must write the finished readable prose — in the shape the repository's template requires — in that same act ([D11](decision-log.md#d11-nothing-rewrites-the-draft-between-authoring-and-the-gate)). Two mechanics follow, and both are load-bearing. + + **The pairing cannot be reconstructed after the fact.** A second pass that reads finished prose and goes looking for supporting evidence is being asked to find justification for a claim it is motivated to accept. That is the same generative act that produced the hallucinated claim in the first place, so it would happily find plausible-looking support for an invented claim and report the gate as clean. Structurally, the only pass that can honestly say *I could not evidence this one* is the pass that tried to write it and found nothing to write from. + + **No pass that touches the words may run between the pairing and the gate.** This is the non-obvious half, and it is why readability and template conformance are folded in rather than applied afterward. A rewrite pass that preserves every fact still restates every claim in new words. The moment it does, the text shown at the gate is no longer the text whose evidence was recorded, and the pairing is a reconstruction again — arriving through a pass nobody would think to call generative. The rule covers *any* pass that edits the draft, not only passes that invent facts: a readability rewrite, a readability self-check that fixes what it finds, and a structural verification step that fixes what it finds are the same hazard in three costumes ([D11](decision-log.md#d11-nothing-rewrites-the-draft-between-authoring-and-the-gate)). + + Note the asymmetry this establishes, because it drives the gate's display rules: the authoring pass can honestly report the *absence* of evidence, because absence is a fact about what it had in hand. It cannot certify the *presence* of evidence, because that is a self-assessment of its own output. Only the human closes that half, which is why the gate shows claim and evidence side by side and applies no "supported" label ([D2](decision-log.md#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for)). + + The rule constrains what may run *before* the gate. It does not forbid the skill from re-joining surviving assertions into readable prose *after* the engineer has ruled on them, which is a different act on already-verified material and is bounded by its own rule ([D7](decision-log.md#d7-the-engineers-verdict-is-binding-and-re-rendering-does-not-re-draft)): no assertion the engineer did not approve may appear, and the result is shown to them before it is published. + +- **Supports decisions:** [D2](decision-log.md#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for), [D7](decision-log.md#d7-the-engineers-verdict-is-binding-and-re-rendering-does-not-re-draft), [D11](decision-log.md#d11-nothing-rewrites-the-draft-between-authoring-and-the-gate), [D12](decision-log.md#d12-a-claim-is-one-independently-verifiable-assertion-and-the-gate-covers-every-assertion-the-reviewer-will-read) +- **Driven by findings:** F2, F7, F9, F18 +- **Referenced in spec:** Primary Flow + +## T2: Refutation is a separate pass that reads the claims and never touches the words + +- **Context:** [T1](#t1-claim-provenance-is-captured-at-authoring-time) establishes that the authoring pass can honestly report the *absence* of evidence but cannot certify its *presence*, because that is a self-assessment of its own output. That leaves half the problem open, and it is the half the research actually measured: a fabricated claim usually arrives with a confident pointer at a real hunk, not with nothing attached. Such a claim is never marked unevidenced, so under the original design it reached the engineer sorted into the cheap tier. The adversarial pass ([D23](decision-log.md#d23-an-adversarial-pass-refutes-each-claim-against-its-evidence-before-the-gate)) exists to close that half, and it is correct only because of the two constraints below. + +- **Technical detail:** The pass runs after authoring and before the gate is assembled. It receives each claim already bound to the evidence recorded for it, and it answers one question per claim: *does this evidence actually support this claim?* A claim it refutes is demoted into the blocking tier and shown with the challenge that refuted it. Two mechanics are load-bearing. + + **It must be a different pass, and it must be pointed at refutation.** The reason T1 forbids reconstruction is that a pass looking for support for finished prose is motivated to find it. The inverse holds and is why this pass works: a pass whose job is to *refute* a claim it did not write is not motivated to accept it. The independence and the adversarial framing are not stylistic choices, they are the whole reason the pass can say something the authoring pass structurally could not. A pass that merely re-reads its own draft and asks "is this well-evidenced?" reproduces the exact failure T1 rules out, in a new costume. + + **It reads, and it writes nothing into the description.** This is what keeps [D11](decision-log.md#d11-nothing-rewrites-the-draft-between-authoring-and-the-gate) and T1 intact. The refuting pass may attach a challenge to a claim and may move a claim between tiers, and it may do nothing else. The moment it is permitted to fix, soften, or requalify a sentence it doubts, the text at the gate stops being the text whose evidence was recorded, the pairing becomes a reconstruction again, and T1's whole argument collapses through a door nobody was watching. The pass changes the claim's *standing*, never the claim's *words*. + + Note what this does and does not buy. It does not make the gate a verifier: a challenge that fails to refute a false claim leaves that claim in the non-blocking tier, and the human is still the only one who closes that. What it buys is that the engineer's attention is spent on the claims an adversary could not defend, rather than spread flat across every row, and that a claim with a fabricated citation now has something standing between it and the pull request ([D24](decision-log.md#d24-the-gate-is-ordered-grouped-counted-and-honest-about-what-it-does-not-know)). + +- **Supports decisions:** [D2](decision-log.md#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for), [D11](decision-log.md#d11-nothing-rewrites-the-draft-between-authoring-and-the-gate), [D23](decision-log.md#d23-an-adversarial-pass-refutes-each-claim-against-its-evidence-before-the-gate), [D24](decision-log.md#d24-the-gate-is-ordered-grouped-counted-and-honest-about-what-it-does-not-know) +- **Driven by findings:** F25, F26, F31 +- **Referenced in spec:** Primary Flow, The Gate diff --git a/docs/plans/pr-description-rebuild/artifacts/review-findings.md b/docs/plans/pr-description-rebuild/artifacts/review-findings.md new file mode 100644 index 00000000..736827c4 --- /dev/null +++ b/docs/plans/pr-description-rebuild/artifacts/review-findings.md @@ -0,0 +1,281 @@ +# Review Findings: Verified PR Descriptions + +Findings raised by `iterative-plan-review` against [../feature-specification.md](../feature-specification.md). + +Numbering starts at F25. F1–F24 were raised during spec authoring and live in [team-findings.md](team-findings.md); [T1](feature-technical-notes.md) cites F2, F7, F9, and F18 from that file, so this review continues the sequence rather than restarting it. + +## Major findings + +### F25: The bulk-accept path on the non-blocking tier is the badge D2 deleted + +- **Agent:** han-core:junior-developer, han-core:adversarial-validator, han-core:user-experience-designer (raised independently by all three) +- **Category:** contradiction +- **Finding:** The gate blocks only on claims the skill *reports* as unevidenced. A fabricated claim that arrives with a plausible-looking evidence pointer is never marked unevidenced, so it lands in the non-blocking tier and is cleared by a single bulk action. D2 removed the "supported" label because a badge "would teach the engineer to skip exactly the rows a fabricated claim hides in," but the tier structure emits that badge in the only currency the engineer feels, which is effort: one tier costs N actions, the other costs one. +- **Evidence considered:** Spec, `## The Gate`: "Every other claim is shown with the evidence recorded for it, and may be accepted together once the blocking items are settled." D2's own rationale, `decision-log.md`: "Making the careless path cost one keystroke while the careful path costs N is a choice architecture that guarantees the careless path." A25/A28, the evidence D2 rests on, describe AI descriptions asserting functionality *absent from the diff*, which in practice means a plausible tie to a nearby hunk rather than zero evidence, so the measured failure mode lands in the bulk-clearable tier. +- **Resolution:** The bulk path is kept, and made honest. The gate names each tier by what it structurally is rather than by its cost, shows the *locus* of each claim's evidence (what the skill pointed at), orders the non-blocking tier weakest-locus first, and words the bulk action as an act of authorship rather than dismissal. The spec now states plainly that the gate shows whether evidence exists, not whether it supports the claim. Separately, an adversarial pass now runs before the gate and demotes refuted claims into the blocking tier (F26), which is what actually closes the hole. +- **Resolved by:** user input +- **Raised in round:** R1 +- **Changed in plan:** `## The Gate`, `## Primary Flow`, `## Outcome` +- **Changed in tech-notes:** T2 + +### F26: The gate has no mechanism for a citation that is real but does not support its claim + +- **Agent:** han-core:adversarial-validator, han-core:junior-developer +- **Category:** flawed premise +- **Finding:** T1's asymmetry covers the *absence* of evidence and nothing else. The pass that confabulates a claim is equally capable of confabulating a citation for it, in the same generative act. A claim citing a real file at the wrong line, or a hunk tangential to the assertion, is displayed at the gate as an evidenced claim beside plausible-looking evidence. The gate structurally cannot surface it, so the Outcome's promise overclaims what the gate closes. +- **Evidence considered:** `feature-technical-notes.md`: "the only pass that can honestly say *I could not evidence this one* is the pass that tried to write it and found nothing to write from" addresses zero-evidence claims only. The spec's D10 row distinguishes only "a claim the skill can point at nothing for" from everything else; correctness of the pointer is never tested anywhere in the spec, D2, D10, or T1. +- **Resolution:** A new adversarial pass runs over every drafted claim before the gate is assembled. It attempts to refute each claim against the evidence recorded for it, and demotes any claim it refutes into the blocking tier with its challenge shown beside the claim. It reads only: it may never edit the words, so D11 and T1 hold intact. This is the structural answer to the half of the problem the authoring pass provably cannot close, because the refuting pass is not motivated to accept the text. The spec also now states the residual limitation honestly rather than implying the gate closes it. +- **Resolved by:** user input +- **Raised in round:** R1 +- **Changed in plan:** `## Primary Flow`, `## The Gate`, `## Outcome`, `## Open Items` +- **Changed in tech-notes:** T2 + +### F27: The reading-order guide is exempted from the gate on a premise the skill's own content rule refutes + +- **Agent:** han-core:evidence-based-investigator, han-core:junior-developer, han-core:adversarial-validator (raised independently by all three) +- **Category:** contradiction +- **Finding:** D9 and D12 exempt the reading-order guide from the gate because it "asserts nothing about the change." The skill's own content rule defines its bullets as pointers to a decision, tradeoff, or risk. Naming a risk in a file is a falsifiable claim about the change, and the section appears only on large diffs, which D2's rationale identifies as where fabrication risk is highest. +- **Evidence considered:** Spec, `## Primary Flow` step 3: "The guide to what to read first asserts nothing about the change and is not a gate item." Against `han-github/skills/update-pr-description/references/template.md:24`: "{Pointer to a decision, tradeoff, or risk the reviewer should weight, in suggested reading order.}" And `han-github/skills/update-pr-description/SKILL.md:110`: "a 2-4 bullet reading-order guide for a large change, pointing at decisions, tradeoffs, or risks in the order to read them." D12's rejected alternative defends the exemption on the guide's *purpose*, not its actual content. +- **Resolution:** The guide's content rule is narrowed to pure navigation. Its bullets point at files and areas in a suggested reading order and characterize nothing as risky, tradeoff-laden, or decision-bearing. D9's exemption is then true by construction, and the gate does not grow on exactly the changes where it is already largest. +- **Resolved by:** user input +- **Raised in round:** R1 +- **Changed in plan:** `## Primary Flow`, `## The Gate` +- **Changed in tech-notes:** — + +### F28: The repository-template path gives the smallest change the longest gate + +- **Agent:** han-core:adversarial-validator, han-core:user-experience-designer +- **Category:** unhandled edge case +- **Finding:** Under D8 and D15, every template section a change does not reach produces a "does not apply" note, which is an absence claim, which blocks individually with no bulk path. A one-line docs fix against an eight-section corporate template therefore produces roughly eight blocking items before a single substantive claim is reached. D12's bound on gate size is scoped to the lean core and says nothing about the templated branch, where section count is set by the repository rather than by the skill. D15's own rationale names this hazard and then routes the most routine item type into the harder tier. +- **Evidence considered:** Spec, `## Edge Cases and Failure Modes`: "That note is a claim of absence made in the engineer's name, so it blocks at the gate like any other absence claim." Spec, `## Open Items`: "[D12] bounds the gate by scoping it to the lean core, which should keep it small." D15's rationale, `decision-log.md`: "A marker that fires routinely, for a reason that is structural rather than suspicious, is a marker the engineer learns to clear without reading." +- **Resolution:** An absence the description *asserts to the reviewer* is separated from a template section the change simply does not reach. The first keeps blocking individually. The second is one judgment about scope, not N: the engineer is shown the set of unreached sections and vouches for the set as one act, pulling any individual section out of it that deserves its own decision. The Open Items entry that predicts gate size now covers the templated branch explicitly. +- **Resolved by:** user input +- **Raised in round:** R1 +- **Changed in plan:** `## The Gate`, `## Edge Cases and Failure Modes`, `## Open Items` +- **Changed in tech-notes:** — + +### F29: The gh CLI guard is a hard stop in today's skill and the spec neither carries it forward nor drops it + +- **Agent:** han-core:evidence-based-investigator, han-core:junior-developer +- **Category:** silently dropped behavior +- **Finding:** D22 claims to enumerate the guards carried forward "so no behavior in the spec rests on nothing," and omits the one guard that stops the skill dead. Worse, the guard is now wrong: D6 runs the gate whether or not a pull request exists and D20 makes a file the deliverable, so a missing or unauthenticated GitHub CLI no longer prevents the skill from doing its job. +- **Evidence considered:** `han-github/skills/update-pr-description/SKILL.md:12-18`: "**If the gh CLI is not found:** ... **Immediately stop** execution of this skill, as it cannot be executed." Against `decision-log.md` D22, which lists only the branch-state and template-discovery guards. A grep for "gh cli" and "prerequisite" across the spec and decision log returns zero matches. +- **Resolution:** The spec now states that the skill needs version control and the branch, and that GitHub access is needed only to publish. Its absence, whether the CLI is missing or present but unauthenticated, degrades the run to the no-pull-request flow: the gate runs and the file is delivered. The hard stop is deliberately not carried forward, and the spec says so. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Primary Flow`, `## Alternate Flows and States`, `## Coordinations` +- **Changed in tech-notes:** — + +### F30: The spec recognizes two template states where the conformance rules it carries forward define three + +- **Agent:** han-core:junior-developer, han-core:evidence-based-investigator +- **Category:** contradiction +- **Finding:** The Outcome says "Where the repository does have a template, the template still dictates the shape." The conformance rules D8 carries forward wholesale define a replace-scaffold template, where a template file exists and the lean core wins anyway. The spec's sentence denies a state its own carried-forward rules create, and that state decides where the feedback line goes and whether "not applicable" notes exist at all, both of which the gate depends on. +- **Evidence considered:** `han-github/skills/update-pr-description/references/template-conformance.md`, section 2: "**Replace-scaffold.** If the template (or a comment in it) instructs the author to replace its content with a written PR description ... Discard the scaffold and produce the default structure instead." The word "scaffold" appears nowhere in the spec or the decision log. +- **Resolution:** The spec now states that the template is discovered and its own instructions are honored, which may mean the lean core is used even when a template file exists. In that case the feedback line and the gate behave exactly as they do when no template exists. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Outcome`, `## Primary Flow`, `## Edge Cases and Failure Modes` +- **Changed in tech-notes:** — + +### F31: Direct editing has no committed interaction, and reconciling an edit needs the reconstruction T1 forbids + +- **Agent:** han-core:user-experience-designer, han-core:junior-developer +- **Category:** missing interaction commitment +- **Finding:** D21 says "any claim they did not touch is still theirs to dispose of." To know which claims an engineer's free-form edit touched, something must read the edited prose and re-derive the claim-to-evidence pairing from it. That is precisely the second pass reading finished text and going looking for support that T1 rules out as structurally dishonest. The spec commits to a rule whose most obvious implementation silently guts the feature. +- **Evidence considered:** Spec, `## The Gate`: "The engineer can edit the draft directly, and text they wrote is text they verified." Against `feature-technical-notes.md` T1: "A second pass that reads finished prose and goes looking for supporting evidence is being asked to find justification for a claim it is motivated to accept." +- **Resolution:** The unit of direct editing is an *item*, not the draft. The engineer edits a claim, a section's prose, or the intent sentence, and the edited text becomes that item's disposition, verified by authorship. The skill never re-derives the item set from edited prose, and the spec says so. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## The Gate` +- **Changed in tech-notes:** T2 + +### F32: The file deliverable has no stated location, and Coordinations declares every disk write read-only + +- **Agent:** han-core:junior-developer +- **Category:** contradiction +- **Finding:** Primary Flow step 6 writes the description to a file. Coordinations lists the repository's own files as read-only and names GitHub as the only write. The spec's one write to disk is a write it does not list. Nothing says where the file lands, and a description file dropped in the working tree is an untracked file an engineer can accidentally commit into the very pull request being described. D19 says nothing about what a re-run does to the prior run's file. +- **Evidence considered:** Spec, `## Coordinations`: "**The repository's own files.** Read-only." Against `## Primary Flow` step 6: "The description is written to a file the engineer can take away." F21 justified D20 partly on the grounds that "the skill already ships a temp-file script," but `han-github/skills/update-pr-description/scripts/create-review-tempfile.sh` is never referenced by the current SKILL.md, so it evidences a capability, not a chosen location. +- **Resolution:** Coordinations gains a write coordination. The file lands outside the repository working tree, so it can never be committed into the change it describes, and a re-run replaces the prior run's file. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Coordinations`, `## Primary Flow`, `## Alternate Flows and States` +- **Changed in tech-notes:** — + +### F33: The description is written to a file on every run, including runs where the engineer never needs it + +- **Agent:** han-core:junior-developer +- **Category:** YAGNI candidate +- **Finding:** D20 commits to the file "whatever happens next," including a run that ends in a successful publish, where the engineer already has the description on GitHub. The evidence behind D20 (F21) is that "a long markdown block in terminal scrollback is hostile to copying," which is a need specific to the paths where the engineer must move the text by hand. The universality is a completeness argument, and it is the version that litters a working tree on every successful run. +- **Evidence considered:** Spec, `## Primary Flow` step 6: "The description is written to a file the engineer can take away, whatever happens next ([D20])." F21 in `team-findings.md`. +- **Resolution:** Replaced with the simpler version that satisfies the same evidence: the description is written to a file whenever the skill does not publish it, which is the no-pull-request path, the declined path, and the fail-closed unverified path. *Reopening trigger:* an engineer asks for the file after a successful publish. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Primary Flow`, `## Deferred (YAGNI)` +- **Changed in tech-notes:** — + +### F34: "Delivered marked unverified" is undefined, and fails closed to nothing on the no-pull-request path + +- **Agent:** han-core:junior-developer +- **Category:** undefined term +- **Finding:** D13's fail-closed rule says the description is "delivered marked unverified, and nothing is published." When no pull request exists, "nothing is published" is a no-op, and the engineer receives a paste-ready file exactly as they would from a successful run. The fail-closed rule has no effect precisely where D6 insists the gate still matters. The spec never says what the mark means: inside the description body it would be pasted into GitHub verbatim, and outside it, it protects nothing. +- **Evidence considered:** Spec, `## The Gate`: "The description is delivered marked unverified, and nothing is published." Against `## Alternate Flows and States`: "**No pull request exists yet.** The gate still runs, and the verified description is still written to a file." +- **Resolution:** An unverified run does not deliver a paste-ready description. It delivers the claims as an un-assembled list, each beside its evidence and its disposition, with the unevidenced and refuted ones named. The engineer can still act on it, but they cannot paste it into GitHub without doing the assembly themselves, which is the point. The fail-closed rule now bites on the no-pull-request path. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## The Gate`, `## Alternate Flows and States` +- **Changed in tech-notes:** — + +### F35: A deliberately blank feedback ask and the empty answer the gate must fail closed on are the same signal + +- **Agent:** han-core:junior-developer +- **Category:** contradiction +- **Finding:** D3 says leaving the feedback ask blank omits the section. D13 says an answer the skill cannot obtain from a human is not approval and the run fails closed. A blank feedback field and an unanswered question are indistinguishable, and the spec prescribes opposite behavior for the same input. +- **Evidence considered:** Spec, `## The Gate`: "leaving the feedback ask blank omits it ([D3])" and, two paragraphs later, "If the skill cannot obtain an answer from a human, that is not approval." The failure mode D13 rests on is documented in `han-plugin-builder/skills/guidance/references/skill-building-guidance/allowed-tools-AskUserQuestion.md`: "The tool returns immediately with empty answers. The user never sees the question." +- **Resolution:** Silence is never a valid answer to a blocking item. Skipping the feedback ask is a positive choice the engineer selects, distinct from not answering at all. The fail-closed rule then holds without exception: every blocking item requires a positive act, and an empty response set is always a failure. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## The Gate` +- **Changed in tech-notes:** — + +### F36: The total-rejection re-draft is an ungated generative pass, and it cites a decision that does not contain it + +- **Agent:** han-core:junior-developer +- **Category:** missing prerequisite +- **Finding:** The edge-case row for a wholly rejected draft says the skill "offers to draft again with their corrections as input ([D17])." D17 says only that the engineer may state intent before the draft is written; it contains no re-draft behavior. Meanwhile D7 forbids re-drafting over the engineer's decisions and D11 forbids any pass that touches the words before the gate. A second draft is a fresh set of unverified claims, and the spec never says it re-gates. +- **Evidence considered:** Spec, `## Edge Cases and Failure Modes`, the "engineer rejects every claim" row. Against D17's outcome in `decision-log.md`: "The engineer can state why they made the change when they invoke the skill. The draft is then written against that intent rather than guessing at it." +- **Resolution:** The spec now says explicitly that the re-draft is a fresh authoring pass that produces a fresh gate, entered from the top, with the engineer's corrections as input. It gets its own decision anchor rather than borrowing D17's, so it cannot be read as "draft again and publish." +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Edge Cases and Failure Modes`, `## Alternate Flows and States` +- **Changed in tech-notes:** — + +### F37: The re-rendered description is shown to the engineer but is not a decision they can act on + +- **Agent:** han-core:junior-developer, han-core:user-experience-designer +- **Category:** missing interaction commitment +- **Finding:** Primary Flow step 5 says "the engineer sees the re-rendered text before it goes anywhere," and D7 calls that the safety net that reconciles re-rendering with D11. But `## User Interactions` lists four interactions and this is not among them, and on the no-pull-request path there is no publish question either. The re-rendered text is therefore displayed and then written to a file with no human decision point after it. "Seen" and "approved" are being used interchangeably. +- **Evidence considered:** Spec, `## Primary Flow` step 5, against `## User Interactions`, which lists only branch, template, gate, and publish. +- **Resolution:** The re-render is named as an interaction in its own right, on both branches. The engineer accepts it, edits it, or returns to the gate. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## User Interactions`, `## Primary Flow` +- **Changed in tech-notes:** — + +### F38: The spec claims the gate does not ask the engineer to re-derive the diff, and then asks exactly that + +- **Agent:** han-core:junior-developer +- **Category:** contradiction +- **Finding:** Actors and Triggers rests the whole design on an assumption stated at its weakest point (the engineer knows their own intent) and silently extends it to its strongest (the engineer can judge whether a hunk supports a claim). Judging a claim against its evidence *is* reading the diff. In the AI-assisted workflow this suite exists to serve, the engineer may not have written or read that code either, which is precisely the population the motivating finding measured. +- **Evidence considered:** Spec, `## Actors and Triggers`: "it does not ask them to re-derive the diff, only to decide what the description is allowed to assert." Against `## The Gate`: "It shows the claim and the evidence side by side and lets the engineer judge." +- **Resolution:** The spec now states the real prerequisite: the gate requires an engineer willing to read the evidence shown beside each claim. The false reassurance is removed. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Actors and Triggers` +- **Changed in tech-notes:** — + +### F39: The incomplete-read disclosure assumes a self-knowledge the skill may not have + +- **Agent:** han-core:junior-developer +- **Category:** unstated assumption +- **Finding:** The spec commits the skill to saying "which parts it did not read." A skill that silently lost the tail of an oversized diff has no way to report the tail it lost. D18's evidence establishes that the hazard exists, not that the skill can detect it, and the spec closes its self-declared "one silent failure surface" with a disclosure that assumes the thing it needs to prove. +- **Evidence considered:** Spec, `## Edge Cases and Failure Modes`: "The skill says at the gate which parts it did not read." D18's evidence in `decision-log.md` establishes only that the current skill "injects the full diff into its context unconditionally ... with no size guard and no truncation signal." +- **Resolution:** The spec now states the behavior that *produces* the knowledge rather than assuming it: the skill establishes the full set of changed files before it drafts, and the disclosure is the difference between that set and the files it actually read. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Primary Flow`, `## Edge Cases and Failure Modes` +- **Changed in tech-notes:** — + +### F40: The gate carries a size fact the spec never defines + +- **Agent:** han-core:junior-developer +- **Category:** undefined term +- **Finding:** The gate carries "how large the change is," and the spec never says what large means, what it counts, or when it fires. The unit, threshold, and exclusions live only in D4. The spec's own stated principle is that "the gate is the feature, so its rules are stated here rather than left to be inferred," and this rule is not stated here. Without it an implementer fires the warning on a lockfile bump, which is the failure F6 was raised to prevent. +- **Evidence considered:** Spec, `## Primary Flow` step 4. D4's outcome in `decision-log.md` carries the definition: "added and deleted lines in significant files," excluding "lockfiles, generated code, and vendored dependencies." +- **Resolution:** The size rule is stated in the spec at behavioral resolution: lines added and deleted in code files, with lockfiles, generated code, vendored dependencies, documentation, and configuration excluded. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Primary Flow`, `## Edge Cases and Failure Modes` +- **Changed in tech-notes:** — + +### F41: There is no way out of the gate except through it + +- **Agent:** han-core:user-experience-designer +- **Category:** usability failure +- **Finding:** The gate "does not finish while any blocking item is undecided." Rejecting everything offers a re-draft, which re-gates from scratch. Nothing says what happens when the engineer decides mid-gate that this run was a mistake and they will write the description by hand. The only committed exit from a started gate is to complete it, and that is a trap in exactly the scenario where the skill has performed worst: a bad draft against a large diff. The rational response to a trap is to stop entering it, which retires the feature. +- **Evidence considered:** Spec, `## Primary Flow` step 4 and `## Edge Cases and Failure Modes`. The correct artifact already exists in D13, which delivers an unverified description and publishes nothing; the spec simply never lets the engineer choose it. +- **Resolution:** The engineer can abandon the gate at any point, and abandoning yields exactly what a fail-closed run yields: the claims delivered un-assembled and marked unverified, nothing published. An exit the engineer can always take is what makes the block on the way to *publication* legitimate. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## The Gate`, `## User Interactions`, `## Alternate Flows and States` +- **Changed in tech-notes:** — + +### F42: Dropping a claim is the cheapest disposition and it shortens the rest of the gate + +- **Agent:** han-core:user-experience-designer +- **Category:** dark pattern / effort asymmetry +- **Finding:** Correcting a claim costs typing prose. Keeping it costs reading, judging, and putting your name on it. Dropping it costs one keystroke and makes the remaining gate shorter. The lazy path is not merely cheap, it is actively rewarded with less work, and every claim the engineer drops was by construction a claim they were entitled to drop. What arrives on GitHub is a description that says less than the change did, and the likeliest casualty is the absence claim D15 itself calls "among the most consequential things a description can get wrong." +- **Evidence considered:** Spec, `## The Gate`: "The engineer keeps it, corrects it, or drops it." F1 in `team-findings.md` killed the one-keystroke *accept* and left the one-keystroke *drop* untouched. +- **Resolution:** No disposition is pre-selected or ordered first as a default. Before publishing, the engineer is shown what the description no longer says: the claims they dropped, in one short list. Hollowing out a description becomes a visible act rather than a silent by-product of taking the cheap exit at each step. This reuses the re-render moment the spec already commits to. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## The Gate`, `## Primary Flow` +- **Changed in tech-notes:** — + +### F43: The gate commits to no order, no grouping, and no count + +- **Agent:** han-core:user-experience-designer +- **Category:** missing interaction commitment +- **Finding:** In a scrolling transcript there is no scrollbar and no progress indicator. An engineer who has answered four items and cannot tell whether four or twenty-four remain stops answering carefully and starts optimizing for exit, which is the wave-through arriving as a time-management decision rather than a careless one. Order matters equally: if a routine "this section does not apply" is item one and a fabricated behavior claim is item eleven, attention has already been spent on the item that deserved none. +- **Evidence considered:** Spec, `## Primary Flow` step 4, which says items exist and that blocking ones must be disposed of, and stops. +- **Resolution:** The gate states how many items it holds and how many block, before the first question. Blocking items come first, grouped by kind, because the kinds ask for different judgments. The non-blocking tier is presented after the blocking tier is settled, ordered weakest-evidence-locus first. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Primary Flow`, `## The Gate` +- **Changed in tech-notes:** — + +### F44: The skill paraphrases an intent the engineer already stated, then blocks on confirming its own paraphrase + +- **Agent:** han-core:user-experience-designer +- **Category:** YAGNI candidate +- **Finding:** D21's rule is that text the engineer wrote is text they verified. An engineer who invoked the skill with their intent has written text. The skill re-renders it "in its own words" and then charges them a blocking item to confirm the rewrite. On the run where the engineer did the right thing, the design taxes them for it. +- **Evidence considered:** Spec, `## The Gate`: "The engineer confirms the intent sentence even when they stated their intent up front. The draft renders that intent in its own words." Against `## The Gate`: "text they wrote is text they verified, so it needs no further evidence." +- **Resolution:** Kept, with the evidence cited. D17 stands as written. The summary sentence has a required shape the engineer's raw statement will not generally satisfy, and the description's voice is uniform by design, so the skill renders the intent and the engineer confirms the rendering. The cost is one blocking item on runs where intent was supplied, and it buys the guarantee that the words the reviewer reads are words a human approved. +- **Resolved by:** user input +- **Raised in round:** R1 +- **Changed in plan:** — (no change; decision affirmed) +- **Changed in tech-notes:** — + +### F45: Publishing destroys the existing description irreversibly, and the mechanism that would make it recoverable is already in the spec + +- **Agent:** han-core:user-experience-designer, han-core:adversarial-validator +- **Category:** usability failure +- **Finding:** The engineer approves claims at the gate, answers one more question, and their hand-written description is gone from a system of record. D14's disclosure tells them what they are about to lose; it gives them no way to get it back if they say yes and regret it, or discover an hour later that the deleted text mattered. Meanwhile D20 already commits to writing text to a file *because a terminal is a bad place to keep text you might need*, and the description about to be destroyed is exactly such text. +- **Evidence considered:** Spec, `## Primary Flow` step 6 and `## Out of Scope`, which forbids *merging* the old description (a fabrication surface) in language that reads as if it also forbids preserving it. The spec's own Deferred section admits the common version: "a re-run deletes a link the engineer added by hand and does not put one back." +- **Resolution:** The description being replaced is written to a file before the replacement happens, and the disclosure names that file. An irreversible act becomes a recoverable one at the cost of one write, using a mechanism the spec already establishes. Out of Scope now distinguishes preserving the prior description as an undo artifact from merging it into the new one. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Primary Flow`, `## Out of Scope`, `## Coordinations`, `## Deferred (YAGNI)` +- **Changed in tech-notes:** — + +### F46: The incomplete-read disclosure dies at the terminal, and two non-actionable facts sit inside the one place the design cannot afford noise + +- **Agent:** han-core:user-experience-designer +- **Category:** missing interaction commitment +- **Finding:** D18 argues that an engineer who verifies every item on the screen would wrongly believe they verified the change. Run the argument one level up: a reviewer who reads a complete-looking description of a partially-read diff wrongly believes they are reading a description of the change. The disclosure never leaves the terminal. Separately, the gate now carries two facts the engineer can do nothing with, sitting alongside N items that demand action, and two ignorable things beside N actionable things is how a human learns that the gate contains ignorable things. +- **Evidence considered:** Spec, `## Primary Flow` step 4 and the oversized-diff row of `## Edge Cases and Failure Modes`. D4's outcome: the size fact "warns; it never blocks." +- **Resolution:** The size fact and the incomplete-read fact are stated once as a preamble before the item list, and are not items. An incomplete read carries a disposition: the description carries a short disclosure to the reviewer that part of the change was not read. The engineer may remove it; the skill does not remove it on their behalf. +- **Resolved by:** evidence +- **Raised in round:** R1 +- **Changed in plan:** `## Primary Flow`, `## The Gate`, `## Edge Cases and Failure Modes` +- **Changed in tech-notes:** — + +## Minor edits + +- F47: A re-run re-gates the intent sentence and the feedback ask, which are stated by the engineer and do not go stale with the diff, so D19's staleness rationale does not reach them — spec now says re-runs re-gate everything deliberately, because the description they attach to has changed — han-core:adversarial-validator — `## Alternate Flows and States` +- F48: The note that replacement destroys a hand-added issue link is scoped to links, when the same path destroys any hand-added content including the engineer's own corrections from a prior run — han-core:adversarial-validator — `## Deferred (YAGNI)` +- F49: The Outcome says the skill "produces a pull request description on GitHub," which two of its own paths contradict — han-core:junior-developer — `## Outcome` +- F50: The three dispositions (keep, correct, drop) are written for prose and do not map onto checklist boxes or "not applicable" notes, and dropping a checklist item is forbidden by the conformance rules the spec carries forward — han-core:junior-developer — `## The Gate` +- F51: D11 removes the verification step without itemizing the structural checks that go with it (nested code fences, leftover template placeholders, the "Generated with Claude Code" strip, verbatim checklist reproduction) — han-core:evidence-based-investigator — `## Primary Flow` +- F52: Coordinations assumes a GitHub read (fetching the existing description body) that today's skill does not perform, so it is new read behavior rather than carried-forward behavior — han-core:evidence-based-investigator — `## Coordinations` +- F53: The spec never commits to showing the engineer the drafted description as prose before decomposing it into gate items, so they judge atomized assertions they have never seen assembled — han-core:user-experience-designer — `## Primary Flow` diff --git a/docs/plans/pr-description-rebuild/artifacts/review-iteration-history.md b/docs/plans/pr-description-rebuild/artifacts/review-iteration-history.md new file mode 100644 index 00000000..c0d6a9b4 --- /dev/null +++ b/docs/plans/pr-description-rebuild/artifacts/review-iteration-history.md @@ -0,0 +1,48 @@ +# Review Iteration History: Verified PR Descriptions + +Rounds run by `iterative-plan-review` against [../feature-specification.md](../feature-specification.md). Findings are recorded in [review-findings.md](review-findings.md). + +## R1 + +- **Mode:** team +- **Spec-aware mode:** engaged (the file is named `feature-specification.md` and carries the canonical feature-spec headings) +- **Size:** medium (one skill's surface plus its docs, with a human-in-the-loop gate as a new cross-cutting concern). Round cap: 2. +- **Specialists engaged:** `han-core:junior-developer`, `han-core:adversarial-validator`, `han-core:evidence-based-investigator`, `han-core:user-experience-designer` +- **Findings raised:** F25, F26, F27, F28, F29, F30, F31, F32, F33, F34, F35, F36, F37, F38, F39, F40, F41, F42, F43, F44, F45, F46 (major); F47, F48, F49, F50, F51, F52, F53 (minor) + +### What the round found + +Three of the four agents independently landed on the same structural hole, which is the strongest signal the round produced. The gate blocks only on claims the skill *reports* as unevidenced, and a fabricated claim typically arrives with a confident pointer at a real hunk rather than with nothing attached. Such a claim was never marked unevidenced, so it landed in the tier the engineer could clear with one keystroke. The bulk-accept affordance had become the "supported" badge that D2 deleted, restated in the only currency the engineer actually feels, which is effort (F25, F26). + +The evidence-based investigator verified the spec's "carried forward" claims against `han-github/skills/update-pr-description/` and refuted three of them. The `gh` CLI hard stop is a real guard that D22 never mentions (F29). The conformance rules define a replace-scaffold state the spec's two-state model denies (F30). And the reading-order guide's exemption from the gate rests on a premise the skill's own content rule contradicts, since its bullets are defined as pointers to "a decision, tradeoff, or risk" (F27). It cleared seven other claims, including D3's description of the lean core, D9's threshold, D11's enumeration of the rewrite passes it removes, and the Open Items claim about `AskUserQuestion` under a parent skill, which it verified word for word against the guidance file. + +The UX review found that the gate's failure mode is not too little rigor but too much of it in the wrong places: the templated path gave the *smallest* change the *longest* gate (F28), dropping a claim was the cheapest disposition and made the remaining gate shorter (F42), and there was no way out of a started gate except through it (F41). + +### Questions put to the user + +Four findings could not be resolved from evidence because each overturned or refined a recorded decision: + +1. **The bulk-accept hole (F25, F26).** Chose to keep the bulk path and make it honest, and added an adversarial pass that refutes each claim against its evidence before the gate and demotes what it refutes. The pass reads only and may never touch the words, which is what keeps D11 and T1 intact. +2. **The reading-order guide (F27).** Chose to narrow its content to pure navigation, making D9's exemption true by construction rather than by assertion. +3. **The paraphrased intent (F44).** Chose to keep D17 as written. The YAGNI candidate is recorded as kept with the evidence cited. +4. **The template N/A blowup (F28).** Chose set-level disposal for sections the change does not reach. + +### Changed in plan + +`## Outcome`, `## Actors and Triggers`, `## Primary Flow` (rewritten from six steps to eight; the file-set pass and the adversarial pass are new), `## The Gate` (substantially rewritten), `## Alternate Flows and States`, `## Edge Cases and Failure Modes`, `## User Interactions`, `## Coordinations`, `## Out of Scope`, `## Deferred (YAGNI)`, `## Open Items`, `## Summary`, `## Review History` (added) + +### Changed in tech-notes + +T2 added: refutation is a separate pass that reads the claims and never touches the words. + +### Changed in decision log + +D23 through D31 added. D22 annotated with the guard it missed. + +### Stability assessment + +Not stable. The round produced 22 major findings, well past the stop rule's threshold of two or fewer with zero major. The spec absorbed a new pass, a restructured gate, an escape hatch, and nine new decisions, and none of that has been reviewed by anyone. + +### Next step + +Run R2 against the rewritten spec. The round should concentrate on what R1 introduced rather than re-reviewing what it fixed: whether the adversarial pass creates a contradiction with T1 or D11 that its own note failed to anticipate, whether the reordered and re-tiered gate is still internally consistent, and whether the escape hatch and the un-assembled claim list interact cleanly with the fail-closed rule. R2 is the last round under the medium cap. diff --git a/docs/plans/pr-description-rebuild/artifacts/team-findings.md b/docs/plans/pr-description-rebuild/artifacts/team-findings.md new file mode 100644 index 00000000..8b2324aa --- /dev/null +++ b/docs/plans/pr-description-rebuild/artifacts/team-findings.md @@ -0,0 +1,213 @@ +# Team Findings: Verified PR Descriptions + +Findings from the review team on the first draft of [../feature-specification.md](../feature-specification.md), and how each was resolved. + +- **Feature size:** Medium +- **Team:** `han-core:junior-developer` (generalist stress-test), `han-core:user-experience-designer` (the gate as an interaction model), `han-core:edge-case-explorer` (boundaries of claim-to-evidence pairing) + +**Headline:** all three reviewers independently reached the same verdict on the first draft — the gate could be defeated with a single keystroke, and therefore reproduced the exact failure mode it was built to prevent. Every finding below is either that defect, a consequence of it, or a hole the reviewers found while pulling on it. Sixteen of the eighteen major findings were resolved from evidence and two went to the user; separately, two reviewer findings were closed without change, with reasons recorded. A synthesis reconciliation pass afterward found four further holes and is recorded at the end. + +## Major findings + +### F1: The gate could be waved through in one action + +- **Raised by:** `junior-developer` (JD-002), `user-experience-designer` (UX-001, UX-002), independently +- **Finding:** The first draft's `## User Interactions` asserted the gate "is not a formality the engineer can wave through," and then, two sentences earlier, offered "confirm the list as it stands." Confirming cost one action regardless of claim count; scrutiny cost N. The effort ratio between the careless and careful paths widened without bound exactly as the diff grew, which is where the fabrication risk is highest. Structurally identical to the yes/no gate being replaced, only longer. The spec's own rationale quoted the research finding it was violating. +- **Resolved by:** evidence +- **Resolution:** The gate now blocks. Unevidenced claims, absence claims, and the intent statement require individual disposition with no bulk path; everything else may be accepted together once those are settled. Costs nothing on a clean change, cannot be skipped on a dirty one. +- **Affected decisions:** D2 +- **Affected tech-notes:** — +- **Changed in spec:** Outcome, The Gate, User Interactions + +### F2: The "supported" label manufactures automation bias + +- **Raised by:** `user-experience-designer` (UX-003) +- **Finding:** T1 argues only the authoring pass can honestly say *I could not evidence this*. It does not establish the same pass can honestly say *this one is evidenced* — that is the identical self-assessment T1 rejects. Labelling a claim "supported" tells the engineer which rows to skip, and a fabricated claim wearing a plausible-looking hunk is precisely what hides in the skipped rows. The reviewer's conclusion: this could make the gate *worse* than the yes/no gate, which at least did not pretend to have checked. +- **Resolved by:** evidence +- **Resolution:** The affirmative label is gone. The gate shows claim and evidence side by side and lets the engineer render the verdict. Only the negative marker survives, which is the only one T1 authorizes. Strictly simpler, and it satisfies the same evidence. +- **Affected decisions:** D2 +- **Affected tech-notes:** T1 +- **Changed in spec:** The Gate + +### F3: The gate failed open on an unanswered question + +- **Raised by:** `user-experience-designer` (UX-004) +- **Finding:** The spec never said what happens when the gate gets no answer. This is not hypothetical: the repository's own guidance documents that the question mechanism can return empty answers with the user never seeing the question, and that this propagates to child skills. A gate whose entire value is that a human answered it would have published unverified claims to a public pull request, under the engineer's name, with zero human error involved. +- **Resolved by:** evidence +- **Resolution:** The gate fails closed. No answer is never approval; the description is delivered marked unverified and nothing is published. The synthesis pass narrowed the evidence claim (the failure occurs when the skill or a parent skill auto-approves the question tool, not unconditionally) and recorded the cost of failing closed as an open item. +- **Affected decisions:** D13 +- **Affected tech-notes:** — +- **Changed in spec:** The Gate, Open Items + +### F4: D3's deferral rested on a cost D2 had already paid + +- **Raised by:** `user-experience-designer` (UX-015) +- **Finding:** The feedback-type section was deferred because it "adds a question to every run." But the gate already stops on every run and already takes free text from the engineer. The deferral's own reopening trigger was satisfied at spec time, by construction. The item is the largest measured effect on merge odds in the entire research base. +- **Resolved by:** user input +- **Resolution:** The user chose to fold the feedback ask into the gate as one field beside the intent confirmation. It rides on an interaction already being paid for. Left blank, the section is omitted. The synthesis pass added where the line goes when a repository template is in use, and recorded the weakness of its single-preprint evidence base as an open item. +- **Affected decisions:** D3 +- **Affected tech-notes:** — +- **Changed in spec:** Outcome, The Gate, Deferred (YAGNI), Open Items + +### F5: The size warning was placed where nobody would read it + +- **Raised by:** `user-experience-designer` (UX-006) +- **Finding:** A non-blocking, admittedly-unactionable message, fired at step two of seven, immediately before the longest silent stretch of the run, then buried under everything after it. The most corroborated finding in the research base, delivered in the form guaranteed to be ignored. Worse: it trains the engineer to skim the skill's non-interactive output, which is the habit that later lets them skim the gate. +- **Resolved by:** evidence +- **Resolution:** The size fact moves to the gate, where the engineer is already stopped and already reading. Same words, same non-blocking intent, placed where attention is. +- **Affected decisions:** D4 +- **Affected tech-notes:** — +- **Changed in spec:** Primary Flow, User Interactions + +### F6: The size threshold had no unit and no exclusions + +- **Raised by:** `junior-developer` (JD-010) +- **Finding:** "More than roughly four hundred lines" never said lines of what, and excluded nothing. A dependency bump with a three-thousand-line lockfile diff would have triggered a lecture about splitting the change. A warning that fires on changes nobody can split is a warning engineers train themselves to ignore — and the research itself names irreducibly-large changes as the known gap in this lever. The skill already has a "significant files" definition and was not reusing it. +- **Resolved by:** evidence +- **Resolution:** Size is added and deleted lines in significant files, carrying the skill's existing definition (code files count; documentation and configuration do not) and extending it to name lockfiles, generated code, and vendored dependencies. The synthesis pass corrected D4's claim that this was a straight reuse: the existing definition does not name those three, so the decision is an extension and now says so. +- **Affected decisions:** D4 +- **Affected tech-notes:** — +- **Changed in spec:** Primary Flow + +### F7: "Re-rendered from what survives" was undefined and contradicted its own decision + +- **Raised by:** `junior-developer` (JD-005), `edge-case-explorer` (EC1, EC4) +- **Finding:** D7 forbade re-drafting, but deleting one claim from a prose paragraph leaves dangling connectives and orphaned fragments. Purely mechanical deletion produces broken English; anything that fixes the English is a re-draft. The spec picked neither. +- **Resolved by:** evidence +- **Resolution:** The line is drawn at assertion, not wording. Re-rendering may rearrange and re-join surviving assertions into readable prose; it may not introduce an assertion the engineer did not approve. The test: could a reader of the re-rendered description learn a fact the engineer never approved? If yes, it is a re-draft. The synthesis pass added that the engineer sees the re-rendered text before publication, which is what makes this rule consistent with D11's objection to any post-approval rewrite. +- **Affected decisions:** D7 +- **Affected tech-notes:** T1 +- **Changed in spec:** Primary Flow, Alternate Flows and States + +### F8: Evidence was defined as the diff, while the spec granted repository reads + +- **Raised by:** `junior-developer` (JD-006), `edge-case-explorer` (EC2), independently +- **Finding:** Coordinations let the drafting pass read source files to understand what the diff does not explain. The gate rule recognized only the diff. So a true, well-evidenced claim resting on an unchanged file ("the flag defaults to off") would be marked unevidenced, identically to a hallucination. Both reviewers reached the same conclusion about why this matters: false positives on the unevidenced marker are more corrosive than false negatives, because an engineer who finds the marker usually means nothing learns to clear it reflexively. That is the wave-through habit arriving through the back door. +- **Resolved by:** evidence +- **Resolution:** Evidence is the diff *or* a cited repository file. Only a claim the skill can point at nothing for is marked unevidenced. A third "repository context" category was considered and rejected under the simpler-version test: it adds a taxonomy without adding a decision. +- **Affected decisions:** D10 +- **Affected tech-notes:** — +- **Changed in spec:** Edge Cases and Failure Modes + +### F9: The readability rewrite pass would have silently gutted the gate + +- **Raised by:** `junior-developer` (JD-004) +- **Finding:** The current skill dispatches a readability editor *after* authoring. The spec neither kept nor dropped it. If kept, it runs between the pass that bound each claim to its evidence and the gate that displays the pairing — and a rewrite that "preserves every fact" still restates every claim in new words, so the text at the gate is no longer the text whose provenance was recorded. That is exactly the reconstruction T1 rules out, arriving through a pass nobody thought of as generative. The likely implementer guess (keep it, it's a house convention) would have quietly destroyed the feature. +- **Resolved by:** user input +- **Resolution:** The user chose to fold readability into the authoring pass. One pass produces the finished prose and records each claim's evidence in the same act. The standard is kept; the second dispatch is gone. The synthesis pass widened the rule to every post-authoring rewrite path, not only the readability editor (see S2). +- **Affected decisions:** D11, D5 +- **Affected tech-notes:** T1 +- **Changed in spec:** Primary Flow + +### F10: "Claim" was never defined, and the gate's entire weight rests on it + +- **Raised by:** `junior-developer` (JD-001), `user-experience-designer` (UX-005), `edge-case-explorer` (EC1) — all three, independently +- **Finding:** Sentence, clause, or assertion? The spec never said. An implementer could satisfy it with three coarse claims each evidenced by the whole diff (a gate that verifies nothing) or twenty-five atomic ones (a gate nobody reads), and neither could be called wrong. The skill's own summary template makes it concrete: `This PR , so that ` fuses an evidenced claim and an unprovable intent in one sentence. +- **Resolved by:** evidence +- **Resolution:** A claim is one independently verifiable assertion, so a sentence carrying two yields two items and the engineer can reject one and keep the other. The gate is bounded by scoping it to the lean core, which is two to five short paragraphs by construction. That the resulting count is small enough to be read is a prediction, and it is carried as an open item rather than asserted. +- **Affected decisions:** D12 +- **Affected tech-notes:** T1 +- **Changed in spec:** Primary Flow, Edge Cases and Failure Modes, Open Items + +### F11: Checklist boxes are claims and were not gated + +- **Raised by:** `junior-developer` (JD-013), `edge-case-explorer` (EC7) +- **Finding:** A checked box is a factual assertion made to the reviewer in the engineer's name. The spec's promise was that the engineer sees every claim matched against evidence; a box checked by the conformance rules and never shown at the gate breaks that promise on the item type that most looks like an attestation. Same question for the "not applicable" notes the template rules generate. +- **Resolved by:** evidence +- **Resolution:** Every checklist box the draft proposes to check is a gate item, as is any prose filled into a template's sections. The reviewers' second question — the "not applicable" notes — was left unanswered by the first resolution and is closed by the synthesis pass (see S1): such a note is a claim of absence and blocks. +- **Affected decisions:** D12, D8 +- **Affected tech-notes:** — +- **Changed in spec:** Primary Flow, Edge Cases and Failure Modes + +### F12: A rejection could empty a mandatory template section, and two rules collided + +- **Raised by:** `edge-case-explorer` (EC4) +- **Finding:** The template rules (carried forward unchanged) require every section to stay present, filled or honestly noted. D7 forbids the skill from drafting new text after the gate. When a rejection empties a mandatory section, the spec named no winner — so an implementer would have silently violated one rule or the other, depending on instinct. +- **Resolved by:** evidence +- **Resolution:** The empty section returns to the engineer: they write it or mark it explicitly not applicable. The template keeps its section, the skill invents nothing. +- **Affected decisions:** D16, D8 +- **Affected tech-notes:** — +- **Changed in spec:** Edge Cases and Failure Modes, User Interactions + +### F13: Publishing destroyed the engineer's own hand-written description, silently + +- **Raised by:** `junior-developer` (JD-008), `user-experience-designer` (UX-008), independently; JD escalated it to `devops-engineer` as a destructive write to a system of record +- **Finding:** The skill replaces the whole PR body. Running it on a pull request that already has a description is not an edge case, it is the skill's name. The confirmation asked whether to proceed, never what would be lost. Both reviewers landed on the same irony: the gate goes to great lengths to protect the reviewer from text the engineer did not validate, while the publish step destroys the text they certainly did. And because the issue link is deliberately not generated, the most common re-run outcome was: delete the engineer's ticket link, put nothing back. +- **Resolved by:** evidence +- **Resolution:** Before asking to publish, the skill says what is about to be lost. Merging into the existing description was considered and rejected: it requires the skill to judge which human sentences survive, which is a bigger feature and a new fabrication surface. The interaction with the deferred issue link is now stated in the spec rather than left to be discovered. +- **Affected decisions:** D14 +- **Affected tech-notes:** — +- **Changed in spec:** Primary Flow, Edge Cases and Failure Modes, Out of Scope, Deferred (YAGNI) + +### F14: Absence claims are unprovable by construction and had no path + +- **Raised by:** `edge-case-explorer` (EC3) +- **Finding:** A diff proves what changed; it can never prove nothing else did. The spec recognized this for intent and gave it a dedicated path, then dropped absence claims ("no behavior changes for existing callers") into the generic unevidenced bucket, where the marker would fire on every no-behavior-change pull request by design. A marker that fires routinely for structural reasons is a marker the engineer learns to clear without reading. +- **Resolved by:** evidence +- **Resolution:** Absence claims are their own blocking category, alongside intent. The engineer vouches or the claim is dropped, and the unevidenced marker keeps meaning what it says. The synthesis pass extended the category to the template's "not applicable" notes, which are the same shape of claim. +- **Affected decisions:** D15 +- **Affected tech-notes:** — +- **Changed in spec:** The Gate, Edge Cases and Failure Modes + +### F15: A truncated diff was conflated with a large one, and never disclosed + +- **Raised by:** `edge-case-explorer` (EC5), `user-experience-designer` (UX-007) +- **Finding:** The spec answered "diff too large to read in full" by pointing at the size warning. But the size warning is about a *reviewer's* defect-finding; it says nothing about the *skill's* blindness, and the two conditions are unrelated. This is the gate's one silent failure surface: the gate can only show claims that exist, so a claim about a file the skill never opened simply isn't there. An engineer who verifies every item on the screen would reasonably believe they verified the change, when they verified a description of an unknown subset of it. +- **Resolved by:** evidence +- **Resolution:** An incomplete read is its own condition, disclosed at the gate, naming what was not read. A gate that cannot be complete must say that it is not. +- **Affected decisions:** D18 +- **Affected tech-notes:** — +- **Changed in spec:** Primary Flow, Edge Cases and Failure Modes + +### F16: "Edit the draft directly" was an unaudited trapdoor + +- **Raised by:** `user-experience-designer` (UX-009) +- **Finding:** The spec offered direct editing as a third option and said nothing about what it did to the gate's state. As written, it was a one-keystroke path from twenty unread claims to publish. +- **Resolved by:** evidence +- **Resolution:** Direct editing is kept and its rule is stated: text the engineer wrote is text they verified, but it verifies only what they touched. Any claim they did not edit is still theirs to dispose of, and the blocking items still block. +- **Affected decisions:** D21 +- **Affected tech-notes:** — +- **Changed in spec:** The Gate + +### F17: Rejecting everything was a dead end, and prevention was cheaper than recovery + +- **Raised by:** `junior-developer` (JD-009), `user-experience-designer` (UX-013) +- **Finding:** "The skill says so rather than publishing a hollow description" leaves the engineer holding nothing, in exactly the case where the draft was worst and they most need help. The UX reviewer went further and found the cheaper fix: the skill already accepts optional context as an argument, unused. A draft written against a stated intent invents fewer claims, which shrinks the gate. Error prevention beats error recovery, using a capability that already exists. +- **Resolved by:** evidence +- **Resolution:** Both. The engineer may state intent up front, and the draft is written against it. Rejecting everything now offers a re-draft with their corrections as input. The synthesis pass settled the follow-on question the resolution left open (see S3): the intent sentence still blocks at the gate even when intent was supplied, because the draft renders it in the skill's words. +- **Affected decisions:** D17 +- **Affected tech-notes:** — +- **Changed in spec:** Actors and Triggers, Primary Flow, The Gate, Edge Cases and Failure Modes + +### F18: Mechanics leaked into the spec + +- **Raised by:** `junior-developer` (JD-017), `user-experience-designer` (UX-014) +- **Finding:** Primary Flow step 4 described a pipeline ("rather than the pairing being reconstructed afterward from the finished text") and D5's outcome named agent selection. Both reviewers noted the leak is user-authorized, so it is not a violation — but the spec should state the behavior, not the mechanism, and should not read as precedent for putting agent selection in specs. +- **Resolved by:** evidence +- **Resolution:** The spec now states the behavior: *the skill never presents a claim as evidenced unless it held that evidence at the moment it wrote the claim,* and *nothing rewrites the description between the moment it is drafted and the moment the engineer sees it.* The mechanism stays in T1, which is its home. D5 is demoted to a trivial decision, with its rationale intact, its exceptional status noted, and no spec reference at all. +- **Affected decisions:** D5 +- **Affected tech-notes:** T1 +- **Changed in spec:** Primary Flow + +## Minor edits + +- F19: "The engineer meets the skill twice" was contradicted by the spec's own five points of contact — `junior-developer` (JD-003), `user-experience-designer` (UX-011) — User Interactions, rewritten around the gate as the one stop, with two conditional questions bracketing it and one following it. +- F20: Gate re-entry was undefined; "correct any entry" read as singular — `user-experience-designer` (UX-010) — The Gate, dispositions now accumulate and the gate holds until the engineer is done. +- F21: The no-PR deliverable had no stated form; a long markdown block in terminal scrollback is hostile to copying, and the skill already ships a temp-file script — `junior-developer` (JD-012), `user-experience-designer` (UX-012) — D20, the description is written to a file. +- F22: Re-runs were unaddressed; the natural workflow is several per pull request — `junior-developer` (JD-014) — D19, a re-run re-drafts and re-gates, and verified claims are not carried across a changed diff. +- F23: Binary-file claims have no part of the diff to point at; correct behavior fell out of the generic rule but was never named — `edge-case-explorer` (EC6) — Edge Cases and Failure Modes, named explicitly so it is not mistaken for a defect. +- F24: Open Items was empty and the spec had no statement of what would prove the gate works — `junior-developer` (JD-011), `user-experience-designer` (OQ4) — Open Items, now carries the unmeasured claim count, the observable that would reveal wave-through recurring, the untested size warning, the weakness of the feedback ask's evidence, and the untried child-skill path. + +## Closed without change + +- **`junior-developer` JD-015 — "diff too large to read" row is a no-op, defer it.** Not deferred. The reviewer was right that the row *as written* added no behavior, but `edge-case-explorer` (EC5) and `user-experience-designer` (UX-007) showed the underlying condition is real, distinct from the size warning, and the gate's only silent failure surface. Resolved by making the row real rather than removing it. See F15. +- **`edge-case-explorer` EC8 — merge-commit content misattributed as intentional design.** No change. The skill's existing merge-base-relative diff already excludes default-branch content. The residual case (conflict-resolution edits inside a merge commit) has no evidence of occurring against this skill's solo and small-team usage. *Reopening trigger:* an engineer reports a claim evidenced against diff content that turned out to be a conflict resolution rather than an intentional change. + +## Synthesis reconciliation + +Found while reconciling the four artifacts against each other and against the code and research they cite. Not reviewer findings; recorded here so the next reader can see the whole chain. + +- **S1: A template's "not applicable" note was still written on the engineer's behalf.** F11 raised the question and the resolution answered it only for boxes and section prose. D16 forbids auto-filling "Not applicable" *after* a rejection, while the conformance rules carried forward under D8 have the skill write exactly that note *during* drafting, so the same assertion was banned at one end of the flow and mandated at the other. Resolved: the note is a claim of absence, so it is a gate item (D12) and it blocks (D15). D8 now records that its fill passes through the gate rather than being final text. +- **S2: Two rewrite paths survived the fix to F9.** D11 named the readability editor and stopped there. The skill also runs a readability self-check and a structural verification step, each ending with an instruction to fix what it finds — both rewrite authored text before the gate, which is the hazard T1 rules out. D11 and T1 now cover any pass that touches the words, and conformance is something the authoring pass satisfies rather than something a later pass repairs. +- **S3: D7's re-rendering was the same objection D11 raises against a post-gate rewrite.** D11 rejects running readability after the gate because it produces "final text no human ever saw," yet D7 permits the skill to re-join surviving assertions into prose after approval, and nothing said the engineer sees the result. Resolved: the re-rendered description is shown to the engineer before anything is published. Related: the intent sentence still blocks even when the engineer supplied their intent up front, because the draft renders it in the skill's words (D17). +- **S4: Two evidence claims were stronger than their sources.** D13 asserted the question mechanism "can return empty answers" as an unconditional property; the cited guidance says it fails when the skill or a parent skill lists the tool among its auto-approved tools, which this skill does not. D4 asserted it was reusing the skill's existing "significant files" definition; that definition names documentation and configuration only, so lockfiles, generated code, and vendored dependencies are an extension. Both decisions stand; both now state what their evidence actually says. The spec's Open Items likewise no longer state the 61% no-review figure as a fact about this design. diff --git a/docs/plans/pr-description-rebuild/feature-specification.md b/docs/plans/pr-description-rebuild/feature-specification.md new file mode 100644 index 00000000..1552ba28 --- /dev/null +++ b/docs/plans/pr-description-rebuild/feature-specification.md @@ -0,0 +1,200 @@ +# Feature Specification: Verified PR Descriptions + +## Outcome + +An engineer finishes work on a branch and runs the skill, which produces a pull request description they have verified. Nothing the skill could not evidence, and nothing only the engineer could know, reaches the reviewer unless the engineer decides it should. Publishing to GitHub is the optional last step, not the deliverable. + +The change from today is the verification gate. Today the skill drafts a description and asks for a single yes-or-no before publishing, so a claim the draft invented can reach the pull request unchallenged. After this change, the skill shows each claim beside the evidence it recorded for that claim. It will not finish while a claim it could not evidence, a claim an adversarial pass could refute, or a statement only the engineer can vouch for, is still undecided ([D2](artifacts/decision-log.md#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for), [D23](artifacts/decision-log.md#d23-an-adversarial-pass-refutes-each-claim-against-its-evidence-before-the-gate)). + +Be precise about what the gate does and does not close. It shows whether evidence exists for a claim, and it puts an adversary between a claim and the pull request. It does not certify that the evidence supports the claim. That judgment stays with the engineer, and the gate is built to spend their attention where invention is most likely rather than to spread it flat across every row ([D24](artifacts/decision-log.md#d24-the-gate-is-ordered-grouped-counted-and-honest-about-what-it-does-not-know), [T2](artifacts/feature-technical-notes.md#t2-refutation-is-a-separate-pass-that-reads-the-claims-and-never-touches-the-words)). + +The description's shape barely changes. Where the repository has no template of its own, the description keeps the lean core the skill already produces and gains one line: what kind of feedback the engineer wants. Where the repository has a template, the template is discovered and its own instructions are honored. Usually that means the template dictates the shape. Where the template is a scaffold that tells the author to replace it, the lean core is used instead, and the feedback line and the gate behave exactly as they do when no template exists ([D3](artifacts/decision-log.md#d3-the-lean-core-is-kept-and-gains-a-feedback-ask), [D8](artifacts/decision-log.md#d8-repository-template-conformance-is-carried-forward-and-its-fill-passes-through-the-gate)). + +## Actors and Triggers + +- **The engineer.** Invokes the skill on a branch with commits ahead of the default branch. They are the only actor, and they are the one who verifies the result. +- **The trigger.** The engineer runs the skill, optionally stating up front why they made the change ([D17](artifacts/decision-log.md#d17-the-engineer-may-state-intent-before-the-draft-is-written)). Nothing runs it automatically. + +The skill assumes the engineer knows why they made the change, and it assumes they are willing to read the evidence shown beside each claim. Both assumptions are load-bearing, and the second is the harder one. Judging whether a hunk supports a claim means reading the hunk. The skill does not pretend otherwise, and an engineer who will not do that gets a gate that launders their inattention into a verified-looking description. + +## Primary Flow + +1. **The skill confirms the branch has something to describe.** A branch with no commits, or with commits but no file changes, produces no description. The skill says so and stops ([D22](artifacts/decision-log.md#d22-the-existing-branch-state-and-template-discovery-guards-are-carried-forward)). + +2. **The skill establishes the full set of changed files before it reads any of them.** This is what lets it know later what it did not read. A disclosure about an unread part of the diff is only honest if the skill knew the part existed ([D18](artifacts/decision-log.md#d18-an-incomplete-read-of-the-diff-is-disclosed-as-its-own-condition)). + + It also measures the change here. Size is lines added and deleted in code files. Documentation, configuration, lockfiles, generated code, and vendored dependencies do not count ([D4](artifacts/decision-log.md#d4-the-size-fact-is-delivered-at-the-gate-not-shouted-past-the-engineer)). + +3. **The skill drafts the description.** The draft is written to the repository's pull request template when one applies, and to the skill's own lean shape when none does ([D8](artifacts/decision-log.md#d8-repository-template-conformance-is-carried-forward-and-its-fill-passes-through-the-gate)). Each claim records the evidence it was written from. The skill never presents a claim as evidenced unless it held that evidence at the moment it wrote the claim ([T1](artifacts/feature-technical-notes.md#t1-claim-provenance-is-captured-at-authoring-time)). + + Nothing rewrites the description between the moment it is drafted and the moment the engineer sees it at the gate. The prose is written to the readability the repository expects, to the shape the template requires, and to the formatting a pull request body must satisfy, as it is written, not corrected into shape afterward. The text at the gate is therefore the text whose evidence was recorded ([D11](artifacts/decision-log.md#d11-nothing-rewrites-the-draft-between-authoring-and-the-gate)). + + Where the engineer stated their intent up front, the draft is written against it rather than guessing at it ([D17](artifacts/decision-log.md#d17-the-engineer-may-state-intent-before-the-draft-is-written)). + +4. **An adversary tries to knock the draft down.** A separate pass reads each claim beside the evidence recorded for it and asks one question: does this evidence actually support this claim? A claim it refutes is demoted into the blocking tier and carries its challenge to the gate. This pass reads. It attaches challenges and moves claims between tiers, and it changes not one word of the description ([D23](artifacts/decision-log.md#d23-an-adversarial-pass-refutes-each-claim-against-its-evidence-before-the-gate), [T2](artifacts/feature-technical-notes.md#t2-refutation-is-a-separate-pass-that-reads-the-claims-and-never-touches-the-words)). + + This is the half of the problem the authoring pass cannot close by itself. A pass can honestly report that it found nothing to write from. It cannot honestly certify its own output, and a fabricated claim usually arrives with a confident pointer at a real hunk rather than with nothing attached. + +5. **The skill assembles the gate.** Every assertion the description makes to the reviewer is a gate item. That includes: + - the sentences of the summary and the behavior changes + - any prose filled into a repository template's sections + - every checklist box the draft proposes to check ([D12](artifacts/decision-log.md#d12-a-claim-is-one-independently-verifiable-assertion-and-the-gate-covers-every-assertion-the-reviewer-will-read)) + + The guide to what to read first points at files and areas and characterizes nothing as a risk, a tradeoff, or a decision, so it asserts nothing about the change and is not a gate item ([D9](artifacts/decision-log.md#d9-the-reading-order-guide-keeps-its-existing-threshold), [D25](artifacts/decision-log.md#d25-the-reading-order-guide-points-and-characterizes-nothing)). + +6. **The skill shows the gate, and holds.** The engineer sees the drafted description in full, as prose, before it is broken into items, because a claim judged out of the sentences around it is judged out of the context that makes it true or misleading. + + Then the gate opens with its preamble: how large the change is, whether the skill read all of it, how many items the gate holds, and how many of them block. These are facts, not items. The engineer disposes of each blocking item, accepts or edits the rest, and the gate does not finish while any blocking item is undecided ([D2](artifacts/decision-log.md#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for), [D4](artifacts/decision-log.md#d4-the-size-fact-is-delivered-at-the-gate-not-shouted-past-the-engineer), [D18](artifacts/decision-log.md#d18-an-incomplete-read-of-the-diff-is-disclosed-as-its-own-condition), [D24](artifacts/decision-log.md#d24-the-gate-is-ordered-grouped-counted-and-honest-about-what-it-does-not-know)). + +7. **The skill applies the engineer's decisions.** A claim they correct is used as they wrote it. A claim they reject is removed. The description is re-rendered from what survives, without re-drafting over it ([D7](artifacts/decision-log.md#d7-the-engineers-verdict-is-binding-and-re-rendering-does-not-re-draft)). + + The engineer then sees two things: the re-rendered description, and a short list of what it no longer says. Dropping a claim is the cheapest thing they can do at the gate and it makes the rest of the gate shorter, so the description they hollowed out is shown to them as a description they hollowed out ([D24](artifacts/decision-log.md#d24-the-gate-is-ordered-grouped-counted-and-honest-about-what-it-does-not-know)). They accept it, edit it, or go back to the gate. + +8. **The skill delivers the verified description, and offers to publish it.** When the branch has an open pull request, the skill offers to update it. It first writes the description that is about to be destroyed to a file, and says what that description said and where it now lives ([D14](artifacts/decision-log.md#d14-replacing-an-existing-description-is-disclosed-before-it-happens), [D28](artifacts/decision-log.md#d28-the-skills-files-live-outside-the-working-tree-and-the-description-it-replaces-is-kept)). It publishes only on the engineer's word. + + When the skill does not publish, whether because no pull request exists, the engineer declined, or the run ended unverified, the verified description is written to a file the engineer can take away. Every file the skill writes lands outside the repository's working tree, so a description can never be committed into the change it describes ([D20](artifacts/decision-log.md#d20-the-verified-description-is-written-to-a-file), [D28](artifacts/decision-log.md#d28-the-skills-files-live-outside-the-working-tree-and-the-description-it-replaces-is-kept)). + +## The Gate + +The gate is the feature, so its rules are stated here rather than left to be inferred. + +**Blocking items.** Four kinds of item must be individually disposed of, and there is no path that accepts them in bulk: + +- **A claim the skill could not evidence.** The engineer keeps it, corrects it, or drops it. +- **A claim the adversarial pass refuted.** It is shown with the challenge that refuted it. The engineer keeps it, corrects it, or drops it, in full view of why the skill's own adversary did not believe it ([D23](artifacts/decision-log.md#d23-an-adversarial-pass-refutes-each-claim-against-its-evidence-before-the-gate)). +- **A claim about absence** that the description asserts to the reviewer, such as a claim that nothing changed for existing callers or that no behavior changed. No diff can evidence an absence, so the engineer vouches for it or it goes ([D15](artifacts/decision-log.md#d15-absence-claims-are-structurally-unprovable-like-intent)). +- **The statement of why the change exists,** together with the ask for what feedback the engineer wants. Neither is in the diff. The engineer writes or confirms both. Skipping the feedback ask is something they choose, not something they achieve by staying silent, because silence is never a valid answer to a blocking item ([D3](artifacts/decision-log.md#d3-the-lean-core-is-kept-and-gains-a-feedback-ask), [D13](artifacts/decision-log.md#d13-the-gate-fails-closed)). The engineer confirms the intent sentence even when they stated their intent up front. The draft renders that intent in its own words, and those words are the ones the reviewer will read ([D17](artifacts/decision-log.md#d17-the-engineer-may-state-intent-before-the-draft-is-written)). + +A template section the change simply does not reach is not one of these. It is one judgment about scope, not many. The engineer is shown the set of sections the change does not reach and vouches for the set in one act, pulling out any individual section that deserves its own decision. Confusing "the diff proves nothing changed here" with "this section plainly does not apply to a one-line docs fix" would make the smallest change draw the longest gate, which is how an engineer learns that blocking items are noise to be cleared ([D26](artifacts/decision-log.md#d26-a-template-section-the-change-does-not-reach-is-one-judgment-not-many)). + +**How each item is disposed of.** A prose claim is kept, corrected, or dropped. A checklist box is confirmed checked or returned to unchecked, and it is never removed from the checklist, because the template's checklist is reproduced whole. A note that a section does not apply is vouched for or replaced with the engineer's own text, and a section left empty by rejections comes back to them ([D16](artifacts/decision-log.md#d16-a-section-emptied-by-rejections-returns-to-the-engineer)). No disposition is offered as a default or pre-selected, because dropping a claim is already the cheapest act at the gate and it does not need help. + +**Non-blocking items.** Every other claim survived the adversary, and is shown with the evidence recorded for it and the locus that evidence came from: a hunk in the diff, a file the change did not touch, or nothing but a commit message. They are ordered weakest locus first, so the engineer's first and freshest attention lands where invention is most likely. They may be accepted together once the blocking items are settled, and that acceptance is worded as what it is: the engineer saying they have read these and stand behind them. + +The gate does not label these claims "supported" and does not vouch for them. **The gate shows whether evidence exists, not whether the evidence supports the claim.** The skill is not in a position to certify a claim it wrote itself, and a badge saying it is would teach the engineer to skip exactly the rows a fabricated claim hides in. What the skill can honestly do is show what it pointed at, put an adversary in front of it, and order the rest so attention is spent where it buys the most ([D2](artifacts/decision-log.md#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for), [D24](artifacts/decision-log.md#d24-the-gate-is-ordered-grouped-counted-and-honest-about-what-it-does-not-know)). + +**The gate announces its shape before the first question.** How many items it holds, and how many of them block. Blocking items come first, grouped by kind, because the four kinds ask for four different judgments. An engineer who cannot see how much gate remains stops answering carefully and starts optimizing for exit, and a wave-through that arrives as time management is still a wave-through ([D24](artifacts/decision-log.md#d24-the-gate-is-ordered-grouped-counted-and-honest-about-what-it-does-not-know)). + +**The gate fails closed.** If the skill cannot obtain an answer from a human, that is not approval ([D13](artifacts/decision-log.md#d13-the-gate-fails-closed)). + +**The engineer can leave at any time,** and leaving is not failing. An abandoned run and a fail-closed run deliver the same thing: the claims as an un-assembled list, each beside its evidence and its standing, with the unevidenced and refuted ones named, and nothing published. That artifact is deliberately not a description. An engineer cannot paste it into GitHub without assembling it themselves, which is exactly the point, and it is what gives the fail-closed rule teeth on a branch that has no pull request to withhold publication from ([D27](artifacts/decision-log.md#d27-the-engineer-can-always-leave-and-an-unverified-run-does-not-hand-them-paste-ready-prose)). + +**The engineer can edit an item directly,** and text they wrote is text they verified, so it needs no further evidence. The unit of editing is the item, not the draft: a claim, a section's prose, the intent sentence. The skill never re-derives the item set by reading edited prose, because a pass that reads finished text and goes looking for the claims in it is the same untrustworthy reconstruction the authoring rule exists to prevent. Editing is not an exit from the gate: any item they did not touch is still theirs to dispose of ([D21](artifacts/decision-log.md#d21-editing-the-draft-directly-verifies-what-was-edited-and-nothing-else), [D30](artifacts/decision-log.md#d30-the-unit-of-direct-editing-is-an-item-not-the-draft)). + +## Alternate Flows and States + +- **No pull request exists yet.** The gate still runs, and the verified description is still written to a file. The engineer is taking the text somewhere regardless of whether the skill publishes it, and an unverified claim is no less wrong for being pasted by hand ([D6](artifacts/decision-log.md#d6-the-gate-runs-whether-or-not-a-pull-request-exists)). + +- **GitHub is unreachable.** The GitHub CLI is missing, or it is present but unauthenticated. The run degrades to the no-pull-request flow: the gate runs and the file is delivered. The skill does not stop, because the gate and the file are worth having without GitHub ([D29](artifacts/decision-log.md#d29-github-access-is-needed-only-to-publish)). + +- **The engineer declines to publish.** The verified description is written to a file and stands as the deliverable. Nothing is written to GitHub. + +- **The engineer abandons the gate.** They get the un-assembled claim list, and nothing is published ([D27](artifacts/decision-log.md#d27-the-engineer-can-always-leave-and-an-unverified-run-does-not-hand-them-paste-ready-prose)). + +- **The skill is run again on the same branch.** It re-drafts and re-gates from the current diff. Nothing is carried forward, including the intent sentence and the feedback ask. The diff a claim was verified against has changed, and the intent and feedback line belong to a description that no longer exists ([D19](artifacts/decision-log.md#d19-a-re-run-re-drafts-and-re-gates)). A re-run also replaces the previous run's file. + +- **The repository offers several templates.** The skill cannot know which the engineer intends, so it asks, offering the option of using none ([D22](artifacts/decision-log.md#d22-the-existing-branch-state-and-template-discovery-guards-are-carried-forward)). + +- **The engineer rejects the stated intent and supplies their own.** Their wording is used as written ([D7](artifacts/decision-log.md#d7-the-engineers-verdict-is-binding-and-re-rendering-does-not-re-draft)). + +- **The engineer rejects every claim.** The skill offers, once, to author a fresh draft with their corrections as input. That draft is a new authoring pass and it produces a new gate, entered from the top. Declining takes the abandon path ([D31](artifacts/decision-log.md#d31-a-wholly-rejected-draft-is-re-authored-and-re-gated-from-the-top)). + +## Edge Cases and Failure Modes + +| Case | Behavior | +|---|---| +| A claim rests on repository context outside the diff (an existing default, code the change interacts with) | It is evidenced by the file it rests on. Evidence means the diff or a cited repository file, not the diff alone. Only a claim the skill can point at nothing for is marked unevidenced ([D10](artifacts/decision-log.md#d10-evidence-is-the-diff-or-a-cited-repository-file)). At the gate it is shown as resting on an unchanged file, which is a weaker locus than a hunk, and it is ordered accordingly ([D24](artifacts/decision-log.md#d24-the-gate-is-ordered-grouped-counted-and-honest-about-what-it-does-not-know)). | +| A claim cites real evidence that does not actually support it | The adversarial pass exists for this case and is the only thing standing in its way. When the pass refutes the claim, it is demoted into the blocking tier and shown with the challenge. When the pass fails to refute it, the claim reaches the engineer in the non-blocking tier, and the engineer is the last line. The spec does not pretend the gate closes this ([D23](artifacts/decision-log.md#d23-an-adversarial-pass-refutes-each-claim-against-its-evidence-before-the-gate), [T2](artifacts/feature-technical-notes.md#t2-refutation-is-a-separate-pass-that-reads-the-claims-and-never-touches-the-words)). | +| A sentence bundles a claim the engineer accepts with one they reject | The sentence is not the unit. Each independently verifiable assertion is disposed of on its own, and the surviving assertions are re-rendered as readable prose ([D12](artifacts/decision-log.md#d12-a-claim-is-one-independently-verifiable-assertion-and-the-gate-covers-every-assertion-the-reviewer-will-read)). | +| The diff is too large for the skill to read in full | The skill knows the full set of changed files before it starts reading, so it can name what it did not read. It says so in the gate's preamble, as a fact separate from the size warning, and the description itself carries a short note to the reviewer that part of the change was not read. The engineer may remove that note; the skill does not remove it for them. A claim the skill never had evidence for is not the same as a file it never opened ([D18](artifacts/decision-log.md#d18-an-incomplete-read-of-the-diff-is-disclosed-as-its-own-condition)). | +| The change touches a binary file | There is no part of the diff to point at, so a claim about what changed inside it is unevidenced and blocks like any other. This is by design, not a defect ([D10](artifacts/decision-log.md#d10-evidence-is-the-diff-or-a-cited-repository-file)). | +| A repository template has a section this change has nothing to say about | The template keeps its section and the draft proposes a note saying it does not apply. The engineer vouches for the set of such sections in one act rather than one at a time, because a section a docs fix does not reach is a fact about scope, not a claim the description is smuggling past a reviewer ([D26](artifacts/decision-log.md#d26-a-template-section-the-change-does-not-reach-is-one-judgment-not-many), [D8](artifacts/decision-log.md#d8-repository-template-conformance-is-carried-forward-and-its-fill-passes-through-the-gate)). | +| A repository template tells the author to replace it | It is a scaffold, not a structure. The lean core is used, and the run proceeds exactly as it would with no template at all ([D8](artifacts/decision-log.md#d8-repository-template-conformance-is-carried-forward-and-its-fill-passes-through-the-gate)). | +| The engineer rejects every claim in a section a repository template requires | The empty section comes back to them: they write it or mark it explicitly not applicable. The skill does not invent an honest-sounding note on their behalf ([D16](artifacts/decision-log.md#d16-a-section-emptied-by-rejections-returns-to-the-engineer)). | +| The engineer rejects every claim | There is no description left. The skill says so and offers, once, to author a fresh draft with their corrections as input. That draft re-gates from the top ([D31](artifacts/decision-log.md#d31-a-wholly-rejected-draft-is-re-authored-and-re-gated-from-the-top)). | +| A repository template carries a checklist | A box is proposed checked only when the diff proves it, and every proposed box is a gate item. An attestation of human action is never checked on the engineer's behalf. The checklist is reproduced whole, so a box is confirmed or returned to unchecked, never deleted ([D8](artifacts/decision-log.md#d8-repository-template-conformance-is-carried-forward-and-its-fill-passes-through-the-gate), [D12](artifacts/decision-log.md#d12-a-claim-is-one-independently-verifiable-assertion-and-the-gate-covers-every-assertion-the-reviewer-will-read)). | +| The pull request already has a description | It will be replaced. Before that happens the skill writes it to a file and says what it said and where it now lives, so replacing it is recoverable rather than merely disclosed ([D14](artifacts/decision-log.md#d14-replacing-an-existing-description-is-disclosed-before-it-happens), [D28](artifacts/decision-log.md#d28-the-skills-files-live-outside-the-working-tree-and-the-description-it-replaces-is-kept)). | +| The default branch cannot be determined | The skill asks which branch to compare against rather than guessing ([D22](artifacts/decision-log.md#d22-the-existing-branch-state-and-template-discovery-guards-are-carried-forward)). | + +## User Interactions + +The gate is the one place the skill genuinely stops in an ordinary run. Two questions can bracket it, and two follow it. + +- **Which branch to compare against:** only when the default branch cannot be determined. +- **Which template to use:** only when the repository offers several. +- **The gate:** always. The engineer disposes of each blocking item, accepts or edits the rest, and cannot reach the end without deciding what the description is allowed to assert. They can abandon it at any point and take the un-assembled claim list instead. +- **The re-rendered description:** always. They accept it, edit it, or return to the gate. Seeing is not approving, and on a branch with no pull request this is the last decision they get. +- **Whether to publish:** always, when a pull request exists, and stating what will be replaced and where it has been saved. + +Two recovery paths return to the engineer rather than adding a routine step. A mandatory template section emptied by rejections comes back for them to fill ([D16](artifacts/decision-log.md#d16-a-section-emptied-by-rejections-returns-to-the-engineer)). A wholly rejected draft is offered once more with their corrections as input ([D31](artifacts/decision-log.md#d31-a-wholly-rejected-draft-is-re-authored-and-re-gated-from-the-top)). Both are consequences of what the engineer decided at the gate, not questions the skill asks of its own accord. + +The size of the change is not another interaction, and neither is the disclosure of an incomplete read. They are facts carried into the gate's preamble, where the engineer is already present and already reading. That is different from a message fired early in the run that scrolls out of sight before anyone reads it ([D4](artifacts/decision-log.md#d4-the-size-fact-is-delivered-at-the-gate-not-shouted-past-the-engineer)). They are stated once, before the items, and they are not items, because two things the engineer can do nothing about sitting among N things that demand action is how a gate teaches its own dismissal. + +## Coordinations + +- **The version control history.** Read-only. The branch's commits, changed files, and diff. +- **The repository's own files.** Read-only. The pull request template when one exists, and any source file a claim rests on. +- **The engineer's filesystem, outside the working tree.** Written. The verified description when the skill does not publish it, the un-assembled claim list when a run ends unverified or abandoned, and the pull request description that is about to be replaced. Never inside the working tree, so nothing the skill writes can be committed into the change it describes ([D28](artifacts/decision-log.md#d28-the-skills-files-live-outside-the-working-tree-and-the-description-it-replaces-is-kept)). +- **GitHub.** Read to find whether the branch has an open pull request, and to read the description that is about to be replaced. Reading that description is new: today the skill asks only whether a pull request exists. Written once, only to set the description, and only after the engineer approves. Needed only to publish, so its absence degrades the run rather than stopping it ([D29](artifacts/decision-log.md#d29-github-access-is-needed-only-to-publish)). + +## Out of Scope + +- Reviewing the code. The skill describes a change; it does not judge it. +- Posting review comments. +- Splitting an oversized change. The skill says the change is large; it does not divide it. +- Writing commit messages or issue bodies. +- Creating the pull request. The skill updates one that exists. +- Merging the existing description into the new one. The old description is replaced, and it is preserved to a file first so the replacement is recoverable. Preserving it is not merging it: reading the old text back into the new one is a fabrication surface the gate cannot cover ([D14](artifacts/decision-log.md#d14-replacing-an-existing-description-is-disclosed-before-it-happens), [D28](artifacts/decision-log.md#d28-the-skills-files-live-outside-the-working-tree-and-the-description-it-replaces-is-kept)). + +## Deferred (YAGNI) + +- **A link to the related issue or ticket.** Nearly universal across the sources, and associated with a faster first reviewer response. Deferred with the rest of the section set ([D3](artifacts/decision-log.md#d3-the-lean-core-is-kept-and-gains-a-feedback-ask)). *Reopening trigger:* a reviewer asks for context that lived in a ticket the description did not link, or a repository template requires the link. + +- **A testing note.** In the research's recommended core, used by a minority of pull requests in practice. Deferred for the same reason ([D3](artifacts/decision-log.md#d3-the-lean-core-is-kept-and-gains-a-feedback-ask)). *Reopening trigger:* a repository template requires it, or reviewers start asking how a change was tested. + +- **Conditional sections for screenshots, rollback, and migrations.** No evidence in this repository shows an engineer needing one. *Reopening trigger:* a change of that kind is described and the engineer adds the section by hand. + +- **Writing the description to a file on a run that publishes successfully.** The need the file serves is that terminal scrollback is a bad place to copy text out of, and that need exists only where the engineer moves the text by hand. On a successful publish they already have it on GitHub ([D28](artifacts/decision-log.md#d28-the-skills-files-live-outside-the-working-tree-and-the-description-it-replaces-is-kept)). *Reopening trigger:* an engineer asks for the file after a successful publish. + +- **Blocking on an oversized change.** Rejected: the work is done by the time the skill runs, so refusing to describe it punishes a decision the engineer can no longer cheaply reverse ([D4](artifacts/decision-log.md#d4-the-size-fact-is-delivered-at-the-gate-not-shouted-past-the-engineer)). + +- **Carrying anything across re-runs.** Rejected as unsound: a claim was verified against a diff that has since changed, and the intent sentence and feedback line belong to a description that no longer exists ([D19](artifacts/decision-log.md#d19-a-re-run-re-drafts-and-re-gates)). + +Replacement destroys everything a human added to the pull request description by hand: a ticket link, a screenshot, a note to a reviewer, a rollback plan, and the engineer's own corrections from an earlier run. The skill writes the old description to a file before it replaces it, so none of that is lost, but nothing puts it back into the new description automatically, and nothing is going to ([D28](artifacts/decision-log.md#d28-the-skills-files-live-outside-the-working-tree-and-the-description-it-replaces-is-kept)). + +## Open Items + +- **Nobody has run this gate, so nobody knows how many items it produces.** The whole design rests on the claim count being small enough that an engineer reads it rather than skims it. [D12](artifacts/decision-log.md#d12-a-claim-is-one-independently-verifiable-assertion-and-the-gate-covers-every-assertion-the-reviewer-will-read) bounds the gate by scoping it to the lean core, which should keep it small, but that bound does not reach the templated branch: there the section count is set by the repository, not by the skill. [D26](artifacts/decision-log.md#d26-a-template-section-the-change-does-not-reach-is-one-judgment-not-many) removes the worst of it by disposing of unreached sections as one judgment, but that is still a prediction. Resolve by drafting the gate against ten real merged pull requests, at least three of them against a repository with a multi-section template, and counting, before the design is treated as settled. + +- **The adversarial pass is a new cost on every run and its hit rate is unknown.** It exists because the authoring pass structurally cannot certify its own citations ([T2](artifacts/feature-technical-notes.md#t2-refutation-is-a-separate-pass-that-reads-the-claims-and-never-touches-the-words)), and that argument is sound. What is unknown is whether it refutes anything worth refuting, or whether it refutes so much that the blocking tier fills with challenges the engineer learns to dismiss. Both failure modes retire the feature. *Resolve by watching, during dogfooding, how often it demotes a claim and how often the engineer agrees with the demotion. A pass that never fires is dead weight. A pass the engineer overrules every time is worse than dead weight, because it trains the override.* + +- **The evidence that AI-authored pull requests go unreviewed is a single 2026 preprint, and the spec cannot tell whether the same thing is happening here.** The finding that motivates the gate is that most AI-authored pull requests draw no recorded human review, roughly 61%. That finding is single-source, shares a dataset with a companion study, and was measured on open-source repositories rather than this workflow ([D2](artifacts/decision-log.md#d2-the-gate-shows-each-claim-with-its-evidence-and-blocks-on-what-the-skill-cannot-vouch-for)). It is a risk signal, not a measurement of this design. The observable that would show the same failure recurring is how often an engineer corrects or rejects something at the gate. A gate that never changes anything is a gate being waved through. Watch it during dogfooding rather than building instrumentation for it. + +- **The feedback ask is the largest measured effect in the research and the weakest-supported commitment in the spec.** The odds-ratio figure comes from one observational 2026 preprint. It is associative rather than causal, and was measured on human-authored pull requests across open-source projects, not on a solo or small-team workflow ([D3](artifacts/decision-log.md#d3-the-lean-core-is-kept-and-gains-a-feedback-ask)). The spec commits to asking for it on every run on that basis. *Resolve by watching whether the field is answered or routinely skipped. If it is routinely skipped, the ask is costing an interaction and buying nothing, and it should be dropped.* + +- **Whether the size warning changes anyone's behavior is untested.** It is delivered where it will be read now, which is the most that can be said for it. + +- **A gate that fails closed has never been run as a child of another skill.** The repository's own guidance documents that the question mechanism returns empty answers when a parent skill has auto-approved it, and that the parent's rules stack onto the child. Under [D13](artifacts/decision-log.md#d13-the-gate-fails-closed), that condition produces an unverified run and no publication. That is correct behavior, but it means the skill can be silently unusable when invoked from another skill rather than by a person. *Resolve by running the skill once as a child of another skill and observing whether the gate can be answered.* + +## Summary + +The skill gains a gate that shows each claim beside the evidence it was written from, after an adversary has tried and failed to refute it. The gate will not finish while an unevidenced claim, a refuted claim, an absence claim, or the statement of intent is undecided. It states honestly that it shows whether evidence exists and not whether the evidence supports the claim, and it spends the engineer's attention where invention is most likely rather than spreading it flat. It fails closed, it can be left at any time, and a run that ends unverified hands back claims rather than a description. Nothing rewrites the description between the moment it is drafted and the moment the engineer sees it, so the text they verify is the text that gets published. It discloses and preserves what it is about to destroy. The description keeps its lean core and gains an ask for the kind of feedback the engineer wants. + +## Review History + +- **Review mode:** team. +- **Spec-aware mode:** engaged. +- **Rounds completed:** 1 (of a 2-round cap) — see [artifacts/review-iteration-history.md](artifacts/review-iteration-history.md). +- **Team composition:** + - `han-core:junior-developer` (required) — generalist stress-test of the gate's hidden assumptions and undefined terms. + - `han-core:adversarial-validator` (required) — attacks on the gate's central premise and the evidence behind D2 and D3. + - `han-core:evidence-based-investigator` (conditionally mandatory) — verification of the spec's many "carried forward from today's skill" claims against `han-github/skills/update-pr-description/`. + - `han-core:user-experience-designer` — the gate is a human-in-the-loop interaction whose value depends on the engineer engaging rather than rubber-stamping. +- **Findings raised:** 29 (F25 through F53) — see [artifacts/review-findings.md](artifacts/review-findings.md). Numbering continues from the 24 findings raised during spec authoring in [artifacts/team-findings.md](artifacts/team-findings.md). Resolved by evidence: 22. Resolved by user input: 5. Deferred: 2. +- **YAGNI candidates:** 2 — the file written on every run including successful publishes (replaced with the simpler version: written only when the skill does not publish), and the confirmation of a paraphrased intent the engineer already supplied (kept, with the evidence cited: the summary sentence has a required shape and the description's voice is uniform by design). +- **Assumptions challenged across all passes:** that the authoring pass can be trusted to flag its own unevidenced claims (it can flag missing evidence but not wrong evidence, which is what forced the adversarial pass in D23); that the engineer is not being asked to re-derive the diff (the gate asks exactly that, and the spec now says so); that the reading-order guide asserts nothing (its own content rule refuted this, forcing D25); that the skill can name what it did not read (only if it establishes the file set first, now in Primary Flow step 2). +- **Consolidations made:** the size fact and the incomplete-read fact were pulled out of the item list into a single gate preamble; the three "not applicable" dispositions collapsed into one set-level judgment (D26); the abandon path and the fail-closed path were unified into one artifact (D27). +- **Ambiguities resolved, and how:** the bulk-accept loophole (user chose to keep the bulk path and make it honest, plus a new adversarial pass); the reading-order guide (user chose to narrow it to pure navigation); the paraphrased intent (user chose to keep D17 as written); the template N/A blowup (user chose set-level disposal). +- **Technical notes added:** 1 — [T2](artifacts/feature-technical-notes.md#t2-refutation-is-a-separate-pass-that-reads-the-claims-and-never-touches-the-words), the two constraints that make the adversarial pass sound. +- **Decisions added:** 9 — D23 through D31 in [artifacts/decision-log.md](artifacts/decision-log.md). +- **Open items remaining:** 6, listed above. None blocks implementation. The first two (gate item count, adversarial-pass hit rate) are dogfooding observations that should be made before the design is treated as settled.