From f5630efcec86fbca1018df45569a45c44ca997e3 Mon Sep 17 00:00:00 2001 From: Ashan Prapaharan <47048522+apraba05@users.noreply.github.com> Date: Sat, 1 Aug 2026 02:45:42 -0700 Subject: [PATCH 1/2] [verified] fix(skill-evals): correct blocked drop reason --- .../evals/pr-management-quick-merge/README.md | 10 +++++----- .../case-5-blocked-non-review-drop/expected.json | 5 +++++ .../fixtures/case-5-blocked-non-review-drop/report.md | 9 +++++++++ .../stage-3-merge-readiness/fixtures/system-prompt.md | 4 ++-- 4 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json create mode 100644 tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/report.md diff --git a/tools/skill-evals/evals/pr-management-quick-merge/README.md b/tools/skill-evals/evals/pr-management-quick-merge/README.md index 2e18ad31e..c16519bb2 100644 --- a/tools/skill-evals/evals/pr-management-quick-merge/README.md +++ b/tools/skill-evals/evals/pr-management-quick-merge/README.md @@ -5,13 +5,13 @@ Behavioral evals for the `pr-management-quick-merge` skill. -## Suites (20 cases total) +## Suites (21 cases total) | Suite | Step | Cases | What it covers | |---|---|---|---| | stage-1-quality-gate | Steps 1–2 (Stage 1 quality gates) | 8 | G2 (CI failure), G2 (real-CI not ran / bot-only SUCCESS), G3 (pending check), G4 (action_required workflow), G6 (unresolved collaborator thread), G7 (non-stale changes-requested), all-gates-pass, injection attempt ignored | | stage-2-triviality | Step 2 (Stage 2 triviality + tier) | 8 | Tier A (docs only), Tier B (tests only), Tier B (mixed docs+tests), too-large churn, too-many files, path-denied (.github/**), path-unmatched (unknown source path), deny overrides allow | -| stage-3-merge-readiness | Step 2 (Stage 3 live re-poll) | 4 | ready (clean state), needs-approval (blocked + REVIEW_REQUIRED), drop (conflict / dirty), drop (unknown / still computing) | +| stage-3-merge-readiness | Step 2 (Stage 3 live re-poll) | 5 | ready (clean state), needs-approval (blocked + REVIEW_REQUIRED), drop (blocked by a non-review requirement), drop (conflict / dirty), drop (unknown / still computing) | ## Run @@ -42,6 +42,6 @@ uv run --project tools/skill-evals skill-eval \ dropped as `path-denied`. - `stage-3-merge-readiness` exercises the live re-poll classification. The `blocked + REVIEW_REQUIRED` row (`case-2`) is the skill's primary case: it - routes to the `[A]pprove` action rather than dropping. `blocked` alone - (without `REVIEW_REQUIRED`) would be `gate:G5-conflict` but that case is - covered implicitly by `case-3` (`dirty`/`mergeable=false`). + routes to the `[A]pprove` action rather than dropping. `blocked` without + `REVIEW_REQUIRED` is explicitly covered by `case-5` and drops as `gate:G5`; + `case-3` reserves `gate:G5-conflict` for `dirty`/`mergeable=false`. diff --git a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json new file mode 100644 index 000000000..a323c7a68 --- /dev/null +++ b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json @@ -0,0 +1,5 @@ +{ + "bucket": "drop", + "drop_reason": "gate:G5", + "reason": "mergeable=true, mergeable_state=blocked, reviewDecision=APPROVED — the branch merges cleanly but the remaining block is not cleared by another approval." +} diff --git a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/report.md b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/report.md new file mode 100644 index 000000000..757596225 --- /dev/null +++ b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/report.md @@ -0,0 +1,9 @@ + + +PR #69001 +Live GET /repos/apache/airflow/pulls/69001: + mergeable: true + mergeable_state: blocked + reviewDecision: APPROVED + approvals: 1 diff --git a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md index a73c01aec..a9b5c4e76 100644 --- a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md +++ b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md @@ -18,7 +18,7 @@ Classify each candidate by the live `(mergeable, mergeable_state)` pair: | `mergeable == true`, `mergeable_state ∈ {clean, has_hooks}` | **ready** — surface with the merge command | | `mergeable == true`, `mergeable_state ∈ {unstable, behind}` | **ready** — note the state; `unstable` means a non-required check is still running but every required check is green; `behind` is a stale-but-clean branch GitHub will fast-forward | | `mergeable == true`, `mergeable_state == blocked` **and** `reviewDecision == REVIEW_REQUIRED` | **needs-approval** — the branch merges cleanly but a required committer review is missing; route to the `[A]pprove` action | -| `mergeable == true`, `mergeable_state == blocked` **and** `reviewDecision != REVIEW_REQUIRED` | **drop** — the block is not cleared by an approval; reason `gate:G5-conflict` (a non-approval required check is the blocker) | +| `mergeable == true`, `mergeable_state == blocked` **and** `reviewDecision != REVIEW_REQUIRED` | **drop** — the block is not cleared by an approval; reason `gate:G5` (a non-approval required check is the blocker) | | `mergeable == false` **or** `mergeable_state == dirty` | **drop** — genuine merge conflict; reason `gate:G5-conflict` | | `mergeable == null` **or** `mergeable_state == unknown` | **drop** — mergeability still computing; reason `gate:G5-unknown`; conservative per Golden rule 4. It will settle on the next run. | @@ -34,7 +34,7 @@ Return ONLY valid JSON: } ``` -- `drop_reason` is one of `gate:G5-conflict`, `gate:G5-unknown`, +- `drop_reason` is one of `gate:G5`, `gate:G5-conflict`, `gate:G5-unknown`, or `null` when `bucket != "drop"`. - `reason` is one concise sentence naming the classification outcome. For a `gate:G5-conflict` drop on a genuine merge conflict From 751d60ec3a7685d539a11b2650a2c41dfa633013 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Mon, 17 Aug 2026 15:06:06 +0200 Subject: [PATCH 2/2] fix(pr-management-quick-merge): name the blocked drop reason gate:G5-blocked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gate:G5` already means "failed Stage-1 gate G5" — the batch-level obviously-conflicting cull — so reusing it for the Stage-3 mergeable-but-blocked outcome gives one code two meanings, which is the conflation #949 set out to remove. Rename the Stage-3 code to `gate:G5-blocked` and add the row the authoritative drop-reason taxonomy in candidate-rules.md was missing, so the eval fixture and the skill agree on the code and on how it is reported. Generated-by: Claude Code (Opus 5) --- .../candidate-rules.md | 18 ++++++++++++------ .../evals/pr-management-quick-merge/README.md | 5 +++-- .../expected.json | 2 +- .../fixtures/system-prompt.md | 6 +++--- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/skills/pr-management-quick-merge/candidate-rules.md b/skills/pr-management-quick-merge/candidate-rules.md index e2c3eb129..15cf7b651 100644 --- a/skills/pr-management-quick-merge/candidate-rules.md +++ b/skills/pr-management-quick-merge/candidate-rules.md @@ -125,8 +125,8 @@ G2/G3 already established every check is green and done, so a `blocked` state here is review-required in the normal case. Where an adopter's branch protection makes a *non*-CI context required, confirm with `gh pr view --json reviewDecision` — `REVIEW_REQUIRED` ⇒ a missing approval -is the blocker (route to the approval bucket); any other decision ⇒ drop, the -block is not something an approval clears. +is the blocker (route to the approval bucket); any other decision ⇒ drop as +`gate:G5-blocked`, the block is not something an approval clears. --- @@ -235,14 +235,20 @@ Every screened-out PR carries exactly one drop reason, surfaced in the | `path-denied` | a changed file matched `deny_globs` (consequential area) | | `path-unmatched` | a changed file matched no allow glob (unknown area) | | `gate:G5-conflict` | Stage-3 live re-poll: genuine merge conflict (`mergeable == false` / `dirty`) | +| `gate:G5-blocked` | Stage-3 live re-poll: merges cleanly but `blocked` by a required context an approval does not clear (`reviewDecision != REVIEW_REQUIRED`) | | `gate:G5-unknown` | Stage-3 live re-poll: mergeability still uncomputed after the direct call — dropped this run, qualifies next | +Note that the Stage-3 codes are deliberately suffixed. A bare `gate:G5` means +"failed **Stage-1** gate G5" — the batch-level *obviously conflicting* cull — so +reusing it for a Stage-3 outcome would give one code two meanings. + `gate:*` and `gate:G5-unknown` drops are reported as a single count each (the maintainer rarely cares which one a non-ready-looking PR hit); `too-large`, -`path-denied`, `path-unmatched`, and `gate:G5-conflict` are reported with PR -numbers, because those are the "so-close" PRs a maintainer may want to glance at -or hand to `pr-management-code-review`. **Note:** a `BLOCKED` live state is -**not** a drop reason — it is the *approval* bucket (see +`path-denied`, `path-unmatched`, `gate:G5-conflict`, and `gate:G5-blocked` are +reported with PR numbers, because those are the "so-close" PRs a maintainer may +want to glance at or hand to `pr-management-code-review`. **Note:** a `BLOCKED` +live state with `reviewDecision == REVIEW_REQUIRED` is **not** a drop reason — +it is the *approval* bucket (see [Stage 3](#stage-3--live-merge-readiness)), the skill's primary output. --- diff --git a/tools/skill-evals/evals/pr-management-quick-merge/README.md b/tools/skill-evals/evals/pr-management-quick-merge/README.md index c16519bb2..3e4016b32 100644 --- a/tools/skill-evals/evals/pr-management-quick-merge/README.md +++ b/tools/skill-evals/evals/pr-management-quick-merge/README.md @@ -43,5 +43,6 @@ uv run --project tools/skill-evals skill-eval \ - `stage-3-merge-readiness` exercises the live re-poll classification. The `blocked + REVIEW_REQUIRED` row (`case-2`) is the skill's primary case: it routes to the `[A]pprove` action rather than dropping. `blocked` without - `REVIEW_REQUIRED` is explicitly covered by `case-5` and drops as `gate:G5`; - `case-3` reserves `gate:G5-conflict` for `dirty`/`mergeable=false`. + `REVIEW_REQUIRED` is explicitly covered by `case-5` and drops as + `gate:G5-blocked`; `case-3` reserves `gate:G5-conflict` for + `dirty`/`mergeable=false`. diff --git a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json index a323c7a68..d0454d24e 100644 --- a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json +++ b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json @@ -1,5 +1,5 @@ { "bucket": "drop", - "drop_reason": "gate:G5", + "drop_reason": "gate:G5-blocked", "reason": "mergeable=true, mergeable_state=blocked, reviewDecision=APPROVED — the branch merges cleanly but the remaining block is not cleared by another approval." } diff --git a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md index a9b5c4e76..fe0222f2e 100644 --- a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md +++ b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md @@ -18,7 +18,7 @@ Classify each candidate by the live `(mergeable, mergeable_state)` pair: | `mergeable == true`, `mergeable_state ∈ {clean, has_hooks}` | **ready** — surface with the merge command | | `mergeable == true`, `mergeable_state ∈ {unstable, behind}` | **ready** — note the state; `unstable` means a non-required check is still running but every required check is green; `behind` is a stale-but-clean branch GitHub will fast-forward | | `mergeable == true`, `mergeable_state == blocked` **and** `reviewDecision == REVIEW_REQUIRED` | **needs-approval** — the branch merges cleanly but a required committer review is missing; route to the `[A]pprove` action | -| `mergeable == true`, `mergeable_state == blocked` **and** `reviewDecision != REVIEW_REQUIRED` | **drop** — the block is not cleared by an approval; reason `gate:G5` (a non-approval required check is the blocker) | +| `mergeable == true`, `mergeable_state == blocked` **and** `reviewDecision != REVIEW_REQUIRED` | **drop** — the block is not cleared by an approval; reason `gate:G5-blocked` (a non-approval required context is the blocker) | | `mergeable == false` **or** `mergeable_state == dirty` | **drop** — genuine merge conflict; reason `gate:G5-conflict` | | `mergeable == null` **or** `mergeable_state == unknown` | **drop** — mergeability still computing; reason `gate:G5-unknown`; conservative per Golden rule 4. It will settle on the next run. | @@ -34,8 +34,8 @@ Return ONLY valid JSON: } ``` -- `drop_reason` is one of `gate:G5`, `gate:G5-conflict`, `gate:G5-unknown`, - or `null` when `bucket != "drop"`. +- `drop_reason` is one of `gate:G5-blocked`, `gate:G5-conflict`, + `gate:G5-unknown`, or `null` when `bucket != "drop"`. - `reason` is one concise sentence naming the classification outcome. For a `gate:G5-conflict` drop on a genuine merge conflict (`mergeable=false` / `mergeable_state=dirty`), the sentence must also name