Fix reviewer reliability and benchmark calibration

agents/api-security-reviewer.md

@@ -35,6 +35,8 @@ Skip if only files outside those roots (e.g. `review.frontendRoots`) are modifie
 - Skip node_modules, generated files, config files
 - Minimal output - let scripts report results
 - Read skill file for detailed rules
+- Checklist `--fail` means the finding must block this commit. Mark non-blocking observations
+  `--pass` (you may mention them in prose); never return PASS with a failed checklist item.
 - **Issue tracking (opt-in, default OFF):** Only when `guard.config.json` has `review.shortcutTracking: true` — before reporting FAIL, check the configured tracker for an existing tracking story. If the finding is already tracked, do not FAIL; report as TRACKED: <brief> | story:<id>. When the toggle is absent or false, skip this and report findings normally.
 </general_rules>
 

agents/backend-performance-reviewer.md

@@ -34,6 +34,8 @@ Skip if only files outside those roots (e.g. `review.frontendRoots`) are modifie
 - Skip node_modules, generated files, config files
 - Minimal output - let scripts report results
 - Read skill file for detailed rules
+- Checklist `--fail` means the finding must block this commit. Mark non-blocking observations
+  `--pass` (you may mention them in prose); never return PASS with a failed checklist item.
 - **Issue tracking (opt-in, default OFF):** Only when `guard.config.json` has `review.shortcutTracking: true` — before reporting FAIL, check the configured tracker for an existing tracking story. If the finding is already tracked, do not FAIL; report as TRACKED: <brief> | story:<id>. When the toggle is absent or false, skip this and report findings normally.
 </general_rules>
 

agents/frontend-performance-reviewer.md

@@ -32,9 +32,19 @@ Skip if only files outside those roots (e.g. `review.backendRoots`) are modified
 - Skip node_modules, generated files, config files
 - Minimal output - let scripts report results
 - Read skill file for detailed rules
+- Checklist `--fail` means the finding must block this commit. Mark non-blocking observations
+  `--pass` (you may mention them in prose); never return PASS with a failed checklist item.
 - **Issue tracking (opt-in, default OFF):** Only when `guard.config.json` has `review.shortcutTracking: true` — before reporting FAIL, check the configured tracker for an existing tracking story. If the finding is already tracked, do not FAIL; report as TRACKED: <brief> | story:<id>. When the toggle is absent or false, skip this and report findings normally.
 </general_rules>
 
+<calibration>
+FAIL only when the staged delta has a concrete performance consequence: name the hot path or
+load path, the repeated/expensive work or resource cost, and why the change makes that cost worse.
+Stale behavior, incompatible signatures, state-machine bugs, and functional regressions are
+correctness/completeness findings, not performance findings. Mark their performance checklist
+items PASS unless you can independently demonstrate a performance consequence.
+</calibration>
+
 <workflow>
 
 ## 1. Read skill for detailed rules:

agents/frontend-security-reviewer.md

@@ -32,6 +32,8 @@ Skip if only files outside those roots (e.g. `review.backendRoots`) are modified
 - Skip node_modules, generated files, config files
 - Minimal output - let scripts report results
 - Read skill file for detailed rules
+- Checklist `--fail` means the finding must block this commit. Mark non-blocking observations
+  `--pass` (you may mention them in prose); never return PASS with a failed checklist item.
 - **Issue tracking (opt-in, default OFF):** Only when `guard.config.json` has `review.shortcutTracking: true` — before reporting FAIL, check the configured tracker for an existing tracking story. If the finding is already tracked, do not FAIL; report as TRACKED: <brief> | story:<id>. When the toggle is absent or false, skip this and report findings normally.
 </general_rules>
 

gate-engine/review/__tests__/reviewer-eval.test.mts

@@ -4,6 +4,7 @@ import { describe, expect, it, vi } from 'vitest';
 import {
   BENCH_REVIEWERS,
   compareReviewer,
+  enforceCorpusMinimums,
   lintRows,
   makeSpyExec,
   runRow,
@@ -93,6 +94,28 @@ describe('lintRows', () => {
   });
 });
 
