From ab87548747c0b76a676f94cb75dbb623bb19f446 Mon Sep 17 00:00:00 2001 From: norvalbv Date: Thu, 30 Jul 2026 23:20:19 +0100 Subject: [PATCH] Fix reviewer reliability and benchmark calibration What - retry ordinary checklist-contract failures once when the synced asset exists and clean partial evidence before retrying - tighten domain reviewer blocking semantics and frontend performance scope - expand every domain benchmark to 25 cases with stronger held-out coverage Why Production telemetry mixed runtime contract failures with reviewer quality, while sparse corpora could not reliably distinguish false positives from useful findings. Impact This changes the live review gate and benchmark runners. Telemetry wire format and status vocabulary remain unchanged. Checks - npm test -- --run: 2911 passed - npm run typecheck - npm run build - benchmark validate and coverage - K=3 targeted production-derived decoys: 12/12 pass - focused prompt suite: 54 passed - duplication preflight: no new candidates --- agents/api-security-reviewer.md | 2 + agents/backend-performance-reviewer.md | 2 + agents/frontend-performance-reviewer.md | 10 +++ agents/frontend-security-reviewer.md | 2 + .../review/__tests__/reviewer-eval.test.mts | 23 ++++++ .../review/__tests__/reviewers.test.mts | 2 + .../review/__tests__/run-review.test.mts | 78 +++++++++++++++++++ gate-engine/review/eval/reviewers/README.md | 18 ++--- .../eval/reviewers/cases-api-security.jsonl | 11 +++ .../reviewers/cases-backend-performance.jsonl | 12 +++ .../cases-frontend-performance.jsonl | 12 +++ .../reviewers/cases-frontend-security.jsonl | 12 +++ gate-engine/review/eval/reviewers/corpus.mts | 22 +++++- gate-engine/review/reviewers.mts | 2 +- gate-engine/review/runtime.mts | 27 +++++-- skills/frontend-performance/SKILL.md | 11 ++- 16 files changed, 228 insertions(+), 18 deletions(-) diff --git a/agents/api-security-reviewer.md b/agents/api-security-reviewer.md index b0bddc4b..1b91cab6 100644 --- a/agents/api-security-reviewer.md +++ b/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. diff --git a/agents/backend-performance-reviewer.md b/agents/backend-performance-reviewer.md index a377a087..514f1cd2 100644 --- a/agents/backend-performance-reviewer.md +++ b/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. diff --git a/agents/frontend-performance-reviewer.md b/agents/frontend-performance-reviewer.md index 5cffab6a..ad2591f9 100644 --- a/agents/frontend-performance-reviewer.md +++ b/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. + +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. + + ## 1. Read skill for detailed rules: diff --git a/agents/frontend-security-reviewer.md b/agents/frontend-security-reviewer.md index 72271f0c..ac13b141 100644 --- a/agents/frontend-security-reviewer.md +++ b/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. diff --git a/gate-engine/review/__tests__/reviewer-eval.test.mts b/gate-engine/review/__tests__/reviewer-eval.test.mts index 33d3a893..13f60fc0 100644 --- a/gate-engine/review/__tests__/reviewer-eval.test.mts +++ b/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 = []; diff --git a/gate-engine/review/__tests__/reviewers.test.mts b/gate-engine/review/__tests__/reviewers.test.mts index c825f25c..cffc964f 100644 --- a/gate-engine/review/__tests__/reviewers.test.mts +++ b/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 --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', () => { diff --git a/gate-engine/review/__tests__/run-review.test.mts b/gate-engine/review/__tests__/run-review.test.mts index c7f48f1f..d4b61b5d 100644 --- a/gate-engine/review/__tests__/run-review.test.mts +++ b/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(); + 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(); + 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(); + 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(); diff --git a/gate-engine/review/eval/reviewers/README.md b/gate-engine/review/eval/reviewers/README.md index dd0c2b2c..034ccd50 100644 --- a/gate-engine/review/eval/reviewers/README.md +++ b/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-.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-.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). diff --git a/gate-engine/review/eval/reviewers/cases-api-security.jsonl b/gate-engine/review/eval/reviewers/cases-api-security.jsonl index 473a5816..58c0f7c1 100644 --- a/gate-engine/review/eval/reviewers/cases-api-security.jsonl +++ b/gate-engine/review/eval/reviewers/cases-api-security.jsonl @@ -12,3 +12,14 @@ {"id": "apisec-tagged-sql-nearmiss", "reviewer": "api-security-reviewer", "expected": "PASS", "repo": {"base": {"api/analytics.ts": "import postgres from 'postgres';\n\nconst sql = postgres(process.env.DATABASE_URL as string);\n\n// Roll up event counts over a fixed, trusted window.\nexport async function dailyCounts(): Promise<{ day: string; total: number }[]> {\n return sql`SELECT day, count(*) AS total FROM events GROUP BY day ORDER BY day`;\n}\n"}, "staged": {"api/analytics.ts": "import postgres from 'postgres';\n\nconst sql = postgres(process.env.DATABASE_URL as string);\n\n// Roll up event counts over a fixed, trusted window.\nexport async function dailyCounts(): Promise<{ day: string; total: number }[]> {\n return sql`SELECT day, count(*) AS total FROM events GROUP BY day ORDER BY day`;\n}\n\n// NOTE: ${tenantId} is interpolated into the tagged template, but this is NOT\n// string-built SQL — the postgres.js `sql` tag sends the value as a bound $1\n// parameter, never as query text, so it is injection-safe.\nexport async function countsForTenant(tenantId: string): Promise {\n const rows =\n await sql`SELECT count(*) AS total FROM events WHERE tenant_id = ${tenantId}`;\n return Number(rows[0].total);\n}\n"}}, "note": "countsForTenant interpolates tenantId into a postgres.js tagged template, which binds it as a $1 parameter rather than SQL text — pattern-matchers flag it but the library exonerates it. PASS.", "difficulty": "adversarial", "provenance": "authored", "variantOf": null, "holdout": false} {"id": "apisec-mass-assignment", "reviewer": "api-security-reviewer", "expected": "FAIL", "expectItems": ["mass-assignment"], "reasonPattern": "mass.?assign|allowlist|named field|spread|role|req\\.body", "repo": {"base": {"api/profile.ts": "import type { Request, Response } from 'express';\nimport { db } from './db';\n\n// Profile updates write named fields only — the row also carries role/orgId columns.\nexport async function updateDisplayName(req: Request, res: Response): Promise {\n const user = await db.user.update({\n where: { id: req.session.userId },\n data: { displayName: String(req.body.displayName ?? '').slice(0, 80) },\n });\n res.json({ id: user.id, displayName: user.displayName });\n}\n"}, "staged": {"api/profile.ts": "import type { Request, Response } from 'express';\nimport { db } from './db';\n\n// Profile updates write named fields only — the row also carries role/orgId columns.\nexport async function updateDisplayName(req: Request, res: Response): Promise {\n const user = await db.user.update({\n where: { id: req.session.userId },\n data: { displayName: String(req.body.displayName ?? '').slice(0, 80) },\n });\n res.json({ id: user.id, displayName: user.displayName });\n}\n\nexport async function updateProfile(req: Request, res: Response): Promise {\n const user = await db.user.update({\n where: { id: req.session.userId },\n data: { ...req.body },\n });\n res.json(user);\n}\n"}}, "note": "Spreading req.body into db.user.update lets a caller set role/orgId or any other column; the sibling handler shows the named-field pattern. Gold for the new mass-assignment item.", "difficulty": "clear", "provenance": "authored", "variantOf": null, "holdout": false} {"id": "apisec-idor-lookup", "reviewer": "api-security-reviewer", "expected": "FAIL", "expectItems": ["object-level-authz"], "reasonPattern": "owner|tenant|authoriz|idor|scope|object.?level", "repo": {"base": {"api/invoices.ts": "import type { Request, Response } from 'express';\nimport { db } from './db';\nimport { requireAuth } from './middleware';\n\n// List endpoint scopes rows to the signed-in owner.\nexport const listInvoices = [\n requireAuth,\n async (req: Request, res: Response): Promise => {\n const invoices = await db.invoice.findMany({\n where: { ownerId: req.session.userId },\n select: { id: true, total: true, issuedAt: true },\n });\n res.json(invoices);\n },\n];\n"}, "staged": {"api/invoices.ts": "import type { Request, Response } from 'express';\nimport { db } from './db';\nimport { requireAuth } from './middleware';\n\n// List endpoint scopes rows to the signed-in owner.\nexport const listInvoices = [\n requireAuth,\n async (req: Request, res: Response): Promise => {\n const invoices = await db.invoice.findMany({\n where: { ownerId: req.session.userId },\n select: { id: true, total: true, issuedAt: true },\n });\n res.json(invoices);\n },\n];\n\nexport const getInvoice = [\n requireAuth,\n async (req: Request, res: Response): Promise => {\n const invoice = await db.invoice.findUnique({ where: { id: req.params.id } });\n if (!invoice) {\n res.status(404).end();\n return;\n }\n res.json(invoice);\n },\n];\n"}}, "note": "getInvoice is authenticated but unscoped: any signed-in user can read any invoice by ID, while the sibling list endpoint shows the ownerId-scoped pattern. Authn present, authz missing — gold for the new object-level-authz item.", "difficulty": "borderline", "provenance": "authored", "variantOf": null, "holdout": false} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":false,"id":"apisec-password-plaintext-compare","expected":"FAIL","expectItems":["auth-mechanism"],"reasonPattern":"password|plaintext|hash|credential","repo":{"base":{"api/login.ts":"export async function login(req, res, users) {\n const user = await users.byEmail(req.body.email);\n return res.json({ ok: await verifyHash(req.body.password, user.passwordHash) });\n}\n"},"staged":{"api/login.ts":"export async function login(req, res, users) {\n const user = await users.byEmail(req.body.email);\n return res.json({ ok: req.body.password === user.password });\n}\n"}},"note":"The staged login path replaces password-hash verification with a plaintext credential comparison, requiring plaintext password storage and exposing every account if the store leaks.","difficulty":"clear","provenance":"authored"} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":false,"id":"apisec-unsafe-yaml-body","expected":"FAIL","expectItems":["xxe-prevention"],"reasonPattern":"yaml|unsafe|object|prototype|parse","repo":{"base":{"api/import.ts":"import { safeLoad } from \"yaml\";\nexport function importConfig(req) { return safeLoad(req.body.text, { schema: \"failsafe\" }); }\n"},"staged":{"api/import.ts":"import yaml from \"js-yaml\";\nexport function importConfig(req) { return yaml.load(req.body.text); }\n"}},"note":"Untrusted request YAML is now passed to the unrestricted loader instead of the constrained failsafe parser.","difficulty":"borderline","provenance":"adapted"} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":true,"id":"apisec-download-path-traversal","expected":"FAIL","expectItems":["path-traversal"],"reasonPattern":"traversal|base|outside|path|req.query","repo":{"base":{"api/download.ts":"import { readFile } from \"node:fs/promises\";\nimport { basename, join } from \"node:path\";\nexport const download = req => readFile(join(\"/srv/exports\", basename(req.query.name)));\n"},"staged":{"api/download.ts":"import { readFile } from \"node:fs/promises\";\nimport { join } from \"node:path\";\nexport const download = req => readFile(join(\"/srv/exports\", req.query.name));\n"}},"note":"The staged path removes basename confinement, allowing ../ segments from req.query.name to read files outside the export directory.","difficulty":"clear","provenance":"mined"} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":false,"id":"apisec-webhook-ssrf","expected":"FAIL","expectItems":["ssrf-prevention"],"reasonPattern":"SSRF|allowlist|scheme|host|internal|metadata","repo":{"base":{"api/webhook.ts":"const ALLOWED = new Set([\"hooks.example.com\"]);\nexport async function test(req) { const u = new URL(req.body.url); if (u.protocol !== \"https:\" || !ALLOWED.has(u.hostname)) throw Error(\"invalid\"); return fetch(u); }\n"},"staged":{"api/webhook.ts":"export async function test(req) { return fetch(req.body.url); }\n"}},"note":"The staged webhook tester removes the scheme and host allowlist before fetching a request-controlled URL, enabling SSRF to internal services.","difficulty":"clear","provenance":"authored"} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":false,"id":"apisec-open-redirect-next","expected":"FAIL","expectItems":["open-redirect"],"reasonPattern":"redirect|allowlist|relative|phishing|next","repo":{"base":{"api/session.ts":"export function done(req, res) { const next = String(req.query.next || \"/\"); res.redirect(next.startsWith(\"/\") && !next.startsWith(\"//\") ? next : \"/\"); }\n"},"staged":{"api/session.ts":"export function done(req, res) { res.redirect(req.query.next || \"/\"); }\n"}},"note":"A raw request-controlled next value is now passed to redirect, allowing an attacker to send users from the trusted origin to a phishing site.","difficulty":"borderline","provenance":"authored"} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":true,"id":"apisec-login-rate-limit-removed","expected":"FAIL","expectItems":["rate-limiting"],"reasonPattern":"rate|limit|brute|credential|login","repo":{"base":{"api/router.ts":"router.post(\"/login\", rateLimit({ max: 5 }), login);\n"},"staged":{"api/router.ts":"// rate limiter removed while investigating proxy addresses\nrouter.post(\"/login\", login);\n"}},"note":"The staged login route removes its only rate limiter, enabling unbounded online credential guessing.","difficulty":"clear","provenance":"mined"} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":true,"id":"apisec-oauth-state-verified-decoy","expected":"PASS","expectItems":["oauth-security","input-validation"],"repo":{"base":{"api/oauth.ts":"export function callback(req, res) { return exchange(req.query.code); }\n"},"staged":{"api/oauth.ts":"export function callback(req, res) { const state = stateSchema.parse(req.query.state); if (state !== req.session.oauthState) throw Error(\"invalid oauth state\"); return exchange(req.query.code); }\n"}},"note":"The staged OAuth callback adds schema validation and an exact session-bound state comparison; this is the CSRF defense, not a vulnerability.","difficulty":"adversarial","provenance":"authored"} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":true,"id":"apisec-zod-body-decoy","expected":"PASS","expectItems":["input-validation"],"repo":{"base":{"api/profile.ts":"export const update = req => profiles.update(req.body);\n"},"staged":{"api/profile.ts":"const Profile = z.object({ displayName: z.string().max(80) }).strict();\nexport const update = req => profiles.update(Profile.parse(req.body));\n"}},"note":"The staged route replaces an unbounded request body with a strict named-field schema; the input-validation lens should recognize the fix and pass it.","difficulty":"borderline","provenance":"authored"} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":false,"id":"apisec-rate-limit-config-decoy","expected":"PASS","expectItems":["rate-limiting","endpoint-auth"],"repo":{"base":{"api/router.ts":"router.post(\"/reset\", requireSession, resetPassword);\n"},"staged":{"api/router.ts":"router.post(\"/reset\", requireSession, rateLimit({ windowMs: 60000, max: 3 }), resetPassword);\n"}},"note":"The authenticated password-reset route gains a tight per-minute rate limit; no security protection is removed or bypassed.","difficulty":"clear","provenance":"authored"} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":false,"id":"apisec-bounded-upload-decoy","expected":"PASS","expectItems":["processing-security","input-validation"],"repo":{"base":{"api/avatar.ts":"export async function avatar(req) { return store(req.file); }\n"},"staged":{"api/avatar.ts":"export async function avatar(req) { if (!req.file || req.file.size > 2_000_000 || ![\"image/png\",\"image/jpeg\"].includes(req.file.mimetype)) throw Error(\"invalid upload\"); return store(req.file); }\n"}},"note":"The staged upload path adds size and MIME bounds before storage, so processing-security is exercised by a defensive change.","difficulty":"borderline","provenance":"adapted"} +{"reviewer":"api-security-reviewer","variantOf":null,"holdout":false,"id":"apisec-general-health-decoy","expected":"PASS","expectItems":["general-security"],"repo":{"base":{"api/health.ts":"export const build = \"old\";\n"},"staged":{"api/health.ts":"export const build = \"2026.07\";\n"}},"note":"A static build label changes with no request input, secret, sink, or security boundary; the general lens must not invent an exploit.","difficulty":"adversarial","provenance":"authored"} diff --git a/gate-engine/review/eval/reviewers/cases-backend-performance.jsonl b/gate-engine/review/eval/reviewers/cases-backend-performance.jsonl index 282c8662..5ea31d9d 100644 --- a/gate-engine/review/eval/reviewers/cases-backend-performance.jsonl +++ b/gate-engine/review/eval/reviewers/cases-backend-performance.jsonl @@ -11,3 +11,15 @@ {"reviewer": "backend-performance-reviewer", "variantOf": null, "holdout": true, "id": "beperf-decoy-lazy-singleton-pool", "expected": "PASS", "repo": {"base": {"api/reports.ts": "import { Router } from 'express';\nimport { Pool } from 'pg';\n\nexport const reports = Router();\n\ninterface ReportRow {\n iso_week: string;\n total_minor: number;\n}\n\n// One pool per process; pg queues checkouts internally when all clients are\n// busy, so request handlers never open their own connections.\nconst pool = new Pool({ connectionString: process.env.DATABASE_URL, max: 10 });\n\n// GET /reports/weekly — 52 pre-aggregated rows from weekly_report_totals\n// (one row per ISO week, maintained by the nightly rollup job).\nreports.get('/reports/weekly', async (_req, res) => {\n const result = await pool.query(\n 'SELECT iso_week, total_minor FROM weekly_report_totals ORDER BY iso_week DESC FETCH FIRST 52 ROWS ONLY',\n );\n return res.json({ rows: result.rows });\n});\n\n// GET /reports/top-projects — pre-aggregated leaderboard, 20 rows.\nreports.get('/reports/top-projects', async (_req, res) => {\n const result = await pool.query(\n 'SELECT project_id, total_minor FROM project_report_totals ORDER BY total_minor DESC FETCH FIRST 20 ROWS ONLY',\n );\n return res.json({ rows: result.rows });\n});\n"}, "staged": {"api/reports.ts": "import { Router } from 'express';\nimport { Pool } from 'pg';\n\nexport const reports = Router();\n\ninterface ReportRow {\n iso_week: string;\n total_minor: number;\n}\n\nlet pool: Pool | null = null;\n\n// Construct the pool on first use instead of at import time: unit tests and\n// the CLI import this router without a database, and an eager Pool would open\n// sockets (and crash on a missing DATABASE_URL) the moment the module loads.\n// After the first call every request reuses the same process-wide pool; pg\n// queues checkouts internally when all clients are busy.\nfunction getPool(): Pool {\n if (pool === null) {\n pool = new Pool({ connectionString: process.env.DATABASE_URL, max: 10 });\n }\n return pool;\n}\n\n// GET /reports/weekly — 52 pre-aggregated rows from weekly_report_totals\n// (one row per ISO week, maintained by the nightly rollup job).\nreports.get('/reports/weekly', async (_req, res) => {\n const result = await getPool().query(\n 'SELECT iso_week, total_minor FROM weekly_report_totals ORDER BY iso_week DESC FETCH FIRST 52 ROWS ONLY',\n );\n return res.json({ rows: result.rows });\n});\n\n// GET /reports/top-projects — pre-aggregated leaderboard, 20 rows.\nreports.get('/reports/top-projects', async (_req, res) => {\n const result = await getPool().query(\n 'SELECT project_id, total_minor FROM project_report_totals ORDER BY total_minor DESC FETCH FIRST 20 ROWS ONLY',\n );\n return res.json({ rows: result.rows });\n});\n"}}, "note": "The diff moves Pool construction from module scope into a function called inside handlers — pattern-matches per-request pool creation — but the `if (pool === null)` memoization (synchronous, no await between check and assign) makes it a process-wide singleton. Test-friendliness refactor, nothing to block.", "difficulty": "adversarial", "provenance": "authored"} {"reviewer": "backend-performance-reviewer", "variantOf": "beperf-nplusone-statement-loop", "holdout": false, "id": "beperf-nplusone-statement-loop-fixed", "expected": "PASS", "repo": {"base": {"api/invoices.ts": "import { Router } from 'express';\nimport { prisma } from './prisma-client';\n\nexport const invoices = Router();\n\nfunction toMinorUnits(amount: string): number {\n return Math.round(Number(amount) * 100);\n}\n\n// GET /invoices/:id — single invoice detail for the drawer view.\ninvoices.get('/invoices/:id', async (req, res) => {\n const invoice = await prisma.invoice.findUnique({\n where: { id: req.params.id },\n include: { customer: true, lines: true },\n });\n if (!invoice) {\n return res.status(404).json({ message: 'unknown invoice' });\n }\n return res.json({\n id: invoice.id,\n customerName: invoice.customer.name,\n totalMinor: toMinorUnits(invoice.total),\n lines: invoice.lines,\n });\n});\n\n// PATCH /invoices/:id/memo — edit the free-text memo shown on the PDF.\ninvoices.patch('/invoices/:id/memo', async (req, res) => {\n const memo = String(req.body.memo ?? '').slice(0, 500);\n const updated = await prisma.invoice.update({\n where: { id: req.params.id },\n data: { memo },\n });\n return res.json({ id: updated.id, memo: updated.memo });\n});\n"}, "staged": {"api/invoices.ts": "import { Router } from 'express';\nimport { prisma } from './prisma-client';\n\nexport const invoices = Router();\n\nfunction toMinorUnits(amount: string): number {\n return Math.round(Number(amount) * 100);\n}\n\n// GET /invoices/:id — single invoice detail for the drawer view.\ninvoices.get('/invoices/:id', async (req, res) => {\n const invoice = await prisma.invoice.findUnique({\n where: { id: req.params.id },\n include: { customer: true, lines: true },\n });\n if (!invoice) {\n return res.status(404).json({ message: 'unknown invoice' });\n }\n return res.json({\n id: invoice.id,\n customerName: invoice.customer.name,\n totalMinor: toMinorUnits(invoice.total),\n lines: invoice.lines,\n });\n});\n\n// PATCH /invoices/:id/memo — edit the free-text memo shown on the PDF.\ninvoices.patch('/invoices/:id/memo', async (req, res) => {\n const memo = String(req.body.memo ?? '').slice(0, 500);\n const updated = await prisma.invoice.update({\n where: { id: req.params.id },\n data: { memo },\n });\n return res.json({ id: updated.id, memo: updated.memo });\n});\n\n// GET /statements/:orgId/:month — monthly statement rollup for the billing\n// screen. A busy org-month holds ~4,000 invoices in production, so customer\n// names and line totals are fetched as two set-based queries and joined in\n// memory.\ninvoices.get('/statements/:orgId/:month', async (req, res) => {\n const rows = await prisma.invoice.findMany({\n where: { orgId: req.params.orgId, month: req.params.month },\n });\n const customerIds = [...new Set(rows.map((r) => r.customerId))];\n const customers = await prisma.customer.findMany({\n where: { id: { in: customerIds } },\n select: { id: true, name: true },\n });\n const nameById = new Map(customers.map((c) => [c.id, c.name]));\n const lineTallies = await prisma.invoiceLine.groupBy({\n by: ['invoiceId'],\n where: { invoiceId: { in: rows.map((r) => r.id) } },\n _count: { _all: true },\n });\n const tallyByInvoice = new Map(lineTallies.map((g) => [g.invoiceId, g._count._all]));\n const statement = rows.map((rec) => ({\n id: rec.id,\n customerName: nameById.get(rec.customerId) ?? '',\n totalMinor: toMinorUnits(rec.total),\n lineCount: tallyByInvoice.get(rec.id) ?? 0,\n }));\n return res.json({ month: req.params.month, statement });\n});\n"}}, "note": "Minimal pair of beperf-nplusone-statement-loop: same base and endpoint, but the per-invoice loop is replaced by one findMany-with-id-list plus one groupBy — three queries total regardless of invoice count.", "difficulty": "clear", "provenance": "authored"} {"id": "beperf-sync-io-handler", "reviewer": "backend-performance-reviewer", "expected": "FAIL", "expectItems": ["sync-io"], "reasonPattern": "sync|block|event.?loop", "repo": {"base": {"api/render.ts": "import { readFile } from 'node:fs/promises';\nimport type { Request, Response } from 'express';\n\nconst templates = new Map();\n\n// Templates load once at boot; handlers read the warm map.\nexport async function warmTemplates(names: string[]): Promise {\n for (const name of names) {\n templates.set(name, await readFile(`templates/${name}.html`, 'utf8'));\n }\n}\n\nexport function renderWelcome(_req: Request, res: Response): void {\n res.send(templates.get('welcome') ?? '');\n}\n"}, "staged": {"api/render.ts": "import { readFile } from 'node:fs/promises';\nimport type { Request, Response } from 'express';\n\nconst templates = new Map();\n\n// Templates load once at boot; handlers read the warm map.\nexport async function warmTemplates(names: string[]): Promise {\n for (const name of names) {\n templates.set(name, await readFile(`templates/${name}.html`, 'utf8'));\n }\n}\n\nexport function renderWelcome(_req: Request, res: Response): void {\n res.send(templates.get('welcome') ?? '');\n}\n\nexport function renderReceipt(req: Request, res: Response): void {\n const { readFileSync } = require('node:fs');\n const partial = readFileSync(`templates/receipt-${req.query.locale ?? 'en'}.html`, 'utf8');\n res.send(partial);\n}\n"}}, "note": "readFileSync inside a request handler blocks the event loop for every concurrent request; the file shows the boot-time async pattern the handler should reuse. Gold for the new sync-io item.", "difficulty": "clear", "provenance": "authored", "variantOf": null, "holdout": false} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":false,"id":"beperf-orders-sort-without-index","expected":"FAIL","expectItems":["indexing","db-query-optimization"],"reasonPattern":"index|orderBy|full scan|latency","repo":{"base":{"api/orders.ts":"export const recent = db.order.findMany({ take: 50 });\n"},"staged":{"api/orders.ts":"// orders has 40M rows; no createdAt index exists\nexport const recent = db.order.findMany({ orderBy: { createdAt: \"desc\" }, take: 50 });\n"}},"note":"The staged query adds a descending sort over a documented 40M-row table without a supporting index, forcing a full scan/sort on every request.","difficulty":"borderline","provenance":"authored"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":true,"id":"beperf-unbounded-tenant-cache","expected":"FAIL","expectItems":["unbounded-cache","caching-strategy"],"reasonPattern":"unbounded|evict|TTL|memory|tenant","repo":{"base":{"api/flags.ts":"export const flags = req => loadFlags(req.tenant.id);\n"},"staged":{"api/flags.ts":"// process-wide cache\nconst tenantCache = new Map();\nexport async function flags(req) { if (!tenantCache.has(req.tenant.id)) tenantCache.set(req.tenant.id, await loadFlags(req.tenant.id)); return tenantCache.get(req.tenant.id); }\n"}},"note":"A process-lifetime Map now retains one entry for every tenant ever seen with no TTL, size bound, or eviction.","difficulty":"clear","provenance":"mined"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":false,"id":"beperf-export-buffered-response","expected":"FAIL","expectItems":["streaming","response-optimization"],"reasonPattern":"stream|buffer|memory|large|response","repo":{"base":{"api/export.ts":"export function download(req, res) { createExportStream(req.org.id).pipe(res); }\n"},"staged":{"api/export.ts":"export async function download(req, res) { const chunks = []; for await (const chunk of createExportStream(req.org.id)) chunks.push(chunk); res.send(Buffer.concat(chunks)); }\n"}},"note":"The staged export buffers the entire multi-gigabyte stream before responding, multiplying memory pressure and delaying first byte.","difficulty":"clear","provenance":"adapted"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":false,"id":"beperf-serial-bulk-write","expected":"FAIL","expectItems":["batching","async-handling"],"reasonPattern":"batch|serial|Promise.all|round.?trip","repo":{"base":{"api/import.ts":"export async function save(rows) { await db.record.createMany({ data: rows }); }\n"},"staged":{"api/import.ts":"// bulk batch path replaced by per-row writes\nexport async function save(rows) { for (const row of rows) await db.record.create({ data: row }); }\n"}},"note":"A single batch insert is replaced by one awaited database round trip per imported row.","difficulty":"clear","provenance":"authored"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":true,"id":"beperf-retry-without-bound","expected":"FAIL","expectItems":["timeout-retry","async-handling"],"reasonPattern":"retry|backoff|bound|infinite|timeout","repo":{"base":{"api/vendor.ts":"export const call = () => fetchVendor({ timeout: 5000, retries: 2 });\n"},"staged":{"api/vendor.ts":"export async function call() { while (true) { try { return await fetchVendor(); } catch { await delay(10); } } }\n"}},"note":"The staged vendor call retries forever with no attempt bound, meaningful backoff, or request timeout, so an outage pins work indefinitely.","difficulty":"borderline","provenance":"authored"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":false,"id":"beperf-hot-loop-debug-log","expected":"FAIL","expectItems":["logging-overhead"],"reasonPattern":"log|hot|loop|volume|serialize","repo":{"base":{"api/events.ts":"export function ingest(events) { for (const event of events) apply(event); }\n"},"staged":{"api/events.ts":"export function ingest(events) { for (const event of events) { console.log(\"full event\", JSON.stringify(event)); apply(event); } }\n"}},"note":"The staged hot ingestion loop serializes and synchronously logs every full event, adding CPU and high-volume I/O proportional to batch size.","difficulty":"clear","provenance":"authored"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":false,"id":"beperf-cache-stampede-bounded-decoy","expected":"PASS","expectItems":["caching-strategy","async-handling"],"repo":{"base":{"api/tool-path.ts":"export const resolveTool = () => probeWindowsRegistry();\n"},"staged":{"api/tool-path.ts":"let cachedTool;\nexport async function resolveTool() { if (cachedTool) return cachedTool; const found = await probeWindowsRegistry(); cachedTool = found; return found; }\n"}},"note":"Production-derived decoy: simultaneous first callers can duplicate one idempotent Windows-only discovery probe, but the result is process-cached and the path is bounded, rare, and non-blocking.","difficulty":"adversarial","provenance":"mined"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":true,"id":"beperf-batched-fetch-decoy","expected":"PASS","expectItems":["batching","async-handling"],"repo":{"base":{"api/dashboard.ts":"export async function load() { return loadSummary(); }\n"},"staged":{"api/dashboard.ts":"export async function load() { const [summary, alerts] = await Promise.all([loadSummary(), loadAlerts()]); return { summary, alerts }; }\n"}},"note":"Independent dashboard reads are deliberately batched with Promise.all; async and batching lenses should pass the optimization.","difficulty":"clear","provenance":"authored"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":true,"id":"beperf-compressed-response-decoy","expected":"PASS","expectItems":["response-optimization"],"repo":{"base":{"api/report.ts":"export const report = (_req, res) => res.json(buildReport());\n"},"staged":{"api/report.ts":"export const report = (_req, res) => { res.setHeader(\"Content-Encoding\", \"gzip\"); return res.send(gzip(buildReport())); };\n"}},"note":"The staged response adds compression to an existing large report payload and does not introduce extra work on a hot loop.","difficulty":"borderline","provenance":"adapted"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":false,"id":"beperf-edge-prefetch-decoy","expected":"PASS","expectItems":["network-optimization"],"repo":{"base":{"api/assets.ts":"export const assetOrigin = \"https://origin.example\";\n"},"staged":{"api/assets.ts":"// edge CDN prefetches the immutable manifest once per deploy\nexport const assetOrigin = \"https://cdn.example\";\n"}},"note":"Moving immutable asset delivery to the existing edge CDN is a network optimization, not a production performance regression.","difficulty":"adversarial","provenance":"authored"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":false,"id":"beperf-startup-sync-read-decoy","expected":"PASS","expectItems":["sync-io"],"repo":{"base":{"api/config.ts":"export const config = {};\n"},"staged":{"api/config.ts":"import { readFileSync } from \"node:fs\";\n// one module-initialization read before the server accepts requests\nexport const config = JSON.parse(readFileSync(\"config.json\", \"utf8\"));\n"}},"note":"The synchronous read runs once during process initialization, outside request-serving paths; the skill explicitly grants startup code latitude.","difficulty":"adversarial","provenance":"authored"} +{"reviewer":"backend-performance-reviewer","variantOf":null,"holdout":false,"id":"beperf-general-constant-decoy","expected":"PASS","repo":{"base":{"api/version.ts":"export const protocol = 1;\n"},"staged":{"api/version.ts":"export const protocol = 2;\n"}},"note":"A module constant changes without adding I/O, allocation growth, repeated work, or algorithmic cost; the generic diff-header indexing trigger must not cause an invented finding.","difficulty":"adversarial","provenance":"authored"} diff --git a/gate-engine/review/eval/reviewers/cases-frontend-performance.jsonl b/gate-engine/review/eval/reviewers/cases-frontend-performance.jsonl index b3615b0b..1b15a8dc 100644 --- a/gate-engine/review/eval/reviewers/cases-frontend-performance.jsonl +++ b/gate-engine/review/eval/reviewers/cases-frontend-performance.jsonl @@ -11,3 +11,15 @@ {"reviewer": "frontend-performance-reviewer", "variantOf": null, "holdout": true, "id": "feperf-per-method-lodash", "expected": "PASS", "repo": {"base": {"web/ReportFilters.tsx": "import { useMemo, useState } from \"react\";\n\ntype ReportFilter = {\n id: string;\n section: string;\n label: string;\n active: boolean;\n};\n\n// Filter rail for the reports screen.\n\nexport function ReportFilters({ filters, onToggle }: { filters: ReportFilter[]; onToggle: (id: string) => void }) {\n const [term, setTerm] = useState(\"\");\n\n const visible = useMemo(\n () => filters.filter((f) => f.label.toLowerCase().includes(term.toLowerCase())),\n [filters, term],\n );\n\n return (\n \n );\n}\n"}, "staged": {"web/ReportFilters.tsx": "import { useEffect, useMemo, useState } from \"react\";\n// Bundle note: the package-root lodash build is ~72 kB min+gz and a default\n// import of it does NOT tree-shake. The per-method entrypoints below cost\n// ~1-2 kB each — do not collapse them into a package-root default import.\nimport debounce from \"lodash/debounce\";\nimport groupBy from \"lodash/groupBy\";\n\ntype ReportFilter = {\n id: string;\n section: string;\n label: string;\n active: boolean;\n};\n\n// Filter rail for the reports screen.\n\nexport function ReportFilters({ filters, onToggle }: { filters: ReportFilter[]; onToggle: (id: string) => void }) {\n const [term, setTerm] = useState(\"\");\n\n const visible = useMemo(\n () => filters.filter((f) => f.label.toLowerCase().includes(term.toLowerCase())),\n [filters, term],\n );\n\n const grouped = useMemo(() => groupBy(visible, (f) => f.section), [visible]);\n\n // Remember the last search so the rail restores it on reopen.\n useEffect(() => {\n const persist = debounce((next: string) => {\n window.localStorage.setItem(\"report-filter-term\", next);\n }, 200);\n persist(term);\n return () => persist.cancel();\n }, [term]);\n\n return (\n \n );\n}\n"}}, "note": "Near-miss twin of the entry-bundle gold: \"lodash\" appears in new imports plus a scary bundle comment, but these are per-method entrypoints (~1-2 kB each, the recommended form), the debounce is cancelled on cleanup, and deps are correct. Pattern-matching \"lodash = bundle bomb\" fails here.", "difficulty": "adversarial", "provenance": "authored"} {"reviewer": "frontend-performance-reviewer", "variantOf": "feperf-activity-refetch-storm", "holdout": false, "id": "feperf-activity-refetch-fixed", "expected": "PASS", "repo": {"base": {"web/ProjectActivity.tsx": "import { useState } from \"react\";\n\ntype ActivityEntry = {\n id: string;\n summary: string;\n actor: string;\n at: string;\n};\n\n// Sidebar panel; the parent keeps it mounted across tab switches.\n\nexport function ProjectActivity({ entries }: { entries: ActivityEntry[] }) {\n const [expanded, setExpanded] = useState(false);\n const shown = expanded ? entries : entries.slice(0, 5);\n\n return (\n \n );\n}\n"}, "staged": {"web/ProjectActivity.tsx": "import { useEffect, useState } from \"react\";\n\ntype ActivityEntry = {\n id: string;\n summary: string;\n actor: string;\n at: string;\n};\n\n// Sidebar panel; the parent keeps it mounted across tab switches.\n\nexport function ProjectActivity({ projectId }: { projectId: string }) {\n const [expanded, setExpanded] = useState(false);\n const [entries, setEntries] = useState([]);\n\n // One request per project; aborted when the panel unmounts or the project\n // changes. AbortError is the expected rejection on cleanup.\n useEffect(() => {\n const ctrl = new AbortController();\n fetch(\"/api/projects/\" + projectId + \"/activity\", { signal: ctrl.signal })\n .then((res) => res.json())\n .then((data: { entries: ActivityEntry[] }) => setEntries(data.entries))\n .catch((err: unknown) => {\n if ((err as Error).name !== \"AbortError\") setEntries([]);\n });\n return () => ctrl.abort();\n }, [projectId]);\n\n const shown = expanded ? entries : entries.slice(0, 5);\n\n return (\n \n );\n}\n"}}, "note": "Minimal pair of feperf-activity-refetch-storm: identical feature but the effect has [projectId] deps and an AbortController cleanup — exactly one fetch per project, so the same fetch/hooks regex surface must now PASS.", "difficulty": "clear", "provenance": "authored"} {"id": "feperf-scroll-reflow", "reviewer": "frontend-performance-reviewer", "expected": "FAIL", "expectItems": ["layout-thrash"], "reasonPattern": "reflow|layout|thrash|getBoundingClientRect|batch|read.*write", "repo": {"base": {"web/Timeline.tsx": "import { useRef } from 'react';\n\nexport interface TimelineItem {\n id: string;\n label: string;\n}\n\nexport function Timeline({ items }: { items: TimelineItem[] }) {\n const listRef = useRef(null);\n return (\n
    \n {items.map((item) => (\n
  • \n {item.label}\n
    • \n ))}\n
