Skip to content

fix(skill-evals): correct blocked drop reason #1061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
apraba05 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(skill-evals): correct blocked drop reason #1061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions tools/skill-evals/evals/pr-management-quick-merge/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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`.
5 changes: 5 additions & 0 deletions ...quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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."
}
9 changes: 9 additions & 0 deletions ...merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->

PR #69001
Live GET /repos/apache/airflow/pulls/69001:
mergeable: true
mergeable_state: blocked
reviewDecision: APPROVED
approvals: 1
4 changes: 2 additions & 2 deletions ...als/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. |

Expand All @@ -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
Expand Down