+describe('enforceCorpusMinimums', () => {
+  const corpus = () =>
+    Array.from({ length: 25 }, (_, i) =>
+      i < 13
+        ? goldRow({ id: `gold-${i}`, holdout: i < 3 })
+        : decoyRow({ id: `decoy-${i}`, holdout: i < 16 }),
+    );
+
+  it('requires 25 rows with at least three held-out golds and decoys', () => {
+    expect(() => enforceCorpusMinimums(corpus(), 'api-security-reviewer')).not.toThrow();
+    expect(() => enforceCorpusMinimums(corpus().slice(0, 24), 'api-security-reviewer')).toThrow(
+      /at least 25 rows/,
+    );
+    expect(() =>
+      enforceCorpusMinimums(
+        corpus().map((row) => ({ ...row, holdout: row.expected === 'PASS' && row.holdout })),
+        'api-security-reviewer',
+      ),
+    ).toThrow(/held-out FAIL/);
+  });
+});
+
 describe('makeSpyExec', () => {
   it('short-circuits the escalate pass when cascade is off — zero delegate calls', async () => {
     const capture = [];

gate-engine/review/__tests__/reviewers.test.mts

@@ -371,6 +371,8 @@ describe('wrapPrompt / escalatePrompt / stripFrontmatter', () => {
     expect(p).toContain('src/main/a.ts');
     expect(p).toContain('node .claude/skills/api-security/scripts/checklist.mjs generate');
     expect(p).toContain('check-item <name> --pass');
+    expect(p).toContain('`--fail` means the finding must block THIS commit');
+    expect(p).toContain('Non-blocking observations MUST be marked `--pass`');
     expect(p).toContain('Do NOT run the `cleanup` step');
   });
   it('lets the packaged brief own enumeration and rewrites its skill paths in review mode', () => {

gate-engine/review/__tests__/run-review.test.mts

@@ -323,6 +323,84 @@ describe('runReviewGate — cascade + exit contract', () => {
     expect(Object.keys(loadCache(repo))).toHaveLength(5);
   });
 
+  it('ordinary commit retries a broken checklist contract once when synced assets exist', async () => {
+    const repo = consumerRepo({ backend: true });
+    syncSkillAssets(repo);
+    const sink = join(repo, 'events.jsonl');
+    process.env.DEVKIT_GATE_EVENTS = sink;
+    process.env.DEVKIT_SHIP_ID = 'ship-contract-retry';
+    const attempts = new Map<string, number>();
+    const exec = mkExec(async ({ label, args }) => {
+      const attempt = (attempts.get(label) ?? 0) + 1;
+      attempts.set(label, attempt);
+      if (label === 'review:api-security-reviewer' && attempt === 1) {
+        writeArtifact(repo, label, { pending: 1 });
+        return 'first pass skipped one checklist item\nVERDICT: PASS';
+      }
+      if (label === 'review:api-security-reviewer') {
+        expect(args[1]).toContain('CHECKLIST-CONTRACT RETRY');
+        expect(existsSync(join(repo, reviewerFromLabel(label).stateFile))).toBe(false);
+      }
+      writeArtifact(repo, label);
+      return 'verified pass\nVERDICT: PASS';
+    });
+
+    expect(await runReviewGate(repo, { exec })).toBe(0);
+    expect(attempts.get('review:api-security-reviewer')).toBe(2);
+    expect(Object.keys(loadCache(repo))).toHaveLength(5);
+    const apiResult = readFileSync(sink, 'utf8')
+      .trim()
+      .split('\n')
+      .map((line) => JSON.parse(line))
+      .find(
+        (event) => event.type === 'review_result' && event.reviewer === 'api-security-reviewer',
+      );
+    expect(apiResult.status).toBe('pass');
+    expect(apiResult.item_tally).toEqual({ pass: 1 });
+    expect(readFileSync(join(repo, apiResult.transcript_ref), 'utf8')).toContain(
+      'CHECKLIST-CONTRACT RETRY',
+    );
+  });
+
+  it('ordinary commit preserves asset-sync inconclusive handling when a checklist asset is absent', async () => {
+    const repo = consumerRepo({ backend: true });
+    syncSkillAssets(repo);
+    rmSync(join(repo, '.claude', 'skills', 'api-security', 'scripts', 'checklist.mjs'), {
+      force: true,
+    });
+    const attempts = new Map<string, number>();
+    const exec = mkExec(async ({ label }) => {
+      attempts.set(label, (attempts.get(label) ?? 0) + 1);
+      if (label !== 'review:api-security-reviewer') writeArtifact(repo, label);
+      return 'VERDICT: PASS';
+    });
+
+    expect(await runReviewGate(repo, { exec })).toBe(2);
+    expect(attempts.get('review:api-security-reviewer')).toBe(1);
+    expect(Object.keys(loadCache(repo))).toHaveLength(4);
+  });
+
+  it('ordinary commit keeps a persistent PASS/failed-item mismatch inconclusive after one retry', async () => {
+    const repo = consumerRepo({ backend: true });
+    syncSkillAssets(repo);
+    const err = vi.spyOn(console, 'error').mockImplementation(() => {});
+    const attempts = new Map<string, number>();
+    const exec = mkExec(async ({ label }) => {
+      attempts.set(label, (attempts.get(label) ?? 0) + 1);
+      if (label === 'review:api-security-reviewer') writeArtifact(repo, label, { failed: 1 });
+      else writeArtifact(repo, label);
+      return 'VERDICT: PASS';
+    });
+
+    expect(await runReviewGate(repo, { exec })).toBe(2);
+    expect(attempts.get('review:api-security-reviewer')).toBe(2);
+    const out = err.mock.calls.flat().join('\n');
+    expect(out).toContain('api-security-reviewer — INCONCLUSIVE');
+    expect(out).toContain('FAILED item(s) but the verdict says PASS');
+    expect(out).not.toContain('api-security-reviewer REVIEW ERROR');
+    expect(Object.keys(loadCache(repo))).toHaveLength(4);
+  });
+
   it('review mode reports a repeated checklist-contract violation as an error, never inconclusive', async () => {
     const repo = consumerRepo({ backend: true });
     const assets = reviewAssets();

gate-engine/review/eval/reviewers/README.md

@@ -56,14 +56,12 @@ BENCH_MODEL=opus node bench.mts run                            # opus ceiling, r
 
 ## Corpus
 
-One JSONL file per reviewer (`cases-<skill>.jsonl`). The four **domain** reviewers carry 13–14
-rows each (api-security 14, the rest 13): the original 12 — 6 gold seeded-bugs (distinct catalog
-items; 3 clear / 2 borderline / 1 adversarial), 3 clean decoys (trigger ≥2 checklist items,
-genuinely fine), 2 near-miss decoys (look vulnerable, provably safe), 1 minimal pair (`variantOf`:
-the fixed twin of a gold row, expected PASS) — plus gold rows for the licensed-source catalog
-refresh items (`mass-assignment`, `object-level-authz`, `sync-io`, `layout-thrash`,
-`postmessage-origin`; `command-injection` reuses its retagged original row). 2 rows per file are
-`holdout: true` (excluded by `--dev`, included in baselines). Dataset-card fields per row:
+One JSONL file per reviewer (`cases-<skill>.jsonl`). Each of the four **domain** reviewers carries
+25 rows spanning gold bugs, clean decoys, near misses, and fixed minimal pairs. The July 2026
+calibration expansion adds production-derived frontend-performance false-positive decoys,
+the bounded backend cache-stampede decoy, and labeled coverage for the highest-frequency
+previously uncovered lenses. Every domain file has at least 3 held-out golds and 3 held-out decoys
+(`holdout: true`, excluded by `--dev`, included in baselines). Dataset-card fields per row:
 `note` (mandatory why), `difficulty`, `provenance` (`authored`/`mined`/`adapted` — mined rows are
 anonymized adaptations of real CodeRabbit/Macroscope findings from our PR history), `variantOf`.
 
@@ -257,4 +255,6 @@ Notes:
   frontend-performance SKILL.md softened the memoize-expensive-computation bar and
   `feperf-sku-rank-per-render` went caught → stable-missed (2-of-2); the bar was sharpened and
   the row recovered 2-of-2 (final re-baseline 13/13).
-- Corpus growth toward absolute-recall claims (≥25 rows/reviewer, mined-real, K=3) is sc-1147.
+- Domain corpus size and holdout floors are now ≥25 rows/reviewer with ≥3 held-out golds and
+  ≥3 held-out decoys. K=3 stability runs and continued mined-real growth remain required before
+  making absolute-recall claims (sc-1147).