From 933c48fb3ca47b571d1363d42f2ac5511d1d4975 Mon Sep 17 00:00:00 2001 From: Dmitry Dementiev Date: Fri, 15 May 2026 16:25:23 +0300 Subject: [PATCH] =?UTF-8?q?feat(skill):=20union-of-scopes=20=D0=B4=D0=B5?= =?UTF-8?q?=D1=82=D0=B5=D0=BA=D1=82=20=D0=B8=D0=B7=D0=BC=D0=B5=D0=BD=D0=B5?= =?UTF-8?q?=D0=BD=D0=B8=D0=B9=20=D0=B4=D0=BB=D1=8F=20code-=D1=80=D0=B5?= =?UTF-8?q?=D0=B6=D0=B8=D0=BC=D0=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Зачем: - типичный PR-флоу: ветка ahead of base + (часто) локальный твик поверх. Прежняя логика смотрела только unstaged/staged и пропускала коммиты ветки, давая пустое или неполное ревью. - Что: - Step 1 auto-detect: добавлен третий независимый scope `git diff ...HEAD` с резолвом base (`symbolic-ref refs/remotes/origin/HEAD` → `origin/main` → `origin/master` → `main` → `master`). - Step 2 dispatch: briefing для `code` и `code-vs-plan` переписан с fallback на union трёх скоупов, явное "do not skip branch-vs-base because working tree is non-empty". - reviewer-prompt.md: `{ARTIFACT}` placeholder теперь спеллует все три scope (a)(b)(c) с инструкцией про labeling mixed-state. - README.md: таблица "Three modes" и пояснение синхронизированы с union-семантикой. - Проверка: - на ветке feat/cross-platform-receiving-review одновременно непустые `git diff` и `git diff origin/main...HEAD` — финальная формулировка прошла Codex review (3 раунда, VERDICT: APPROVED). --- README.md | 17 ++++++++++++----- SKILL.md | 25 ++++++++++++++++++++++--- reviewer-prompt.md | 24 ++++++++++++++++++++++-- 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 1934ead..a033dcc 100644 --- a/README.md +++ b/README.md @@ -73,14 +73,21 @@ runs without operator access conclude unverified. ### Three modes -| Mode | What it reviews | When to use | -|----------------|------------------------------------|--------------------------| -| `plan` | Implementation plan | Before writing code | -| `code` | Git diff (unstaged, staged, branch)| After writing code | -| `code-vs-plan` | Code changes against the plan | Verify implementation | +| Mode | What it reviews | When to use | +|----------------|------------------------------------------|--------------------------| +| `plan` | Implementation plan | Before writing code | +| `code` | Working-tree, staged, and branch changes | After writing code | +| `code-vs-plan` | Code changes against the plan | Verify implementation | Mode is auto-detected from context, or you can force it with an argument. +For `code` mode, "branch changes" means commits on the current branch +since it diverged from its base (`main`, `master`, or the remote +default). The three scopes (unstaged, staged, branch-vs-base) are +reviewed as a union, not a fallback chain — so the typical PR case +(committed changes on the branch plus an in-progress local tweak) is +covered without an explicit flag. + ### Language The skill detects the operator's session language from recent diff --git a/SKILL.md b/SKILL.md index c61d72e..3f87f08 100644 --- a/SKILL.md +++ b/SKILL.md @@ -56,9 +56,20 @@ message "Plan mode is active" → mode = `plan`, skip git. **3. Auto-detect** (no explicit argument, not in Plan Mode): -1. Check for code changes (any non-empty output means changes exist): +1. Check for code changes — there are **three independent scopes**, and + any non-empty output from any of them means code changes exist: - `git diff --name-only` — unstaged - `git diff --cached --name-only` — staged + - `git diff --name-only ...HEAD` — committed on the current + branch since it diverged from its base. Resolve `` by trying + in order: `git symbolic-ref refs/remotes/origin/HEAD` (canonical + remote default), then `origin/main`, `origin/master`, `main`, + `master`. If none resolve, skip this check. + + The three scopes are independent, not a fallback chain. The typical + PR has commits on the branch *and* often an in-progress local tweak + on top; the briefing in Step 2 reviews the union, not just the first + non-empty one. 2. Check if a **structured plan artifact** exists in the current conversation context. This means one of: - The user pasted or referenced a markdown plan document with @@ -112,8 +123,16 @@ commands, and reads files on its own. **Do not pre-collect diffs or file lists for it.** Pass mode-specific context only: - For `plan` — path to plan file, or inline plan text -- For `code` — instruct it to use git status / git diff -- For `code-vs-plan` — pass the plan and instruct git lookup for changes +- For `code` — instruct the reviewer to review the **union of three + scopes**: unstaged (`git diff`), staged (`git diff --cached`), and + committed since base (`git diff ...HEAD`, base resolved the + same way as in Step 1). Branch-vs-base is *not* a fallback — do not + skip it just because the working tree is non-empty. The typical PR + has both. When scopes are mixed, the reviewer should label findings + by scope where it matters and surface the mixed-state to the + operator. +- For `code-vs-plan` — pass the plan and instruct the same union git + lookup as `code` (all three scopes, not a fallback chain) Fill the placeholders in `reviewer-prompt.md` and pass the result as the subagent's prompt. The template includes operating stance, finding bar, diff --git a/reviewer-prompt.md b/reviewer-prompt.md index ab5c5d1..aeb0e92 100644 --- a/reviewer-prompt.md +++ b/reviewer-prompt.md @@ -27,8 +27,28 @@ is a real weakness. {ARTIFACT} # For mode=plan: path to plan file, OR full inline plan text -# For mode=code: "Use git status / git diff to find changes." -# For mode=code-vs-plan: plan path or inline plan + "use git for code changes" +# For mode=code: "Find the changes via git. Review the UNION of +# three independent scopes — do not stop after +# the first non-empty one: +# (a) unstaged: `git diff` +# (b) staged: `git diff --cached` +# (c) branch-vs-base: resolve by +# trying `git symbolic-ref +# refs/remotes/origin/HEAD`, then +# `origin/main`, `origin/master`, `main`, +# `master`; then +# `git diff ...HEAD` plus +# `git log ..HEAD` for commit context. +# Do not skip (c) because (a) or (b) are +# non-empty — the typical PR has commits on the +# branch and an in-progress local tweak on top, +# and the operator is asking about both. If the +# scopes are mixed (e.g., committed changes plus +# unstaged ones), label findings by scope where +# it matters and surface the mixed state in the +# summary." +# For mode=code-vs-plan: plan path or inline plan + the same union git +# lookup instruction as mode=code ## Round