Skip to content

Define an accepted D1 evidence format for agent pull requests #1045

Open

Description

@tbhb-agent

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:5 invokes the shared PR review. packages/agents/source/skills/pr/shared/scripts/review.sh:125 selects the description-review skill. Its source at packages/agents/source/skills/review-pr-description/parts/body.md:43 says to flag counts of any kind, including tests and percentages. The writer repeats the count prohibition at packages/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:11 asks 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:362 checks 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.md exited 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:173 checks the draft-bound stamp, and line 194 reruns independent review before publication. packages/agents/source/skills/agent-pr/shared/scripts/publish.sh:5 uses 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

  1. 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.
  2. 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.
  3. 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.
  4. 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.
  5. 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-check and mise run cover-py-check green, 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-tokens and apm install in that order; run mise run check-generated-skills and mise run check. Inspect the consumer note and compatibility declaration against docs/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

Activity

  1. added theissue type on Nov 1, 2026
  2. tbhb-agent commented on Nov 1, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: NOT READY — DOD obligations are missing from the acceptance criteria and Verify plan

    Checked

    • origin identifies tbhb-dev/devtools; fetched origin/main is the cited commit 37f71fd4f085d8354eb6bd1803b57c4c0385830c.
    • Live issue Define an accepted D1 evidence format for agent pull requests #1045 is open, has type Defect, no labels, and no comments.
    • The Problem’s six findings are presented as unattributed transcript evidence. Its summary of what they support correctly limits the claim to invalid Command or Result entries, test counts, assertion walkthroughs, and diff narration. It expressly says they do not show a ban on a well-formed AC-to-command table. The blanket-count rules are at packages/agents/source/skills/review-pr-description/parts/body.md:43 and packages/agents/source/skills/write-pr-description/parts/write.md:35.
    • AC1 and AC2 define a mapping from each AC ID to a named test or command and an observed result or link. They allow comment evidence only when Verification cites the AC IDs and links the evidence, and require both the description review and D1 review to accept it. That placement and recognition condition is testable, though the D1 reviewer and acceptance result should be identified in the evidence recorded.
    • AC3’s contradiction cases and negative controls could test conflicting rules while retaining rejection of unsupported results and count padding. Focused consistency fixtures fit this repository’s existing script and fixture-based regression approach; using a stubbed PASS alone is explicitly ruled out.
    • AC4 preserves gates; AC5 covers generated variants and consumer guidance. No extra knob or layer is required by the ACs. The listed touched source and template paths are not in owner_paths or constitutional_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

    1. Blocking — DOD coverage: The ACs and Verify plan do not cover several checklist obligations, including exact merged-head approval and resolution of review findings, commit and squash signing and provenance, the provenance:agent-assisted label, 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

    • No split indicated; the focused instruction, template, regression, generated output, and guidance changes can fit one PR.

    Loosens: no

    Owner-tier: no

    Definition of done

    • D1 Not fully covered: AC1 and its Verify step cover per-AC evidence and merged-head confirmation, but not that each AC is verified on the merged head and cited in the PR’s Verification section.
    • D2 Not covered: no criterion requires independent approval of the exact merged head or resolution/follow-up of findings.
    • D3 Partly covered: AC4 preserves gates, but the Verify plan does not require the owner’s approval if a gate is loosened.
    • D4 Partly covered: AC4 preserves leak scans; its Verify step does not name the required scans or leak review.
    • D5 Not covered: no signing requirement for commits or squash.
    • D6 Partly covered: AC5 covers consumer guidance, but not the docs-match-or-“No doc impact” requirement.
    • D7 Not covered: no Assisted-by trailers or provenance:agent-assisted label.
    • D8 Not covered: size S is stated, but no hard-limit check or owner approval path is specified.
    • D9 Not covered: no follow-up issue handling or Related-section requirement.
    • D10 Not covered: no closing reference, board status, or final Status comment requirement.
    • defect-1 Covered: AC3 and its Verify step require regression fixtures and control cases.
    • devtools-1 Partly covered: AC4 calls for workflow suites and mise run check, but not mise run cover-go-check or mise run cover-py-check.
    • devtools-2 Covered: AC5 and its Verify step specify generation order and generated-output checks.
    • devtools-3 Covered: AC5 requires consumer guidance and checks it against compatibility guidance.

    Round: 1 of 3

  3. tbhb-agent commented on Nov 1, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: READY — Definition-of-Done coverage gap closed

    Checked

    • origin identifies tbhb-dev/devtools. origin/main is at 37f71fd4f085d8354eb6bd1803b57c4c0385830c.
    • Live issue Define an accepted D1 evidence format for agent pull requests #1045 is open, type Defect, size S. AC1–AC5 remain intact.
    • The diff adds the D1 reviewer acceptance result to AC1 and its Verify step; names coverage and leak scans in AC4; and adds the docs-match requirement in AC5. No other changes appear in the proposal diff.
    • The AC4 checks are available on origin/main: the shared task files define devtools:cover-go-check, devtools:cover-py-check, devtools:gitleaks, and devtools:lint-leaks; root task aliases provide the stated mise run commands.

    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

    • D1 AC1 and its Verify step cover per-AC evidence, the reviewer’s acceptance, and confirmation against the merged commit.
    • D2 Exact-head approval and review-finding handling remain delivery duties, not issue-specific outcomes.
    • D3 AC4 and its Verify step require preserving enforcement and inspecting gates for weakening or bypass.
    • D4 AC4 names the gitleaks and check-leaks scans.
    • D5 Signing remains a generic delivery duty.
    • D6 AC5 and its Verify step require docs to match or a reasoned “No doc impact.”
    • D7 Assisted-by trailers and provenance label remain generic delivery duties.
    • D8 Size handling remains a generic delivery duty.
    • D9 Follow-up issue handling remains a generic delivery duty.
    • D10 Issue closure and final status remain generic delivery duties.
    • defect-1 AC3 and its Verify step cover regression fixtures and negative controls.
    • devtools-1 AC4 and its Verify step name mise run check and both coverage threshold checks.
    • devtools-2 AC5 and its Verify step cover skill generation and generated-output checks.
    • devtools-3 AC5 and its Verify step cover consumer guidance and compatibility.

    Round: 2 of 3

  4. added
    refinement:readyPassed the Definition of Ready; may be picked up
    and removed
    refinement:activeA refinement round is open; not ready for implementation
    on Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    refinement:readyPassed the Definition of Ready; may be picked up

    Type

    Fields

    Priority

    None yet

    Severity

    None yet

    Work type

    None yet

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions