From b561e35b373d6f878f4c7bb515209123bdba01fa Mon Sep 17 00:00:00 2001 From: Kent Huang Date: Wed, 28 Oct 2026 15:34:14 +0800 Subject: [PATCH] Fix committed comparison input selection Signed-off-by: Kent Huang --- docs/architecture.md | 16 +++++ plugin/skills/behavior-diff/SKILL.md | 66 ++++++++++++------- .../references/scenario-and-test-data.md | 40 +++++++++-- 3 files changed, 93 insertions(+), 29 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 0a62d68..a82feaa 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -132,6 +132,22 @@ project data instead of generating more. It asks the user only when a material requirement cannot be reached otherwise. The task must not reveal the expected behavior. The instruction change must remain the only source of that guidance. +Input selection starts with the user's requested source: PR/Git revisions, then +explicit Before/After files, then local-edit discovery. Only local-edit discovery +requires an uncommitted change. Historical comparisons pin revisions and prepare +exact instruction snapshots outside the user's checkout, using the existing +`--before-file` mechanism. A clean checkout is valid. The surrounding project +context stays common to both variants; multi-file PR coverage must be established, +not inferred from a single instruction swap. + +```text +Requested comparison + -> PR/revisions: pin versions -> exact private historical inputs + -> Before/After files: use supplied inputs + -> local edit: discover changed instruction or report none + -> shared purpose/scenario/preflight/freeze/launch procedure +``` + Before launch, the skill preflights the real host environment and freezes the task, data, criteria, purpose, source revisions, and execution settings. It then runs the existing runner with the default trial counts; more edge cases add diff --git a/plugin/skills/behavior-diff/SKILL.md b/plugin/skills/behavior-diff/SKILL.md index 9f51d2b..e67042c 100644 --- a/plugin/skills/behavior-diff/SKILL.md +++ b/plugin/skills/behavior-diff/SKILL.md @@ -1,14 +1,15 @@ --- name: behavior-diff -description: Compare agent behavior before/after an uncommitted change to CLAUDE.md, AGENTS.md, or a skill file. Use for "behavior diff", "test my CLAUDE.md change", "did my rule change the agent's behavior", "before/after check on my rule edit". +description: Compare agent behavior before/after an instruction-file change from a local edit, PR, Git revisions, or explicit Before/After files. Use for "behavior diff", "test my CLAUDE.md change", "compare this PR", or "did my rule change the agent's behavior". --- # Behavior diff -Runs Behavior Diff on a local uncommitted instruction-file change. It starts -fresh headless agent trials with and without the change. The report shows the -git diff, a flow diff, and every trial's commands and final answer. There is -no automatic verdict. The user judges the evidence. +Compares local uncommitted edits, committed changes identified by a PR or Git +revisions, and explicit Before/After instruction files. It starts fresh headless +agent trials with each version. The report shows the instruction diff, a flow +diff, and every trial's commands and final answer. There is no automatic verdict. +The user judges the evidence. Preparation never modifies the user's checkout. ## Ownership @@ -70,16 +71,32 @@ the single-role or two-agent path. Create the fixtures with ## Steps -1. **Find the change.** In the user's current git repo, run - `git status --porcelain` and keep only modified instruction files - (CLAUDE.md, AGENTS.md, SKILL.md, agent-facing README.md). Exactly one - candidate: use it. Several: ask the user which one. None: explain that - the change must exist as an uncommitted edit first, and stop. - Then read `git diff -- ` so you understand what rule changed. - Read the selected instruction and its relevant unchanged references before - deriving the intent; a diff alone may omit behavior that must be preserved. - - Three cases have no git "before", and each still works: +1. **Resolve the requested comparison first.** Do not use working-tree status + as an eligibility check for every request. Select inputs in this order: + - **PR or Git revisions:** Resolve and pin the exact Before/After commits and + selected instruction path from Git or the forge. Honor an explicit baseline; + otherwise use the PR's merge base with its pinned head, or a single ordinary + commit's parent and that commit. Ask only if a material ambiguity remains + (such as which parent of a merge commit or which instruction to compare). + Read the historical preparation section in + `references/scenario-and-test-data.md` before preparing exact snapshots. + Use the existing `--before-file` route. **A clean working tree is valid.** + Never require an uncommitted edit or substitute unrelated working-tree files. + - **Explicit Before/After files:** Use the supplied versions with + `--file --before-file `, regardless of Git status. + - **Local edit:** Only when neither source above was requested, inspect + `git status --porcelain` for changed instruction files (CLAUDE.md, AGENTS.md, + SKILL.md, agent-facing README.md). Use one candidate; ask which if several. + If none exists and no explicit file/baseline applies below, report that no + local instruction change was found and stop. This stop applies only to + local-edit discovery, never to an explicitly requested historical comparison. + + Read the selected instruction, its diff and relevant unchanged references + before deriving intent; the diff alone can omit behavior to preserve. + For explicit files, use `diff `; for historical inputs use the + selected revisions, not the current working-tree diff. + + Additional file/baseline cases: - **Untracked file** (git does not know it — e.g. `~/.claude/CLAUDE.md`): the plugin's backup hook saved the pre-edit original under `${BEHAVIOR_DIFF_HOME:-~/.behavior-diff}/baselines/`, and the runner @@ -89,8 +106,6 @@ the single-role or two-agent path. Create the fixtures with right entry from that store and pass it as `--before-file`. If the runner exits saying no baseline exists, ask the user for the original content and pass it as `--before-file `. - - **User-given before/after files** ("compare A.md and B.md"): - run with `--file B.md --before-file A.md`. - **Non-git folder**: the runner builds both sandboxes from a plain copy of the working folder instead of a HEAD snapshot. Every trial copies the whole folder, so run from the smallest folder that holds @@ -114,11 +129,11 @@ the single-role or two-agent path. Create the fixtures with stores, resume other sessions, or infer access to missing turns. The same rule applies when orchestrating Pi or OMP. - For an already-supported historical comparison prepared with - `--before-file` or isolated replay fixtures, use the relevant selected commit - message and diff instead of the current session's motives. Do not use later - commits or trial results to invent the original reason. This does not add a - committed-edit invocation mode. + Historical comparisons use the relevant selected commit message and diff + instead of the current session's motives. Do not use later commits or trial + results to invent the original reason. There is no dedicated PR runner flag: + the agent resolves the requested PR into exact Before/After inputs using + `--before-file` or supported isolated replay fixtures. Write only the derived goals to a private scratch JSON file outside the project. Create the scratch directory with mode `0700` and the JSON file @@ -321,5 +336,8 @@ the single-role or two-agent path. Create the fixtures with - Never write, stage, stash, or commit shared data in the user's checkout. - The report quotes the user's real code. Never publish it anywhere without the user reviewing it first. -- Do not edit the user's instruction file yourself — this skill only - measures a change the user already made. +- Do not author, repair or alter the instruction change under test, or modify + the user's checkout to manufacture an uncommitted edit. Reading committed + versions and materializing byte-preserving copies in private scratch space + is permitted preparation. Preserve required companion files and record the + selected revisions; never weaken Before or silently repair After. diff --git a/plugin/skills/behavior-diff/references/scenario-and-test-data.md b/plugin/skills/behavior-diff/references/scenario-and-test-data.md index 4dd36da..15a1442 100644 --- a/plugin/skills/behavior-diff/references/scenario-and-test-data.md +++ b/plugin/skills/behavior-diff/references/scenario-and-test-data.md @@ -294,6 +294,36 @@ narrower experiment silently. Never write, stage, stash, or commit files in the user's checkout to make data reach the trials. +## Historical comparison inputs + +A PR or commit request supplies the comparison source; it does not require an +uncommitted edit. Resolve the source repository, exact revisions and selected +instruction path before local-edit discovery. Follow the source-isolation rule +above before deriving purpose or designing a scenario. + +Prepare a private scratch project from the pinned After revision, not the user's +working tree. Materialize the selected Before instruction from the pinned Before +revision into a separate file outside that project. Preserve exact bytes; pass it +as `--before-file` to the preparation helper. The scratch project's selected +instruction is After. Keep the original checkout, index, branch and unrelated +modifications untouched. Obtain source provenance from the original repository, +not the scratch project. No dedicated PR flag or synthetic local edit is needed. + +The ordinary runner compares one instruction while holding the surrounding +project constant. Retain references needed by either version and document the +common project context and any reviewed shared-file replacements. If companion +instructions also changed, do not claim that swapping one file tests the entire +PR. Use a supported content-preserving assembly or replay fixture with explicit +mapping and scope; if it cannot represent the requested comparison, report that +specific limitation rather than silently dropping changes. Do not substitute an +empty file for an absent historical instruction; use a supported absence fixture +or report that prerequisite. Missing revisions, ambiguous baselines and unsupported +capabilities are real blockers; a clean working tree is not. + +After resolving the inputs, continue the same purpose, scenario, preflight, +freeze, authorization and launch procedure. Historical preparation does not +authorize extra runs, private-data disclosure, source changes or PR operations. + ## Prepare the inputs 1. **Shared data.** Put the trial inputs in a data directory outside the @@ -337,11 +367,11 @@ python3 "$SKILL_DIR/scripts/prepare_shared.py" prepare \ Optional arguments: -- `--before-file ` supplies the Before content: a user-supplied original, - user-given before/after files, or one entry that you selected from the hook - baseline store. The helper always reads this file as content, even if its - name ends in `-ABSENT`. If the file is inside the project, the helper excludes - it from the stage automatically. +- `--before-file ` supplies the Before content: an exact file extracted + from the selected historical revision, a user-supplied original, explicit + Before/After files, or a selected hook baseline. The helper reads it as content, + even if its name ends in `-ABSENT`. If it is inside the project, the helper + excludes it from the stage automatically. - `--exclude ` keeps one project path out of the stage. Repeat the option for each path.