test(review): add guard-removal eval cases for quality-gate

plugins/review/.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "review",
-  "version": "0.42.0",
+  "version": "0.42.1",
   "description": "Code-review toolkit: nine agents, read-only over the reviewed code, plus orchestration skills for the quality gate, fan-out, and enforceability audit (/review:audit-enforceability), a pull-request change digest with an interactive view (/review:explain-change), a fan-out sweep workflow (/review:fanout-sweep), and CI lane commands (/review:code-review, /review:security-review) for org reusable workflows.",
   "author": {
     "name": "Melodic Software",

plugins/review/CHANGELOG.md

@@ -3,6 +3,12 @@
 All notable changes to the `review` plugin are documented here. Format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.
 
+## [0.42.1] - 2026-10-04
+
+### Added
+
+- **`review` ships a `claude plugin eval` suite with three cases for `/review:quality-gate` (tag `row37`).** Each case scaffolds a git branch whose commit removes a guard as a "simplification": an empty-list early return, a null-user check, and a length check that returned short titles unchanged. A case passes when the review names the input the removed guard handled and the behavior that now breaks for it. Each case has a regex grader for the input and a judge rubric for the regression, all with pass and fail samples. The cases need `--scaffold` and `--allow-tools Bash,Write`. No skill text changes.
+
 ## [0.42.0] - 2026-10-04
 
 ### Added

plugins/review/evals/guard-removal-empty-ratings/case.yaml

@@ -0,0 +1,4 @@
+schema_version: "1.1"
+name: guard-removal-empty-ratings
+context:
+  scaffold_script: scaffold.sh

plugins/review/evals/guard-removal-empty-ratings/graders/flags-regression.md

@@ -0,0 +1,7 @@
+---
+type: llm
+---
+
+PASS if the review reports, as a finding that needs action, that the change removed the handling for a product with no reviews (an empty reviews list), and says what now happens for that input: average_rating divides by zero and raises ZeroDivisionError, so the product page fails for such a product (for example "Cable tray" in products.json).
+
+FAIL if the review approves the change, treats the removed early return only as style or dead code, mentions that a guard or early return was removed without naming the empty-reviews input it handled, or names the input without saying the change now fails on it.

plugins/review/evals/guard-removal-empty-ratings/graders/names-empty-input.md

@@ -0,0 +1,5 @@
+---
+type: regex
+pattern: 'empty (list|reviews|review list|ratings)|no reviews|zero reviews|without (any )?reviews|reviews (is|are) empty|len\(reviews\) *(==|is) *0|"reviews": *\[\]'
+flags: i
+---

plugins/review/evals/guard-removal-empty-ratings/prompt.md

@@ -0,0 +1,11 @@
+---
+description: "Self review of a 'simplify' commit that drops the empty-reviews guard names the empty list and the crash it now causes"
+tags: [review, row37]
+runs: 3
+max_turns: 40
+timeout_seconds: 900
+allowed_tools: [Read, Glob, Grep, Skill, Agent, Bash, Write]
+expected_outcome: "The review reports, as a finding, that average_rating no longer handles a product with no reviews (an empty list), which now raises ZeroDivisionError on the product page"
+---
+
+/review:quality-gate self I finished a small cleanup on this branch. Review it before I open the PR.

plugins/review/evals/guard-removal-empty-ratings/samples/flags-regression.json

@@ -0,0 +1,18 @@
+{
+  "pass": [
+    {
+      "answer": "| 1 | IMPORTANT | Correctness | The refactor removed the `if not reviews: return 0.0` branch. A product with an empty reviews list (Cable tray in products.json) now hits `/ len(reviews)` with 0 and raises ZeroDivisionError, so product_page.py fails. | ratings.py:2 | Restore the empty-list return or handle it in the expression |",
+      "why": "names the input, the failure, and asks for action"
+    }
+  ],
+  "fail": [
+    {
+      "answer": "No self-review issues found in changed files. The one-expression version is easier to read.",
+      "why": "approves"
+    },
+    {
+      "answer": "| 1 | SUGGESTION | Style | The early return was removed; consider a docstring. | ratings.py:1 | Add docstring |",
+      "why": "mentions the removal as style, no input, no failure"
+    }
+  ]
+}

plugins/review/evals/guard-removal-empty-ratings/samples/names-empty-input.json

@@ -0,0 +1,34 @@
+{
+  "pass": [
+    {
+      "answer": "IMPORTANT: ratings.py:2 dropped the early return for an empty list, so a product with no reviews now raises ZeroDivisionError.",
+      "why": "names the empty list"
+    },
+    {
+      "answer": "The one-liner crashes for products without reviews, such as Cable tray in products.json.",
+      "why": "paraphrase: without reviews"
+    },
+    {
+      "answer": "When len(reviews) == 0 the division fails; restore the guard.",
+      "why": "names the input as a length check"
+    }
+  ],
+  "fail": [
+    {
+      "answer": "",
+      "why": "empty reply"
+    },
+    {
+      "answer": "No self-review issues found in changed files. The refactor is cleaner.",
+      "why": "approves"
+    },
+    {
+      "answer": "SUGGESTION: the `if not reviews` early return was removed; the single expression reads better.",
+      "why": "near miss: names the guard, not the input or the failure"
+    },
+    {
+      "answer": "Here is a summary of the release notes for version 2.3.",
+      "why": "answer to a different question"
+    }
+  ]
+}

plugins/review/evals/guard-removal-empty-ratings/scaffold.sh

@@ -0,0 +1,53 @@
+#!/usr/bin/env bash
+# main averages star ratings with an early return for a product that has no reviews yet;
+# the branch folds the function into one expression and drops that return.
+set -euo pipefail
+
+git init -q -b main
+git config user.name eval
+git config user.email eval@example.invalid
+
+cat >ratings.py <<'PY'
+def average_rating(reviews):
+    if not reviews:
+        return 0.0
+    total = sum(review["stars"] for review in reviews)
+    return round(total / len(reviews), 1)
+PY
+
+cat >product_page.py <<'PY'
+import json
+import sys
+
+from ratings import average_rating
+
+
+def render(product):
+    return "%s: %.1f stars" % (product["name"], average_rating(product["reviews"]))
+
+
+if __name__ == "__main__":
+    with open(sys.argv[1]) as handle:
+        for product in json.load(handle):
+            print(render(product))
+PY
+
+cat >products.json <<'JSON'
+[
+  {"name": "Desk lamp", "reviews": [{"stars": 4}, {"stars": 5}]},
+  {"name": "Cable tray", "reviews": []}
+]
+JSON
+
+git add .
+git commit -q -m "feat: product page with average rating"
+git remote add origin "$PWD"
+git fetch -q origin
+git remote set-head origin main >/dev/null
+
+git checkout -q -b refactor/ratings
+cat >ratings.py <<'PY'
+def average_rating(reviews):
+    return round(sum(review["stars"] for review in reviews) / len(reviews), 1)
+PY
+git commit -q -am "refactor: simplify average_rating to one expression"

plugins/review/evals/guard-removal-null-user/case.yaml

@@ -0,0 +1,4 @@
+schema_version: "1.1"
+name: guard-removal-null-user
+context:
+  scaffold_script: scaffold.sh

plugins/review/evals/guard-removal-null-user/graders/flags-regression.md

@@ -0,0 +1,7 @@
+---
+type: llm
+---
+
+PASS if the review reports, as a finding that needs action, that the change removed the handling for a missing user (session.user is null for a visitor who has not signed in) or a user with no profile, and says what now happens: displayName reads `.profile` of null or undefined and throws a TypeError, so renderHeader fails for that visitor. A review that also says the commit message's claim "every caller passes a signed-in user" is contradicted by src/header.js satisfies this, but the claim check alone is not required.
+
+FAIL if the review approves the change, accepts the commit message's claim that the branch was dead code, mentions that a check was removed without naming the null or signed-out input it handled, or names that input without saying the change now fails on it.

plugins/review/evals/guard-removal-null-user/graders/names-null-input.md

@@ -0,0 +1,5 @@
+---
+type: regex
+pattern: 'null(?! check)|undefined|anonymous|(signed|logged)[- ]out|not (yet )?(signed|logged) in|before (the visitor )?signs? in|without a profile|no profile|missing profile'
+flags: i
+---

plugins/review/evals/guard-removal-null-user/prompt.md

@@ -0,0 +1,11 @@
+---
+description: "Code review of a commit that drops a 'defensive' null check names the signed-out visitor (null user) the check handled"
+tags: [review, row37]
+runs: 3
+max_turns: 40
+timeout_seconds: 900
+allowed_tools: [Read, Glob, Grep, Skill, Agent, Bash, Write]
+expected_outcome: "The review reports that displayName no longer handles a null user or a user without a profile, contradicts the commit message's claim that every caller passes a signed-in user by pointing at renderHeader, and says the header now throws a TypeError for a signed-out visitor"
+---
+
+/review:quality-gate code Please review the change on this branch.

plugins/review/evals/guard-removal-null-user/samples/flags-regression.json

@@ -0,0 +1,18 @@
+{
+  "pass": [
+    {
+      "answer": "| 1 | CRITICAL | Correctness | The commit says every caller passes a signed-in user, but src/header.js:4 passes session.user, which is null until sign-in. With the `!user || !user.profile` check gone, displayName(null) throws TypeError reading `profile`, so the header breaks for every signed-out visitor. | src/display-name.js:2 | Restore the Guest fallback |",
+      "why": "names the input, refutes the claim, states the failure"
+    }
+  ],
+  "fail": [
+    {
+      "answer": "No issues found. The commit message explains the check was dead code, and the function is shorter now.",
+      "why": "accepts the dead-code claim"
+    },
+    {
+      "answer": "| 1 | SUGGESTION | Readability | A defensive check was removed; consider adding a JSDoc type for user. | src/display-name.js:1 | Add JSDoc |",
+      "why": "names the removal, not the input or the failure"
+    }
+  ]
+}

plugins/review/evals/guard-removal-null-user/samples/names-null-input.json

@@ -0,0 +1,34 @@
+{
+  "pass": [
+    {
+      "answer": "IMPORTANT: renderHeader passes session.user, which is null until the visitor signs in, so displayName now throws on `null.profile`.",
+      "why": "names null and signed-in state"
+    },
+    {
+      "answer": "A signed-out visitor now crashes the header: the removed check returned Guest for them.",
+      "why": "paraphrase: signed-out visitor"
+    },
+    {
+      "answer": "A user without a profile hits TypeError: Cannot read properties of undefined.",
+      "why": "names the missing-profile input"
+    }
+  ],
+  "fail": [
+    {
+      "answer": "",
+      "why": "empty reply"
+    },
+    {
+      "answer": "Looks good. Removing the null check simplifies displayName and the commit explains why.",
+      "why": "near miss: quotes the null check and approves"
+    },
+    {
+      "answer": "No issues found. The refactor is clean.",
+      "why": "approves"
+    },
+    {
+      "answer": "Here is a summary of the release notes for version 2.3.",
+      "why": "answer to a different question"
+    }
+  ]
+}

plugins/review/evals/guard-removal-null-user/scaffold.sh

@@ -0,0 +1,47 @@
+#!/usr/bin/env bash
+# main returns "Guest" for a missing user or profile; the branch removes that check on the
+# commit message's claim that every caller passes a signed-in user, which renderHeader contradicts.
+set -euo pipefail
+
+git init -q -b main
+git config user.name eval
+git config user.email eval@example.invalid
+mkdir -p src
+
+cat >src/display-name.js <<'JS'
+export function displayName(user) {
+  if (!user || !user.profile) {
+    return "Guest";
+  }
+  return user.profile.nickname || user.profile.fullName;
+}
+JS
+
+cat >src/header.js <<'JS'
+import { displayName } from "./display-name.js";
+
+// session.user stays null until the visitor signs in.
+export function renderHeader(session) {
+  return `<span class="who">${displayName(session.user)}</span>`;
+}
+JS
+
+cat >package.json <<'JSON'
+{ "name": "storefront-header", "type": "module", "private": true }
+JSON
+
+git add .
+git commit -q -m "feat: header shows the visitor's display name"
+git remote add origin "$PWD"
+git fetch -q origin
+git remote set-head origin main >/dev/null
+
+git checkout -q -b refactor/display-name
+cat >src/display-name.js <<'JS'
+export function displayName(user) {
+  return user.profile.nickname || user.profile.fullName;
+}
+JS
+git commit -q -am "refactor: drop defensive check in displayName
+
+Every caller passes a signed-in user, so the branch was dead code."

plugins/review/evals/guard-removal-short-title/case.yaml

@@ -0,0 +1,4 @@
+schema_version: "1.1"
+name: guard-removal-short-title
+context:
+  scaffold_script: scaffold.sh

plugins/review/evals/guard-removal-short-title/graders/flags-regression.md

@@ -0,0 +1,7 @@
+---
+type: llm
+---
+
+PASS if the review reports, as a finding that needs action, that the change removed the handling for titles at or under the limit, and says what now happens for them: shorten appends an ellipsis to a title that did not need shortening (for example "Desk lamp" becomes "Desk lamp…"), or a title of exactly the limit length loses its last character. The finding must reject the commit message's reasoning that slicing makes the check redundant, or at least show the output it now produces.
+
+FAIL if the review approves the change, agrees the check was redundant because slicing is safe, mentions that a length check was removed without naming the short titles it handled, or names short titles without saying how their output changes.

plugins/review/evals/guard-removal-short-title/graders/names-short-input.md

@@ -0,0 +1,5 @@
+---
+type: regex
+pattern: 'short(er)? (titles?|strings?|inputs?|names?)|titles? (shorter|under|within|at or under|no longer)|(fit|fits|already fits) (within|in|under)|(within|under|at or below|at or under) the limit|<= ?limit|less than (or equal to )?(the )?limit|exactly (40|the limit)|already short|Desk lamp|Cable tray'
+flags: i
+---

plugins/review/evals/guard-removal-short-title/prompt.md

@@ -0,0 +1,11 @@
+---
+description: "Self review of a commit that drops a 'redundant' length check names the titles at or under the limit that now get an ellipsis"
+tags: [review, row37]
+runs: 3
+max_turns: 40
+timeout_seconds: 900
+allowed_tools: [Read, Glob, Grep, Skill, Agent, Bash, Write]
+expected_outcome: "The review reports that shorten no longer returns a title at or under the limit unchanged: short titles such as 'Desk lamp' now come back with a trailing ellipsis, and a title of exactly 40 characters loses its last character"
+---
+
+/review:quality-gate self Quick tidy-up on this branch, can you give it a review pass?

plugins/review/evals/guard-removal-short-title/samples/flags-regression.json

@@ -0,0 +1,18 @@
+{
+  "pass": [
+    {
+      "answer": "| 1 | IMPORTANT | Correctness | Slicing is safe, but the removed `len(title) <= limit` branch returned short titles unchanged. Now every title gets the ellipsis: listing.py prints 'Desk lamp…' and 'Cable tray…', and a title of exactly 40 characters is cut to 39 plus the ellipsis. | titles.py:2 | Restore the early return |",
+      "why": "names the input class and the changed output"
+    }
+  ],
+  "fail": [
+    {
+      "answer": "No self-review issues found in changed files. Python slicing already handles short strings.",
+      "why": "agrees with the commit"
+    },
+    {
+      "answer": "| 1 | SUGGESTION | Style | The length check was removed; add a type hint for limit. | titles.py:1 | Add hint |",
+      "why": "names the removal as style only"
+    }
+  ]
+}

plugins/review/evals/guard-removal-short-title/samples/names-short-input.json

@@ -0,0 +1,34 @@
+{
+  "pass": [
+    {
+      "answer": "IMPORTANT: short titles now get an ellipsis too: shorten('Desk lamp') returns 'Desk lamp…'.",
+      "why": "names short titles"
+    },
+    {
+      "answer": "A title that already fits within the limit is no longer returned unchanged.",
+      "why": "paraphrase: fits within"
+    },
+    {
+      "answer": "Without the len(title) <= limit branch, a 40-character title loses its last character.",
+      "why": "names the boundary input"
+    }
+  ],
+  "fail": [
+    {
+      "answer": "",
+      "why": "empty reply"
+    },
+    {
+      "answer": "No self-review issues found in changed files. Slicing is safe, so the length check was redundant.",
+      "why": "agrees with the commit"
+    },
+    {
+      "answer": "SUGGESTION: the length check was removed; consider a docstring for shorten.",
+      "why": "near miss: names the check, not the input"
+    },
+    {
+      "answer": "Here is a summary of the release notes for version 2.3.",
+      "why": "answer to a different question"
+    }
+  ]
+}