From 15caf929c00b6a95f3fc01ca41e2899f8b5ccfa6 Mon Sep 17 00:00:00 2001 From: Anthony Casagrande Date: Mon, 6 Jul 2026 15:17:28 -0700 Subject: [PATCH] docs(skills): aiperf-code-review clones the PR to a temp dir with per-PR artifacts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review always runs from a fresh temp-directory clone of the PR head — never the user's working tree — diffed against its merge-base with origin/main (three-dot, matching the diff GitHub shows on the PR). The living document and runtime receipts are scoped to artifacts/pr-/ in the invoking repo instead of loose top-level paths. One escape hatch: on explicit user request only, in-place mode reviews uncommitted/unpushed code that a clone cannot see, diffing merge-base to the working tree without mutating it. In that mode the deliverable is the living document; inline PR comments require an explicit ask and all reviewed changes on the PR head, since comments cannot anchor to unpushed lines. Co-Authored-By: Claude Fable 5 Signed-off-by: Anthony Casagrande --- .agents/skills/aiperf-code-review/SKILL.md | 50 ++++++++++++++++------ 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/.agents/skills/aiperf-code-review/SKILL.md b/.agents/skills/aiperf-code-review/SKILL.md index d0d1954138..f3d04703e4 100644 --- a/.agents/skills/aiperf-code-review/SKILL.md +++ b/.agents/skills/aiperf-code-review/SKILL.md @@ -1,22 +1,47 @@ --- name: aiperf-code-review -description: Review the current branch against origin/main, capture findings in artifacts/code-review.md as a living document, validate every finding against the actual code, reproduce confirmed issues with the aiperf CLI against the in-repo mock server, and draft inline GitHub PR review comments anchored to specific files and lines. Use when the user asks for a branch or PR code review. +description: Clone the PR under review into a temp directory (or, on explicit request, review uncommitted/unpushed code in-place in the current workspace), review it against its merge-base with origin/main, capture findings in artifacts/pr-/code-review.md as a living document, validate every finding against the actual code, reproduce confirmed issues with the aiperf CLI against the in-repo mock server, and draft inline GitHub PR review comments anchored to specific files and lines. Use when the user asks for a branch or PR code review. --- - Review the current branch against `origin/main`, then carry the whole task through end-to-end without stopping at analysis. + Review a PR (or the current branch's PR) against its merge-base with `origin/main`, then carry the whole task through end-to-end without stopping at analysis. + + Setup — ALWAYS review from a fresh temp-directory clone: + - Resolve the PR under review: use the PR number the user gave; otherwise run `gh pr view --json number,headRefName,url` on the current branch. + - Derive the PR identifier `` as `pr-` (e.g. `pr-482`). If no PR exists yet, fall back to `branch-` where `` is the branch name with every character outside `[A-Za-z0-9._-]` (including `/`) replaced by `-` (e.g. `ajc/new-feature` -> `branch-ajc-new-feature`). `` is interpolated into filesystem paths and must never contain a path separator. + - Clone into a temp directory — never review, build, or reproduce in the user's working tree: + - `tmp=$(mktemp -d "${TMPDIR:-/tmp}/aiperf-review--XXXXXX")` (respects macOS's per-user `$TMPDIR`; works with GNU and BSD mktemp) + - Clone the repo into `"$tmp/aiperf"` (a local clone of the invoking repo followed by `git fetch origin` is fine for speed). + - Check out the PR head inside the clone: `gh pr checkout `, or `git fetch origin "pull//head" && git checkout FETCH_HEAD`. + - Set up the clone's environment with `make first-time-setup` before any runtime reproduction. + - All code reading, diffing, and reproduction happens inside the clone. The user's working tree stays untouched. Never skip the clone on your own judgment — even if the current working tree already has the PR branch checked out. The only exception is the explicit in-place mode below. + + In-place mode — reviewing uncommitted/unpushed code, explicit user request only: + - Purpose: review work that exists only in the invoking workspace — uncommitted changes and/or local commits not pushed to any remote. A temp clone cannot see this code, so the clone is skipped. + - Applies only when the user explicitly asks to review the current workspace (wording like "in place", "my working tree", "my uncommitted/unpushed changes", "don't clone"). Convenience, speed, or "the branch is already checked out" are never triggers — if the code under review is pushed, use the clone. + - Review the invoking working tree exactly as it stands, including uncommitted changes. Do not switch branches, check out refs, or otherwise mutate the tree. + - Diff basis in this mode: `git diff "$(git merge-base origin/main HEAD)"` — the working tree against the merge-base, so staged, unstaged, and unpushed committed work are all included. Do NOT use `git diff origin/main...` here: the omitted side resolves to HEAD, silently dropping uncommitted changes. Untracked files appear in no diff — check `git status --short` and review new files directly. Runtime reproduction also runs from the working tree. + - Record in the living document that the review ran in-place: the `git rev-parse HEAD` commit and whether the tree was dirty. + - `` is `pr-` if the branch has an open PR, otherwise the branch slug. Artifact paths are unchanged — everything still goes under `artifacts//`. + - The GitHub deliverable does not apply to code that is not pushed: inline comments cannot anchor to uncommitted/unpushed lines. In this mode the deliverable is the living document; only draft PR comments if the user asks and all reviewed changes are actually on the PR head. + + Artifact locations — everything is scoped to a per-PR subdirectory: + - The artifact root is `/artifacts//` in the repo where the skill was invoked, NOT inside the temp clone, so results survive clone cleanup. + - Living document: `artifacts//code-review.md`. + - Runtime receipts: `artifacts//repro-runtime-YYYYMMDD/`. + - Never write review artifacts loose at the top level of `artifacts/`. Goals: - 1. Collect the review findings for the branch relative to `origin/main`. - 2. Write them into `artifacts/code-review.md` as a living document. - 3. Validate every finding against the actual current code. + 1. Collect the review findings for the PR relative to its merge-base with `origin/main` (`git diff origin/main...HEAD`, matching the diff GitHub shows on the PR). + 2. Write them into `artifacts//code-review.md` as a living document. + 3. Validate every finding against the actual code in the clone. 4. Assign practical severity to each issue. - 5. Reproduce the confirmed issues with the real `aiperf` CLI against the in-repo mock server. - 6. Keep runtime receipts under `artifacts/`. + 5. Reproduce the confirmed issues with the real `aiperf` CLI against the in-repo mock server, run from the clone. + 6. Keep runtime receipts under `artifacts//`. 7. Update the living document with both source-level and runtime evidence. 8. Draft inline GitHub PR review comments anchored to the exact file and line of each finding, plus a short top-level summary comment. Requirements: - - Treat `artifacts/code-review.md` as a living document. Update it in place if it already exists. + - Treat `artifacts//code-review.md` as a living document. Update it in place if it already exists; re-reviews of the same PR reuse the same subdirectory. - For each finding, record: - status: `Confirmed`, `Partially confirmed`, or `Not confirmed` - source-level evidence with exact file paths and line references @@ -27,15 +52,15 @@ description: Review the current branch against origin/main, capture findings in - Use the real codebase, not assumptions. - If a finding is not valid, say so explicitly and explain why. - If a finding is only partially valid, narrow it precisely. - - Reproduce with the real `aiperf` binary and the in-repo mock server on a random localhost port. + - Reproduce with the real `aiperf` binary and the in-repo mock server on a random localhost port, both running from the review checkout (the temp clone, or the working tree in in-place mode). - Run outside the sandbox when needed and ask for approval through the normal tool flow. - - Save all receipts under a dedicated directory such as `artifacts/repro-runtime-YYYYMMDD/`. + - Save all receipts under `artifacts//repro-runtime-YYYYMMDD/` in the invoking repo, copying them into place if they were produced elsewhere (e.g. inside the temp clone). - Keep logs, command outputs, relevant generated files, and small summaries that make the proof easy to inspect. - If MLflow reproduction is needed, use a local SQLite MLflow backend so unrelated MLflow filesystem-store issues do not pollute the validation. - Do not overwrite unrelated user changes. - Do not stop after gathering evidence; finish by updating the document, then present the planned GitHub comments to the user for confirmation before posting. - GitHub deliverable: + GitHub deliverable (clone-mode reviews of pushed code only — in in-place mode the living document is the deliverable, see In-place mode above): - Post inline review comments using the GitHub PR review API (`gh api repos/{owner}/{repo}/pulls/{number}/reviews`). - Each confirmed finding gets its own inline comment anchored to the relevant file path and diff line number. - Include a short top-level summary in the review body covering: fix order, overall assessment, and what is working well. @@ -47,5 +72,6 @@ description: Review the current branch against origin/main, capture findings in - Keep it concise. - Tell me where the living document is. - Tell me where the receipts are. + - Tell me the temp clone path (it can be deleted once the review is accepted). - Tell me the GitHub review URL. - - Mention any caveats encountered during reproduction. \ No newline at end of file + - Mention any caveats encountered during reproduction.