From 165be3d158bb7769aaa60d26d3a58bf30158d34b Mon Sep 17 00:00:00 2001 From: Kris Hicks Date: Mon, 24 Aug 2026 11:07:54 -0700 Subject: [PATCH] docs(agents): clarify user-visible PR review feedback Agents will often give a ton of feedback, but it can be hard to quantify the impact of the issue the feedback is about. This change updates the PR review feedback template to be more human-readable, rooting any concerns in user-visible behavior when appropriate, and comparing the new behavior to old behavior so that PR authors can make a determination of whether they want to accept or reject the feedback. Signed-off-by: Kris Hicks --- .agents/skills/review-github-pr/SKILL.md | 27 ++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/.agents/skills/review-github-pr/SKILL.md b/.agents/skills/review-github-pr/SKILL.md index 1058bfb3e9..21cd131885 100644 --- a/.agents/skills/review-github-pr/SKILL.md +++ b/.agents/skills/review-github-pr/SKILL.md @@ -102,7 +102,9 @@ Read through the full diff (and the PR description if available). Produce a summ ### Potential Concerns <- omit if none -- +- **** — Before this PR, + experienced . With this PR, , so + . Details: `:`. ``` **Guidelines for the summary:** @@ -110,7 +112,28 @@ Read through the full diff (and the PR description if available). Produce a summ - **Overview**: State what changed and why. Pull context from the PR description if available. - **Key Design Decisions**: Focus on _why_ something was done a particular way, not _what_ changed. Include `file_path:line_number` references. Examples: choice of algorithm, new abstraction introduced, API contract change, migration strategy. - **Notable Code**: Include only the most instructive or surprising snippets. Keep each snippet under 15 lines. Always include the file path above the code block. -- **Potential Concerns**: Only include if there are genuine risks — missing error handling, breaking changes, performance implications, security issues. Do not fabricate concerns. +- **Potential Concerns**: Only include genuine risks that warrant a change or a + deliberate accept/reject decision. Describe each concern in terms of observable + behavior for the affected persona, such as a sandbox creator, sandbox user, + operator, administrator, SDK consumer, or developer maintaining the system. + Always compare the previous behavior with the new concerning behavior and state + the resulting user-visible impact. Prefer the compact form: "Before this PR, + `` experienced ``. With this PR, ``, so + ``." Add only the minimum file and line references needed to substantiate + the finding. + - Use the PR base as the normal previous-behavior baseline. Review older history + only when the change is fixing or extending an earlier feature and that history + is necessary to explain the behavioral contract. In that case, describe the + relevant transitions explicitly: "Before ``, ... After ``, ... + With this PR, ...". + - Translate internal failure modes and race conditions into what the affected + person would observe. Internal implementation details belong in the trailing + file and line references, not in place of the behavior description. + - Do not assign P0/P1/P2 or similar priority labels. The behavioral comparison + and impact should give maintainers enough context to accept or reject the + suggested change. + - Do not fabricate concerns or claim a behavioral regression without evidence + for both the prior and proposed behavior. - **Agent infrastructure**: When the PR changes behavior, commands, or development workflows, use the `sync-agent-infra` maintenance map to check that related skills were updated. When it adds, removes, or renames skills or crates; changes workflow relationships or skill coverage; modifies issue or PR templates; or changes agent cross-references, apply the full consistency checklist. Report missing companion updates or drift under **Potential Concerns**. ## Step 5: Output