\n );\n}\n"}, "staged": {"web/Timeline.tsx": "import { useRef } from 'react';\n\nexport interface TimelineItem {\n id: string;\n label: string;\n}\n\nexport function Timeline({ items }: { items: TimelineItem[] }) {\n const listRef = useRef(null);\n const handleScroll = () => {\n const rows = listRef.current?.querySelectorAll('.timeline-row') ?? [];\n for (const row of rows) {\n const rect = row.getBoundingClientRect();\n row.style.height = `${Math.max(32, rect.height)}px`;\n row.style.opacity = rect.top < 0 ? '0.5' : '1';\n }\n };\n return (\n
    \n {items.map((item) => (\n
  • \n {item.label}\n
    • \n ))}\n
\n );\n}\n"}}, "note": "Scroll handler interleaves a layout read (getBoundingClientRect) with style writes per row on every scroll event — forced synchronous reflow per item on a hot path. Gold for the new layout-thrash item.", "difficulty": "clear", "provenance": "authored", "variantOf": null, "holdout": false} +{"reviewer":"frontend-performance-reviewer","variantOf":null,"holdout":true,"id":"feperf-prod-stale-stop-callback-decoy","expected":"PASS","expectItems":["hooks-optimization"],"repo":{"base":{"web/Session.tsx":"import { useCallback } from \"react\";\nexport function Session({ runId, stop }) { const onStop = useCallback(() => stop(runId), [runId, stop]); return ; }\n"},"staged":{"web/Session.tsx":"import { useCallback } from \"react\";\nexport function Session({ runId, stop }) { const onStop = useCallback(() => stop(runId), [stop]); return ; }\n"}},"note":"Production-derived decoy: omitting runId makes Stop target stale state, a correctness defect, but it does not add repeated work, rerenders, I/O, or another performance consequence.","difficulty":"adversarial","provenance":"mined"} +{"reviewer":"frontend-performance-reviewer","variantOf":null,"holdout":true,"id":"feperf-prod-incompatible-prop-decoy","expected":"PASS","expectItems":["react-rendering"],"repo":{"base":{"web/ReviewPanel.tsx":"export function ReviewPanel({ onOpen }) { return ; }\n"},"staged":{"web/ReviewPanel.tsx":"export function ReviewPanel({ onOpen }) { return ; }\n"}},"note":"Production-derived decoy: passing an object to a caller that still expects a string can break functionality, but one click allocation is not a demonstrated rendering or runtime performance regression.","difficulty":"adversarial","provenance":"mined"} +{"reviewer":"frontend-performance-reviewer","variantOf":null,"holdout":false,"id":"feperf-prod-scroll-behavior-decoy","expected":"PASS","expectItems":["hooks-optimization"],"repo":{"base":{"web/Transcript.tsx":"import { useEffect } from \"react\";\nexport function Transcript({ selectedId }) { useEffect(() => { document.getElementById(selectedId)?.scrollIntoView(); }, [selectedId]); return
; }\n"},"staged":{"web/Transcript.tsx":"import { useEffect } from \"react\";\nexport function Transcript({ selectedId }) { useEffect(() => { document.getElementById(selectedId)?.scrollIntoView(); }, []); return
; }\n"}},"note":"Production-derived decoy: the effect now misses later selection changes, a functional regression; it actually runs less often and has no independent performance cost.","difficulty":"adversarial","provenance":"mined"} +{"reviewer":"frontend-performance-reviewer","variantOf":null,"holdout":true,"id":"feperf-hero-image-layout-shift","expected":"FAIL","expectItems":["image-optimization"],"reasonPattern":"image|dimension|layout shift|lazy|LCP","repo":{"base":{"web/Hero.tsx":"export const Hero = () => \"Product\";\n"},"staged":{"web/Hero.tsx":"export const Hero = () => \"Product\";\n"}},"note":"The staged hero swaps an optimized dimensioned asset for an undimensioned raw PNG, worsening transfer size and causing layout shift.","difficulty":"clear","provenance":"authored"} +{"reviewer":"frontend-performance-reviewer","variantOf":null,"holdout":false,"id":"feperf-hot-row-inline-style","expected":"FAIL","expectItems":["inline-styles","list-rendering"],"reasonPattern":"style|object|memo|row|render","repo":{"base":{"web/Rows.tsx":"import { memo } from \"react\";\nconst Row = memo(({ item }) =>
  • {item.name}
    • );\nexport const Rows = ({ items }) =>
      {items.map(item => )}
    ;\n"},"staged":{"web/Rows.tsx":"import { memo } from \"react\";\nconst Row = memo(({ item, style }) =>
  • {item.name}
    • );\nexport const Rows = ({ items }) =>
      {items.map(item => )}
    ;\n"}},"note":"Every render of a large mapped list creates a fresh style object for each memoized row, defeating reference equality and reallocating on the hot path.","difficulty":"borderline","provenance":"authored"} +{"reviewer":"frontend-performance-reviewer","variantOf":null,"holdout":false,"id":"feperf-offscreen-iframe-eager","expected":"FAIL","expectItems":["iframe-usage"],"reasonPattern":"iframe|lazy|offscreen|document","repo":{"base":{"web/Map.tsx":"export const Map = () => null;\n"},"staged":{"web/Map.tsx":"// Below the pricing FAQ, well outside the initial viewport.\nexport const Map = () =>