Repository navigation
Define an accepted D1 evidence format for agent pull requests #1045
Description
Problem
D1 requires per-AC evidence in the PR Verification section, while the PR description reviewer prohibits counts of any kind and the writer also prohibits counting. The shared instructions do not document how to present that evidence consistently. Clarify and protect an accepted evidence format so an author can meet D1 through the normal review and publication gates.
Investigated current origin/main at 37f71fd4f085d8354eb6bd1803b57c4c0385830c in a disposable checkout after fetching origin:
- The published Definition of done for #886 requires the author to verify each AC on the merged head and cite each AC ID in the PR's Verification section with its command and result, or a link. The round-2 review of #1033, finding R2, requests the author's AC1 through AC10 mapping and explicitly says the reviewer's own table cannot replace it. Merged-head confirmation remains a merge-time obligation in that review.
packages/agents/source/skills/agent-pr/shared/scripts/review.sh:5invokes the shared PR review.packages/agents/source/skills/pr/shared/scripts/review.sh:125selects the description-review skill. Its source atpackages/agents/source/skills/review-pr-description/parts/body.md:43says to flag counts of any kind, including tests and percentages. The writer repeats the count prohibition atpackages/agents/source/skills/write-pr-description/parts/write.md:35. Neither distinguishes AC identifiers or per-criterion evidence from prohibited counting.- The description reviewer checks command suitability and unverified results at
packages/agents/source/skills/review-pr-description/parts/body.md:44. The PR template at.github/pull_request_template.md:11asks for commands and their results, but does not mention D1 or per-AC evidence. No DoD instruction or template requiring that map was found in the inspected devtools description-review sources; the public refinement checklist and PR review above establish that requirement. packages/agents/source/skills/pr/shared/scripts/validate-description.sh:362checks required sections and order; line 420 requires a backticked command in Verification. It has no explicit ban on AC lists or result tables. A synthetic draft with an AC1 command/result bullet and an AC2 command/result table passed the deployed validator at this revision:bash .agents/skills/pr/scripts/validate-description.sh PR_AGENTDESC.mdexited 0. This checked syntax only, not semantic review or publication.- A description edit must be reviewed again:
packages/agents/source/skills/pr/shared/scripts/create-pr.sh:173checks the draft-bound stamp, and line 194 reruns independent review before publication.packages/agents/source/skills/agent-pr/shared/scripts/publish.sh:5uses that publisher. - An author comment on #1033 already maps AC IDs to tests and commands. The current PR Verification section does not link that comment or carry the AC IDs. The comment alone therefore does not establish compliance with the cited D1 wording.
The author of the reports confirms that it never saw a reviewer reject an AC list or result table as such. It extracted these findings from its transcript; they are not attributable to a particular PR or review round:
- "AC4 is not a verification command and restates the Why section instead of reporting a test command result." The requested fix was to remove AC4 from the Verification table.
- "The Command column in the Verification table names a file path rather than a verification command." The row put README.md in Command; the requested fix was to name the command executed, such as
mise run lint-prose. - "Verification describes test assertions without naming the commands run or their results, as the template requests."
- "Verification includes implementation narration and workflow bookkeeping instead of commands and results."
- "Verification includes a test count instead of simply reporting the relevant command and result." The rejected list was "TestIssueEditTypeAC1 through TestIssueEditTypeAC11, one per acceptance criterion".
- "AC2 asserts that verification is unfinished and requires a CI rerun while draft is false, and includes count filler."
These findings support rejection of AC rows whose Command or Result is not a command and an observed result, test counts, assertion walkthroughs and diff narration inside Verification. They do not show a ban on a table mapping each AC to a real command and observed result. Current sources contain no explicit ban on AC lists or per-criterion result tables, and the mechanical validator accepted both. No rejection of a well-formed AC-to-command table was reproduced. The defect is the missing shared evidence contract and regression protection, rather than a proven impossibility of satisfying D1.
Acceptance criteria
- AC1: An agent PR with valid evidence for every AC passes description review without CHANGES REQUIRED for the evidence format and passes the publication gates. The accepted format maps each AC ID to a named test or command and its observed result or evidence link, identifies the verified head, and records merged-head confirmation when available. Evidence in a PR comment is accepted only when Verification cites the relevant AC IDs and links that evidence, and the D1 review recognises that placement. The PR’s Verification section records the D1 reviewer’s acceptance of the evidence format as the observed review result.
- AC2: The author instructions, description-review instructions and PR template describe a consistent way to meet D1. They distinguish AC identifiers and necessary evidence from prohibited aggregate counting; the same valid mapping is acceptable to both description review and D1 review.
- AC3: A regression check passes for the documented D1 evidence format and fails when the writer, template or reviewer rules contradict that format. Control cases still reject unsupported results and unrelated count padding.
- AC4: Non-functional: Description review, leak scans, lint, tests, identity, write authority and draft-bound review stamps remain enforced. Unrelated description-review requirements retain their existing effect; no gate is loosened or bypassed.
- AC5: Non-functional: Both packaged skill variants match the maintained sources, generated-output checks pass, and consumer guidance explains the accepted evidence placement without requiring a new consumer binary or changing the N-2 compatibility contract.
Scope and non-goals
Limit changes to the PR writer and description-review instruction sources, the PR template, focused regression coverage, generated skill outputs and necessary consumer guidance. Prefer evidence directly in Verification, which the current D1 wording already permits. A linked comment is an alternative only if that wording and its review still receive each AC ID and verifiable evidence.
Non-goal: Relaxing unrelated prose rules, permitting unsupported claims or decorative counts, changing D1's verification requirement, adding reviewer bypasses, or changing merge policy, authority, protected configuration or workflows. Fixing the implementation or coverage findings on #1033 is out of scope.
Approach
Document a concrete accepted example with AC IDs, named tests or commands, observed results and the verified commit. Make the writer, template and reviewer apply the same distinction between required evidence and aggregate padding. Preserve truthfulness checks and all existing publication gates. Add focused consistency fixtures with positive evidence cases and negative controls; do not treat a stubbed PASS verdict as proof that the semantic reviewer accepts the format. Generate both skill variants from their maintained sources and document the consumer steps.
Dependencies
None. #1033 and #886 are evidence and context, not delivery blockers.
Size
S
Verify plan
The delivery duties D1 to D10 of the Definition of Done apply to this pull request as to any other and are not restated as criteria.
- AC1: On a disposable committed agent branch, prepare a valid per-AC evidence map for its head, run agent-pr's description checks and independent review, then exercise the publication gates in dry-run mode. Record the review verdict and gate results, including the D1 reviewer’s acceptance of the evidence format in the PR’s Verification section. After merge, record confirmation against the merged commit. If using a comment, verify the Verification links and AC IDs and obtain D1 review acceptance of that placement.
- AC2: Compare the writer instructions, reviewer instructions and PR template with the published D1 checklist; exercise the same evidence example against both description review and D1 review and record matching decisions.
- AC3: Run the focused regression check with valid evidence fixtures, conflicting-rule mutations and negative controls for unsupported results and unrelated counting. Record which cases pass or fail and why.
- AC4: Run the existing PR workflow and agent-pr regression suites, including stale-draft stamp rejection and failed-gate refusal. Keep
mise run check,mise run cover-go-checkandmise run cover-py-checkgreen, and run the gitleaks and check-leaks scans. Inspect the diff and gate definitions to confirm no enforcement is weakened or bypassed. - AC5: Run
mise run generate-skills,mise run skill-tokensandapm installin that order; runmise run check-generated-skillsandmise run check. Inspect the consumer note and compatibility declaration againstdocs/compatibility.md. Confirm the docs, including guidance in the skill sources and PR template, match the change, or the PR states "No doc impact" with a reason.
Risks
An overbroad evidence allowance could admit unsupported claims or count padding. Keep the allowance tied to identified AC evidence and retain negative controls and truthfulness review. Model verdicts can vary, so retain the actual semantic review result alongside deterministic consistency checks. This change loosens no gate. Undo: revert the focused instruction, template, regression and guidance change and regenerate the affected skill outputs; retain existing evidence and review records.
Operator authority
None
Metadata
Metadata
Assignees
Labels
Type
Fields
Priority
Severity
Work type
Projects
- StatusShow more project fieldsBacklog
Activity
tbhb-agent commentedon Nov 1, 2026
Refinement: NOT READY — DOD obligations are missing from the acceptance criteria and Verify plan
Checked
originidentifiestbhb-dev/devtools; fetchedorigin/mainis the cited commit37f71fd4f085d8354eb6bd1803b57c4c0385830c.packages/agents/source/skills/review-pr-description/parts/body.md:43andpackages/agents/source/skills/write-pr-description/parts/write.md:35.owner_pathsorconstitutional_paths; CODEOWNERS’ default owner applies, but these changes do not trigger the configured owner tier. AC IDs AC1–AC5 are intact and there are five live ACs.Gaps
provenance:agent-assistedlabel, size-limit handling, follow-up issue handling, issue closure and final Status comment, and the Go/Python coverage floors. Add appropriate outcome criteria and checks, or explicitly establish why a checklist item is not applicable to this issue.Split
Loosens: no
Owner-tier: no
Definition of done
provenance:agent-assistedlabel.mise run check, but notmise run cover-go-checkormise run cover-py-check.Round: 1 of 3
tbhb-agent commentedon Nov 1, 2026
Refinement: READY — Definition-of-Done coverage gap closed
Checked
originidentifiestbhb-dev/devtools.origin/mainis at37f71fd4f085d8354eb6bd1803b57c4c0385830c.origin/main: the shared task files definedevtools:cover-go-check,devtools:cover-py-check,devtools:gitleaks, anddevtools:lint-leaks; root task aliases provide the statedmise runcommands.Gaps
None.
Split
No split indicated. The instruction, template, regression, generated output, and guidance work remains cohesive.
Loosens: no
Owner-tier: no
Definition of done
mise run checkand both coverage threshold checks.Round: 2 of 3