From 057c0e3c9ec468c701e676076b6d99e65aac9c34 Mon Sep 17 00:00:00 2001 From: norvalbv Date: Sat, 1 Aug 2026 18:29:27 +0100 Subject: [PATCH] feat(reviewer-eval): propose/finalize adaptation pipeline + correctness batch 1 (+24 rows) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR B of the corpus-growth effort. Pipeline (methodology item 5 — immutable raw, overlay-only audit edits): - propose.mts: deterministic triage of candidates.jsonl — hard drops (unresolved outcome, missing commit anchor, no line info, outdated-only, truncated hunk, already-in-corpus), category+path routing to the five suites, severity/scope priority sort, base-content enrichment pinned to ?ref= (observation-instant inputs, item 13) into raw/queue-.jsonl. - finalize.mts: --check lints a single proposal, then (only when structurally clean) validateRow-materializes it; substring private-repo leak scan (compound identifiers included) runs in BOTH --check and --append — devkit is public; --append takes an exclusive stale-aware lock beside the cases file (concurrent appends would double-append ids), applies raw/audit-overlay.jsonl, lints every row pre-append, skips existing ids, assigns holdout deterministically (PASS rows default to the dev split; the >=3-holdout-per-class floor outranks the dev-bias and reports every flip), never splits a caseId across batches. - raw/ gitignored (immutable quarry, never committed). Batch 1: 12 mined correctness golds (outcome=fixed, anonymized fixtures, one lens each, caseId/sourcePr/outcomeEvidence/scopeConfirmed stamped) + 12 minimal-pair decoys (real fix applied, variantOf/adapted). Corpus 66 -> 90; bench validate green. Co-Authored-By: Claude Fable 5 --- .gitignore | 1 + .../eval/reviewers/cases-correctness.jsonl | 24 + .../review/eval/reviewers/finalize.mts | 419 ++++++++++++++++++ gate-engine/review/eval/reviewers/propose.mts | 278 ++++++++++++ 4 files changed, 722 insertions(+) create mode 100644 gate-engine/review/eval/reviewers/finalize.mts create mode 100644 gate-engine/review/eval/reviewers/propose.mts diff --git a/.gitignore b/.gitignore index f497ff6e..3da59175 100644 --- a/.gitignore +++ b/.gitignore @@ -58,6 +58,7 @@ gate-engine/review/eval/transcripts/ gate-engine/review/eval/reviewers/*.log gate-engine/review/eval/reviewers/results.baseline.json gate-engine/review/eval/reviewers/candidates.jsonl +gate-engine/review/eval/reviewers/raw/ gate-engine/review/eval/reviewers/progress-*.jsonl # conventions-eval: ledger + per-run transcripts are local telemetry (mirrors critique-eval). diff --git a/gate-engine/review/eval/reviewers/cases-correctness.jsonl b/gate-engine/review/eval/reviewers/cases-correctness.jsonl index 0eb09a6f..b902ecff 100644 --- a/gate-engine/review/eval/reviewers/cases-correctness.jsonl +++ b/gate-engine/review/eval/reviewers/cases-correctness.jsonl @@ -64,3 +64,27 @@ {"id":"corr-decoy-signature-callsite-updated","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/fmt/money.ts":"export function formatMoney(amount: number, currency: string): string {\n return `${currency} ${amount.toFixed(2)}`;\n}\n\nexport function receipt(total: number): string {\n return `Total: ${formatMoney(total, 'USD')}`;\n}\n"},"staged":{"src/fmt/money.ts":"export function formatMoney(currency: string, amount: number): string {\n return `${currency} ${amount.toFixed(2)}`;\n}\n\nexport function receipt(total: number): string {\n return `Total: ${formatMoney('USD', total)}`;\n}\n"}},"note":"Correct: the signature change to (currency, amount) is matched at the receipt() call site, which now passes ('USD', total). Contract holds. Correctness stays silent.","difficulty":"clear","provenance":"authored","variantOf":"corr-signature-change-callsite-missed","holdout":true} {"id":"corr-broadcast-fanout-no-dedup","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["writer-reader-contracts"],"reasonPattern":"broadcast|fan.?out|each|listener|window|dedup|target|N times|every|duplicat","repo":{"base":{"src/sync/broadcast.ts":"import { getWindows } from './windows';\n\nexport function notifySaved(id: string): void {\n const wins = getWindows();\n const target = wins.find((w) => w.owns(id));\n if (target) target.postMessage({ type: 'saved', id });\n}\n"},"staged":{"src/sync/broadcast.ts":"import { getWindows } from './windows';\n\nexport function notifySaved(id: string): void {\n const wins = getWindows();\n for (const w of wins) w.postMessage({ type: 'saved', id });\n}\n"}},"note":"The diff broadcasts 'saved' to EVERY window instead of the one that owns the id. Each window handles the message (e.g. re-persists), so the save effect now runs once per open window instead of once. Broadcast needs targeting or dedup.","difficulty":"borderline","provenance":"authored","variantOf":null,"holdout":true} {"id":"corr-decoy-broadcast-targeted","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/sync/broadcast.ts":"import { getWindows } from './windows';\n\nexport function notifySaved(id: string): void {\n const wins = getWindows();\n for (const w of wins) w.postMessage({ type: 'saved', id });\n}\n"},"staged":{"src/sync/broadcast.ts":"import { getWindows } from './windows';\n\nexport function notifySaved(id: string): void {\n const wins = getWindows();\n // Only the owning window handles the effect — no N-way fan-out.\n const target = wins.find((w) => w.owns(id));\n if (target) target.postMessage({ type: 'saved', id });\n}\n"}},"note":"Correct: the diff narrows the broadcast to the single owning window, so the effect runs once. No fan-out duplication. Correctness stays silent.","difficulty":"borderline","provenance":"authored","variantOf":"corr-broadcast-fanout-no-dedup","holdout":true} +{"id":"corr-pr138-ci-breaking-gate-engine-coverage-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/events/dispatch.ts":"const ALLOWED_EVENT_TYPES = ['order.created', 'order.updated', 'order.cancelled'] as const;\ntype EventType = (typeof ALLOWED_EVENT_TYPES)[number];\n\nexport interface RoutableEvent {\n type: string;\n payload: unknown;\n}\n\nexport function routeEvent(event: RoutableEvent): void {\n if (!ALLOWED_EVENT_TYPES.includes(event.type as EventType)) {\n throw new Error(`unrecognized event type: ${event.type}`);\n }\n dispatch(event.type as EventType, event.payload);\n}\n\nfunction dispatch(type: EventType, payload: unknown): void {\n console.log(`dispatching ${type}`, payload);\n}\n","src/events/session-watcher.ts":"// Session lifecycle helper. Timeout handling currently only logs locally.\nexport function onSessionTimeout(sessionId: string): void {\n console.log(`session ${sessionId} timed out`);\n}\n"},"staged":{"src/events/dispatch.ts":"const ALLOWED_EVENT_TYPES = [\n 'order.created',\n 'order.updated',\n 'order.cancelled',\n 'session.expired',\n] as const;\ntype EventType = (typeof ALLOWED_EVENT_TYPES)[number];\n\nexport interface RoutableEvent {\n type: string;\n payload: unknown;\n}\n\nexport function routeEvent(event: RoutableEvent): void {\n if (!ALLOWED_EVENT_TYPES.includes(event.type as EventType)) {\n throw new Error(`unrecognized event type: ${event.type}`);\n }\n dispatch(event.type as EventType, event.payload);\n}\n\nfunction dispatch(type: EventType, payload: unknown): void {\n console.log(`dispatching ${type}`, payload);\n}\n","src/events/session-watcher.ts":"import { routeEvent } from './dispatch';\n\n// Session lifecycle helper. Timeout now also notifies the shared event router.\nexport function onSessionTimeout(sessionId: string): void {\n console.log(`session ${sessionId} timed out`);\n routeEvent({ type: 'session.expired', payload: { sessionId } });\n}\n"}},"note":"Correct: the diff registers 'session.expired' in dispatch.ts's ALLOWED_EVENT_TYPES in the same change that wires the new producer, so the reader recognizes exactly what the writer now emits. No mismatch, no thrown rejection.","difficulty":"clear","provenance":"adapted","source":{"repo":"norvalbv/devkit","pr":138,"url":"https://github.com/norvalbv/devkit/pull/138#discussion_r3610792062"},"caseId":"corr-pr138-ci-breaking-gate-engine-coverage","sourcePr":138,"variantOf":"corr-pr138-ci-breaking-gate-engine-coverage","holdout":false} +{"id":"corr-pr138-ci-breaking-gate-engine-coverage","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["writer-reader-contracts"],"reasonPattern":"unrecognized|allow.?list|not registered|reject|throws?|unknown event|whitelist|stale|mismatch|missing entry","repo":{"base":{"src/events/dispatch.ts":"const ALLOWED_EVENT_TYPES = ['order.created', 'order.updated', 'order.cancelled'] as const;\ntype EventType = (typeof ALLOWED_EVENT_TYPES)[number];\n\nexport interface RoutableEvent {\n type: string;\n payload: unknown;\n}\n\nexport function routeEvent(event: RoutableEvent): void {\n if (!ALLOWED_EVENT_TYPES.includes(event.type as EventType)) {\n throw new Error(`unrecognized event type: ${event.type}`);\n }\n dispatch(event.type as EventType, event.payload);\n}\n\nfunction dispatch(type: EventType, payload: unknown): void {\n console.log(`dispatching ${type}`, payload);\n}\n","src/events/session-watcher.ts":"// Session lifecycle helper. Timeout handling currently only logs locally.\nexport function onSessionTimeout(sessionId: string): void {\n console.log(`session ${sessionId} timed out`);\n}\n"},"staged":{"src/events/dispatch.ts":"const ALLOWED_EVENT_TYPES = ['order.created', 'order.updated', 'order.cancelled'] as const;\ntype EventType = (typeof ALLOWED_EVENT_TYPES)[number];\n\nexport interface RoutableEvent {\n type: string;\n payload: unknown;\n}\n\nexport function routeEvent(event: RoutableEvent): void {\n if (!ALLOWED_EVENT_TYPES.includes(event.type as EventType)) {\n throw new Error(`unrecognized event type: ${event.type}`);\n }\n dispatch(event.type as EventType, event.payload);\n}\n\nfunction dispatch(type: EventType, payload: unknown): void {\n console.log(`dispatching ${type}`, payload);\n}\n","src/events/session-watcher.ts":"import { routeEvent } from './dispatch';\n\n// Session lifecycle helper. Timeout now also notifies the shared event router.\nexport function onSessionTimeout(sessionId: string): void {\n console.log(`session ${sessionId} timed out`);\n routeEvent({ type: 'session.expired', payload: { sessionId } });\n}\n"}},"note":"session-watcher.ts starts emitting a new 'session.expired' event through the shared routeEvent producer, but dispatch.ts's ALLOWED_EVENT_TYPES allowlist was never updated to include it. Every call to onSessionTimeout now throws instead of dispatching, because the reader's registered set is out of sync with what the writer produces. The two files must change together; this diff only touched the producer side.","difficulty":"clear","provenance":"mined","source":{"repo":"norvalbv/devkit","pr":138,"url":"https://github.com/norvalbv/devkit/pull/138#discussion_r3610792062"},"caseId":"corr-pr138-ci-breaking-gate-engine-coverage","sourcePr":138,"holdout":true,"outcomeEvidence":"resolved+line-touched","scopeConfirmed":"unverifiable"} +{"id":"corr-pr172-duplicate-let-spawnoptions-declaration-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/jobs/launch-with-capture.ts":"export interface LaunchOptions {\n mode?: string;\n env?: Record;\n}\n\n// Captures the options a launcher was actually invoked with, so the caller can inspect them once\n// the async launch settles.\nexport async function launchAndCapture(\n runLauncher: (onStart: (opts: LaunchOptions) => void) => Promise,\n): Promise {\n let capturedOptions: LaunchOptions | undefined;\n await runLauncher((opts) => {\n capturedOptions = opts;\n });\n return capturedOptions;\n}\n"},"staged":{"src/jobs/launch-with-capture.ts":"export interface LaunchOptions {\n mode?: string;\n env?: Record;\n}\n\n// Captures the options a launcher was actually invoked with, so the caller can inspect them once\n// the async launch settles. Retry attempts share the same capture target.\nexport async function launchAndCapture(\n runLauncher: (onStart: (opts: LaunchOptions) => void) => Promise,\n): Promise {\n let capturedOptions: LaunchOptions | undefined;\n {\n await runLauncher((opts) => {\n capturedOptions = opts;\n });\n }\n return capturedOptions;\n}\n"}},"note":"Correct: the nested block no longer re-declares `capturedOptions` — there is only ONE binding in the whole function, in the outer scope. The launcher callback writes to that same binding, and the return reads it back, so a genuinely reported value is never discarded.","difficulty":"clear","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":172,"url":"https://github.com/benord-labs/frink/pull/172#discussion_r3651628520"},"caseId":"corr-pr172-duplicate-let-spawnoptions-declaration","sourcePr":172,"variantOf":"corr-pr172-duplicate-let-spawnoptions-declaration","holdout":false} +{"id":"corr-pr172-duplicate-let-spawnoptions-declaration","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["writer-reader-contracts"],"reasonPattern":"shadow|nested block|closure|inner scope|never (get|gets) assigned|stale|outer|discard|lost|undefined","repo":{"base":{"src/jobs/launch-with-capture.ts":"export interface LaunchOptions {\n mode?: string;\n env?: Record;\n}\n\n// Captures the options a launcher was actually invoked with, so the caller can inspect them once\n// the async launch settles.\nexport async function launchAndCapture(\n runLauncher: (onStart: (opts: LaunchOptions) => void) => Promise,\n): Promise {\n let capturedOptions: LaunchOptions | undefined;\n await runLauncher((opts) => {\n capturedOptions = opts;\n });\n return capturedOptions;\n}\n"},"staged":{"src/jobs/launch-with-capture.ts":"export interface LaunchOptions {\n mode?: string;\n env?: Record;\n}\n\n// Captures the options a launcher was actually invoked with, so the caller can inspect them once\n// the async launch settles. Retry attempts get their own local pass before the shared result.\nexport async function launchAndCapture(\n runLauncher: (onStart: (opts: LaunchOptions) => void) => Promise,\n): Promise {\n let capturedOptions: LaunchOptions | undefined;\n {\n let capturedOptions: LaunchOptions | undefined;\n await runLauncher((opts) => {\n capturedOptions = opts;\n });\n }\n return capturedOptions;\n}\n"}},"note":"The block-scoped `{ let capturedOptions ... }` re-declares the same name inside a nested block, shadowing the outer variable. The launcher callback closes over the INNER (shadowed) binding and writes to it, but the function returns the OUTER binding, which is never assigned. Every call now returns undefined even though the launcher genuinely reported options, silently discarding the captured value. This mirrors a real duplicate-declaration finding on a mutable capture variable in the same function.","difficulty":"borderline","provenance":"mined","source":{"repo":"benord-labs/frink","pr":172,"url":"https://github.com/benord-labs/frink/pull/172#discussion_r3651628520"},"caseId":"corr-pr172-duplicate-let-spawnoptions-declaration","sourcePr":172,"holdout":false,"outcomeEvidence":"resolved+line-touched","scopeConfirmed":"unverifiable"} +{"id":"corr-pr200-important-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"web/panel/active-items.ts":"import { useQuery } from '../lib/query';\n\nconst EMPTY: Item[] = [];\n\n// listActive's underlying transport can briefly resolve to a placeholder object while it\n// reconnects; every consumer here normalizes before use.\nexport function useActiveItems(sessionExpired: boolean): Item[] {\n const { data: rawItems } = useQuery('items.listActive', { enabled: !sessionExpired });\n return Array.isArray(rawItems) ? rawItems : EMPTY;\n}\n"},"staged":{"web/panel/active-items.ts":"import { useQuery } from '../lib/query';\n\nconst EMPTY: Item[] = [];\n\n// listActive's underlying transport can briefly resolve to a placeholder object while it\n// reconnects; every consumer here normalizes before use.\nexport function useActiveItems(sessionExpired: boolean): Item[] {\n const { data: rawItems } = useQuery('items.listActive', { enabled: !sessionExpired });\n return Array.isArray(rawItems) ? rawItems : EMPTY;\n}\n\nexport function useArchivedItems(sessionExpired: boolean): Item[] {\n const { data: rawItems } = useQuery('items.listArchived', { enabled: !sessionExpired });\n return Array.isArray(rawItems) ? rawItems : EMPTY;\n}\n"}},"note":"Fixed: useArchivedItems now normalizes with Array.isArray, matching useActiveItems, so the transient non-array placeholder the transport can briefly return is caught the same way for both queries before any caller iterates the result.","difficulty":"clear","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":200,"url":"https://github.com/benord-labs/frink/pull/200#discussion_r3669675817"},"caseId":"corr-pr200-important","sourcePr":200,"variantOf":"corr-pr200-important","holdout":false} +{"id":"corr-pr200-important","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"array|isarray|guard|throw|filter|fallback|coalesc|shape|omit","repo":{"base":{"web/panel/active-items.ts":"import { useQuery } from '../lib/query';\n\nconst EMPTY: Item[] = [];\n\n// listActive's underlying transport can briefly resolve to a placeholder object while it\n// reconnects; every consumer here normalizes before use.\nexport function useActiveItems(sessionExpired: boolean): Item[] {\n const { data: rawItems } = useQuery('items.listActive', { enabled: !sessionExpired });\n return Array.isArray(rawItems) ? rawItems : EMPTY;\n}\n"},"staged":{"web/panel/active-items.ts":"import { useQuery } from '../lib/query';\n\nconst EMPTY: Item[] = [];\n\n// listActive's underlying transport can briefly resolve to a placeholder object while it\n// reconnects; every consumer here normalizes before use.\nexport function useActiveItems(sessionExpired: boolean): Item[] {\n const { data: rawItems } = useQuery('items.listActive', { enabled: !sessionExpired });\n return Array.isArray(rawItems) ? rawItems : EMPTY;\n}\n\nexport function useArchivedItems(sessionExpired: boolean): Item[] {\n const { data: rawItems } = useQuery('items.listArchived', { enabled: !sessionExpired });\n return rawItems ?? EMPTY;\n}\n"}},"note":"useArchivedItems switched from the sibling useActiveItems' Array.isArray normalization to a plain nullish-coalescing fallback, so it only guards against undefined. The doc comment says the query's raw data can briefly be a non-array placeholder while the transport reconnects; when that happens here, the un-normalized value flows straight to a caller that expects an array and will throw on the first array method it calls.","difficulty":"clear","provenance":"mined","source":{"repo":"benord-labs/frink","pr":200,"url":"https://github.com/benord-labs/frink/pull/200#discussion_r3669675817"},"caseId":"corr-pr200-important","sourcePr":200,"variantOf":null,"holdout":true,"outcomeEvidence":"addressed-marker","scopeConfirmed":"confirmed"} +{"id":"corr-pr200-ref-mutated-during-render-move-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"web/hooks/use-window-event.ts":"import { useEffect } from 'react';\n\n// Subscribes to a window event for the lifetime of the component; re-subscribes whenever\n// eventName or the handler identity changes.\nexport function useWindowEvent(eventName: string, handler: (event: Event) => void): void {\n useEffect(() => {\n window.addEventListener(eventName, handler);\n return () => window.removeEventListener(eventName, handler);\n }, [eventName, handler]);\n}\n"},"staged":{"web/hooks/use-window-event.ts":"import { useEffect, useRef } from 'react';\n\n// Subscribes to a window event for the lifetime of the component. The latest handler is kept in\n// a ref so a caller may pass an inline arrow without re-subscribing on every render.\nexport function useWindowEvent(eventName: string, handler: (event: Event) => void): void {\n const handlerRef = useRef(handler);\n\n useEffect(() => {\n handlerRef.current = handler;\n });\n\n useEffect(() => {\n const listener = (event: Event): void => handlerRef.current(event);\n window.addEventListener(eventName, listener);\n return () => window.removeEventListener(eventName, listener);\n }, [eventName]);\n}\n"}},"note":"Fixed: the ref write moved into its own dependency-free useEffect, so it only runs post-commit. A discarded or replayed render can no longer leak a write, and the subscription effect still keys only on eventName, so a changing handler identity does not force a re-subscribe.","difficulty":"clear","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":200,"url":"https://github.com/benord-labs/frink/pull/200#discussion_r3669675832"},"caseId":"corr-pr200-ref-mutated-during-render-move","sourcePr":200,"variantOf":"corr-pr200-ref-mutated-during-render-move","holdout":false} +{"id":"corr-pr200-ref-mutated-during-render-move","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["concurrency-races"],"reasonPattern":"render|replay|discard|impure|pure|mutat|commit|effect|leak","repo":{"base":{"web/hooks/use-window-event.ts":"import { useEffect } from 'react';\n\n// Subscribes to a window event for the lifetime of the component; re-subscribes whenever\n// eventName or the handler identity changes.\nexport function useWindowEvent(eventName: string, handler: (event: Event) => void): void {\n useEffect(() => {\n window.addEventListener(eventName, handler);\n return () => window.removeEventListener(eventName, handler);\n }, [eventName, handler]);\n}\n"},"staged":{"web/hooks/use-window-event.ts":"import { useEffect, useRef } from 'react';\n\n// Subscribes to a window event for the lifetime of the component. The latest handler is kept in\n// a ref so a caller may pass an inline arrow without a fresh subscription on every pass.\nexport function useWindowEvent(eventName: string, handler: (event: Event) => void): void {\n const handlerRef = useRef(handler);\n handlerRef.current = handler;\n\n useEffect(() => {\n const listener = (event: Event): void => handlerRef.current(event);\n window.addEventListener(eventName, listener);\n return () => window.removeEventListener(eventName, listener);\n }, [eventName]);\n}\n"}},"note":"handlerRef.current = handler executes directly in the render body, which violates React's render-purity contract: a render can be started, replayed, or discarded without committing, so the ref write can leak from work that never actually commits to the screen. The write must happen post-render, inside its own dependency-free effect, not inline during render.","difficulty":"clear","provenance":"mined","source":{"repo":"benord-labs/frink","pr":200,"url":"https://github.com/benord-labs/frink/pull/200#discussion_r3669675832"},"caseId":"corr-pr200-ref-mutated-during-render-move","sourcePr":200,"variantOf":null,"holdout":false,"outcomeEvidence":"addressed-marker","scopeConfirmed":"confirmed"} +{"id":"corr-pr21-fail-closed-when-the-remote-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/cli/ship-branch.ts":"import { execFileSync } from 'node:child_process';\n\n// Creates a new local branch and pushes it upstream under the same name.\nexport function shipBranch(branchName: string): void {\n execFileSync('git', ['checkout', '-b', branchName]);\n execFileSync('git', ['push', '-u', 'origin', branchName]);\n}\n"},"staged":{"src/cli/ship-branch.ts":"import { execFileSync } from 'node:child_process';\n\n// Creates a new local branch and pushes it upstream, rejecting first if a remote branch of the\n// same name is already in use.\nexport function shipBranch(branchName: string): void {\n execFileSync('git', ['checkout', '-b', branchName]);\n if (remoteBranchExists(branchName)) {\n throw new Error(`remote branch already exists: origin/${branchName}`);\n }\n execFileSync('git', ['push', '-u', 'origin', branchName]);\n}\n\n// git ls-remote --exit-code exits 0 on a match, 2 on \"no match\", and anything else on a genuine\n// lookup failure (network, auth, timeout) — those three cases are not interchangeable.\nfunction remoteBranchExists(branchName: string): boolean {\n try {\n execFileSync('git', ['ls-remote', '--exit-code', '--heads', 'origin', branchName]);\n return true;\n } catch (err) {\n if ((err as { status?: number }).status === 2) return false;\n throw new Error(`could not verify remote branch does not exist: origin/${branchName}`);\n }\n}\n"}},"note":"Fixed: remoteBranchExists now inspects the child process's exit status and only treats status 2 (git ls-remote's documented \"no match\" code) as absence. Any other failure — network, auth, timeout — rethrows instead of being swallowed, so shipBranch aborts instead of pushing onto a branch it never actually confirmed was free.","difficulty":"borderline","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":21,"url":"https://github.com/benord-labs/frink/pull/21#discussion_r3484655345"},"caseId":"corr-pr21-fail-closed-when-the-remote","sourcePr":21,"variantOf":"corr-pr21-fail-closed-when-the-remote","holdout":false} +{"id":"corr-pr21-fail-closed-when-the-remote","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"exit code|non-zero|network|closed|open|error|treat|distinguish|collide","repo":{"base":{"src/cli/ship-branch.ts":"import { execFileSync } from 'node:child_process';\n\n// Creates a new local branch and pushes it upstream under the same name.\nexport function shipBranch(branchName: string): void {\n execFileSync('git', ['checkout', '-b', branchName]);\n execFileSync('git', ['push', '-u', 'origin', branchName]);\n}\n"},"staged":{"src/cli/ship-branch.ts":"import { execFileSync } from 'node:child_process';\n\n// Creates a new local branch and pushes it upstream, rejecting first if a remote branch of the\n// same name is already in use.\nexport function shipBranch(branchName: string): void {\n execFileSync('git', ['checkout', '-b', branchName]);\n if (remoteBranchExists(branchName)) {\n throw new Error(`remote branch already exists: origin/${branchName}`);\n }\n execFileSync('git', ['push', '-u', 'origin', branchName]);\n}\n\nfunction remoteBranchExists(branchName: string): boolean {\n try {\n execFileSync('git', ['ls-remote', '--exit-code', '--heads', 'origin', branchName]);\n return true;\n } catch {\n return false;\n }\n}\n"}},"note":"git ls-remote --exit-code returns non-zero both when the branch is genuinely absent and when the lookup itself fails (network blip, auth, timeout). remoteBranchExists collapses every thrown error into false, so a transient lookup failure is indistinguishable from \"branch not found\" and shipBranch proceeds to push -u, which can silently append onto an existing remote branch/PR. The failure classes must be told apart, and a lookup error must fail closed (abort), not open.","difficulty":"borderline","provenance":"mined","source":{"repo":"benord-labs/frink","pr":21,"url":"https://github.com/benord-labs/frink/pull/21#discussion_r3484655345"},"caseId":"corr-pr21-fail-closed-when-the-remote","sourcePr":21,"variantOf":null,"holdout":true,"outcomeEvidence":"resolved+line-touched","scopeConfirmed":"unverifiable"} +{"id":"corr-pr21-validate-the-parsed-github-repo-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"api/release/publish.ts":"import { execSync } from 'node:child_process';\n\n// Packages the release bundle locally; remote publishing lands in a later step.\nexport function publishRelease(branch: string): void {\n execSync('npm pack');\n console.log(`packaged release bundle for ${branch}`);\n}\n"},"staged":{"api/release/publish.ts":"import { execSync } from 'node:child_process';\n\nconst TARGET_RE = /^[\\w.-]+\\/[\\w.-]+$/;\n\n// Derives the deploy target from the remote URL, then ships the branch upstream.\nfunction parseTarget(remoteUrl: string): string {\n const target = remoteUrl.replace(/^.*github\\.com[:/]/, '').replace(/\\.git$/, '');\n if (!TARGET_RE.test(target)) {\n throw new Error(`could not resolve deploy target: ${target}`);\n }\n return target;\n}\n\nexport function publishRelease(remoteUrl: string, branch: string): void {\n const target = parseTarget(remoteUrl);\n execSync(`git push origin ${branch}`);\n execSync(`gh pr create --repo ${target} --head ${branch}`);\n}\n"}},"note":"parseTarget now checks the derived target against an owner/repo shape and throws before publishRelease does anything network-visible. A malformed remote URL is rejected up front instead of surfacing only after the branch has already been pushed, so there is no correctness defect left to flag here.","difficulty":"clear","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":21,"url":"https://github.com/benord-labs/frink/pull/21#discussion_r3484655350"},"caseId":"corr-pr21-validate-the-parsed-github-repo","sourcePr":21,"variantOf":"corr-pr21-validate-the-parsed-github-repo","holdout":false} +{"id":"corr-pr21-validate-the-parsed-github-repo","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"malformed|unvalidated|invalid target|garbage|shape check|before push|silently proceed|not verified|regex leftover","repo":{"base":{"api/release/publish.ts":"import { execSync } from 'node:child_process';\n\n// Packages the release bundle locally; remote publishing lands in a later step.\nexport function publishRelease(branch: string): void {\n execSync('npm pack');\n console.log(`packaged release bundle for ${branch}`);\n}\n"},"staged":{"api/release/publish.ts":"import { execSync } from 'node:child_process';\n\n// Derives the deploy target from the remote URL, then ships the branch upstream.\nfunction parseTarget(remoteUrl: string): string {\n return remoteUrl.replace(/^.*github\\.com[:/]/, '').replace(/\\.git$/, '');\n}\n\nexport function publishRelease(remoteUrl: string, branch: string): void {\n const target = parseTarget(remoteUrl);\n execSync(`git push origin ${branch}`);\n execSync(`gh pr create --repo ${target} --head ${branch}`);\n}\n"}},"note":"parseTarget derives the deploy target from remoteUrl with a plain regex substitution and never checks the result's shape. When remoteUrl doesn't match one of the expected host forms, parseTarget returns whatever is left over instead of a clean owner/repo slug, and publishRelease has already pushed the branch before the malformed target surfaces as a failure from gh pr create. The target's shape needs validating right after parsing, before the push runs.","difficulty":"clear","provenance":"mined","source":{"repo":"benord-labs/frink","pr":21,"url":"https://github.com/benord-labs/frink/pull/21#discussion_r3484655350"},"caseId":"corr-pr21-validate-the-parsed-github-repo","sourcePr":21,"variantOf":null,"holdout":false,"outcomeEvidence":"resolved+line-touched","scopeConfirmed":"unverifiable"} +{"id":"corr-pr26-cover-the-restart-interrupted-flow-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/runtime/dispatch.ts":"export type TurnContext = {\n flowRunId: string | null;\n restartRunId: string | null;\n};\n\nfunction runTool(toolName: string, ctx: TurnContext): unknown {\n return { toolName, ran: true };\n}\n\nexport function dispatchTool(toolName: string, ctx: TurnContext): unknown {\n return runTool(toolName, ctx);\n}\n"},"staged":{"src/runtime/dispatch.ts":"export type TurnContext = {\n flowRunId: string | null;\n restartRunId: string | null;\n};\n\nfunction isFlowDrivenTurn(ctx: TurnContext): boolean {\n return ctx.flowRunId != null || ctx.restartRunId != null;\n}\n\nfunction denyAskInFlow(active: boolean): unknown {\n return active ? { blocked: true } : null;\n}\n\nfunction runTool(toolName: string, ctx: TurnContext): unknown {\n return { toolName, ran: true };\n}\n\nexport function dispatchTool(toolName: string, ctx: TurnContext): unknown {\n if (toolName === 'AskUserQuestion') {\n const denied = denyAskInFlow(isFlowDrivenTurn(ctx));\n if (denied) return denied;\n }\n return runTool(toolName, ctx);\n}\n"}},"note":"isFlowDrivenTurn now treats either flowRunId or restartRunId as evidence of a flow-driven turn, so a turn resuming after a restart is classified the same as a normal in-progress flow turn and AskUserQuestion is denied on both paths. There is no missed state-transition left to flag.","difficulty":"borderline","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":26,"url":"https://github.com/benord-labs/frink/pull/26#discussion_r3486608263"},"caseId":"corr-pr26-cover-the-restart-interrupted-flow","sourcePr":26,"variantOf":"corr-pr26-cover-the-restart-interrupted-flow","holdout":false} +{"id":"corr-pr26-cover-the-restart-interrupted-flow","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["state-transitions"],"reasonPattern":"restart|resume|resumed|interrupted|recover|missed state|stall|timeout|flow.?driven|bypass","repo":{"base":{"src/runtime/dispatch.ts":"export type TurnContext = {\n flowRunId: string | null;\n restartRunId: string | null;\n};\n\nfunction runTool(toolName: string, ctx: TurnContext): unknown {\n return { toolName, ran: true };\n}\n\nexport function dispatchTool(toolName: string, ctx: TurnContext): unknown {\n return runTool(toolName, ctx);\n}\n"},"staged":{"src/runtime/dispatch.ts":"export type TurnContext = {\n flowRunId: string | null;\n restartRunId: string | null;\n};\n\nfunction isFlowDrivenTurn(ctx: TurnContext): boolean {\n return ctx.flowRunId != null;\n}\n\nfunction denyAskInFlow(active: boolean): unknown {\n return active ? { blocked: true } : null;\n}\n\nfunction runTool(toolName: string, ctx: TurnContext): unknown {\n return { toolName, ran: true };\n}\n\nexport function dispatchTool(toolName: string, ctx: TurnContext): unknown {\n if (toolName === 'AskUserQuestion') {\n const denied = denyAskInFlow(isFlowDrivenTurn(ctx));\n if (denied) return denied;\n }\n return runTool(toolName, ctx);\n}\n"}},"note":"isFlowDrivenTurn only treats ctx.flowRunId as evidence of a flow-driven turn, but a turn that resumes after a restart carries that context in ctx.restartRunId instead, leaving flowRunId null. dispatchTool's gate never fires for a restart-resumed turn, so AskUserQuestion is allowed through and can block on its user-facing timeout even though the turn is really executing inside a recovered flow. isFlowDrivenTurn needs to treat restartRunId as flow-driven too.","difficulty":"borderline","provenance":"mined","source":{"repo":"benord-labs/frink","pr":26,"url":"https://github.com/benord-labs/frink/pull/26#discussion_r3486608263"},"caseId":"corr-pr26-cover-the-restart-interrupted-flow","sourcePr":26,"variantOf":null,"holdout":true,"outcomeEvidence":"resolved+line-touched","scopeConfirmed":"unverifiable"} +{"id":"corr-pr26-keep-the-mcp-inputschema-in-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"api/tools/task-signal-schema.ts":"// Static shape returned by tools/list for this tool.\nexport const taskSignalSchema = {\n type: 'object' as const,\n properties: {\n state: { type: 'string' },\n summary: { type: 'string' },\n },\n required: ['state', 'summary'],\n};\n","api/tools/task-signal-parser.ts":"import { z } from 'zod';\n\nconst taskSignalInput = z.object({\n state: z.string(),\n summary: z.string(),\n});\n\nexport function parseTaskSignal(raw: unknown) {\n return taskSignalInput.safeParse(raw);\n}\n"},"staged":{"api/tools/task-signal-schema.ts":"// Static shape returned by tools/list for this tool.\nexport const taskSignalSchema = {\n type: 'object' as const,\n properties: {\n state: { type: 'string' },\n summary: { type: 'string' },\n prompts: {\n type: 'array',\n maxItems: 10,\n items: {\n type: 'object',\n properties: {\n choices: { type: 'array', minItems: 1, maxItems: 20, items: { type: 'string' } },\n },\n },\n },\n },\n required: ['state', 'summary'],\n};\n","api/tools/task-signal-parser.ts":"import { z } from 'zod';\n\nconst taskSignalInput = z.object({\n state: z.string(),\n summary: z.string(),\n prompts: z\n .array(\n z.object({\n choices: z.array(z.string()).min(1).max(20),\n }),\n )\n .max(10)\n .optional(),\n});\n\nexport function parseTaskSignal(raw: unknown) {\n return taskSignalInput.safeParse(raw);\n}\n"}},"note":"taskSignalSchema now carries the same maxItems/minItems bounds parseTaskSignal enforces at runtime, so a call that passes discovery can no longer be rejected by the handler for exceeding a limit the schema never advertised. Discovery and execution agree on one contract, so there is no correctness defect left to flag.","difficulty":"clear","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":26,"url":"https://github.com/benord-labs/frink/pull/26#discussion_r3486608255"},"caseId":"corr-pr26-keep-the-mcp-inputschema-in","sourcePr":26,"variantOf":"corr-pr26-keep-the-mcp-inputschema-in","holdout":false} +{"id":"corr-pr26-keep-the-mcp-inputschema-in","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["writer-reader-contracts"],"reasonPattern":"out of sync|mismatch|bounds|max items|min items|maxitems|minitems|advertise|discovery|drift","repo":{"base":{"api/tools/task-signal-schema.ts":"// Static shape returned by tools/list for this tool.\nexport const taskSignalSchema = {\n type: 'object' as const,\n properties: {\n state: { type: 'string' },\n summary: { type: 'string' },\n },\n required: ['state', 'summary'],\n};\n","api/tools/task-signal-parser.ts":"import { z } from 'zod';\n\nconst taskSignalInput = z.object({\n state: z.string(),\n summary: z.string(),\n});\n\nexport function parseTaskSignal(raw: unknown) {\n return taskSignalInput.safeParse(raw);\n}\n"},"staged":{"api/tools/task-signal-schema.ts":"// Static shape returned by tools/list for this tool.\nexport const taskSignalSchema = {\n type: 'object' as const,\n properties: {\n state: { type: 'string' },\n summary: { type: 'string' },\n prompts: {\n type: 'array',\n items: {\n type: 'object',\n properties: {\n choices: { type: 'array', items: { type: 'string' } },\n },\n },\n },\n },\n required: ['state', 'summary'],\n};\n","api/tools/task-signal-parser.ts":"import { z } from 'zod';\n\nconst taskSignalInput = z.object({\n state: z.string(),\n summary: z.string(),\n prompts: z\n .array(\n z.object({\n choices: z.array(z.string()).min(1).max(20),\n }),\n )\n .max(10)\n .optional(),\n});\n\nexport function parseTaskSignal(raw: unknown) {\n return taskSignalInput.safeParse(raw);\n}\n"}},"note":"taskSignalSchema advertises prompts as an unbounded array of unbounded choice lists, but parseTaskSignal enforces at most 10 prompts and between 1 and 20 choices each. A caller relying only on the discovery schema can build a call that passes discovery and then gets rejected at execution because the two contracts diverge. The schema needs the same maxItems/minItems bounds the parser already enforces.","difficulty":"clear","provenance":"mined","source":{"repo":"benord-labs/frink","pr":26,"url":"https://github.com/benord-labs/frink/pull/26#discussion_r3486608255"},"caseId":"corr-pr26-keep-the-mcp-inputschema-in","sourcePr":26,"variantOf":null,"holdout":false,"outcomeEvidence":"resolved+line-touched","scopeConfirmed":"unverifiable"} +{"id":"corr-pr38-check-the-full-r2-credential-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"api/publish/finalize-release.ts":"export async function finalizeRelease(): Promise {\n // notification dispatch lands in a later step\n}\n","api/publish/webhook-client.ts":"export interface WebhookAuth {\n clientId: string;\n clientSecret: string;\n accountId: string;\n}\n\nexport async function sendWebhook(auth: WebhookAuth, payload: unknown): Promise {\n if (!auth.clientId || !auth.clientSecret || !auth.accountId) {\n throw new Error('webhook client rejected incomplete auth');\n }\n await fetch('/internal/dispatch', {\n method: 'POST',\n headers: { 'x-client-id': auth.clientId },\n body: JSON.stringify(payload),\n });\n}\n"},"staged":{"api/publish/finalize-release.ts":"import { sendWebhook, type WebhookAuth } from './webhook-client';\n\nexport async function finalizeRelease(env: NodeJS.ProcessEnv): Promise {\n if (env.HOOK_CLIENT_ID && env.HOOK_CLIENT_SECRET && env.HOOK_ACCOUNT_ID) {\n const auth: WebhookAuth = {\n clientId: env.HOOK_CLIENT_ID,\n clientSecret: env.HOOK_CLIENT_SECRET,\n accountId: env.HOOK_ACCOUNT_ID,\n };\n await sendWebhook(auth, { event: 'release.published' });\n }\n}\n","api/publish/webhook-client.ts":"export interface WebhookAuth {\n clientId: string;\n clientSecret: string;\n accountId: string;\n}\n\nexport async function sendWebhook(auth: WebhookAuth, payload: unknown): Promise {\n if (!auth.clientId || !auth.clientSecret || !auth.accountId) {\n throw new Error('webhook client rejected incomplete auth');\n }\n await fetch('/internal/dispatch', {\n method: 'POST',\n headers: { 'x-client-id': auth.clientId },\n body: JSON.stringify(payload),\n });\n}\n"}},"note":"finalizeRelease now requires the full webhook credential set (clientId, clientSecret, accountId) before calling sendWebhook, so a partially configured repo skips the notification cleanly instead of crashing on a rejected auth object.","difficulty":"clear","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":38,"url":"https://github.com/benord-labs/frink/pull/38#discussion_r3501128595"},"caseId":"corr-pr38-check-the-full-r2-credential","sourcePr":38,"variantOf":"corr-pr38-check-the-full-r2-credential","holdout":false} +{"id":"corr-pr38-check-the-full-r2-credential","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"credential|partial|incomplete|webhook|missing|fallback|precondition|requires|throw|skip","repo":{"base":{"api/publish/finalize-release.ts":"export async function finalizeRelease(): Promise {\n // notification dispatch lands in a later step\n}\n","api/publish/webhook-client.ts":"export interface WebhookAuth {\n clientId: string;\n clientSecret: string;\n accountId: string;\n}\n\nexport async function sendWebhook(auth: WebhookAuth, payload: unknown): Promise {\n if (!auth.clientId || !auth.clientSecret || !auth.accountId) {\n throw new Error('webhook client rejected incomplete auth');\n }\n await fetch('/internal/dispatch', {\n method: 'POST',\n headers: { 'x-client-id': auth.clientId },\n body: JSON.stringify(payload),\n });\n}\n"},"staged":{"api/publish/finalize-release.ts":"import { sendWebhook, type WebhookAuth } from './webhook-client';\n\nexport async function finalizeRelease(env: NodeJS.ProcessEnv): Promise {\n if (env.HOOK_CLIENT_ID) {\n const auth: WebhookAuth = {\n clientId: env.HOOK_CLIENT_ID,\n clientSecret: env.HOOK_CLIENT_SECRET ?? '',\n accountId: env.HOOK_ACCOUNT_ID ?? '',\n };\n await sendWebhook(auth, { event: 'release.published' });\n }\n}\n","api/publish/webhook-client.ts":"export interface WebhookAuth {\n clientId: string;\n clientSecret: string;\n accountId: string;\n}\n\nexport async function sendWebhook(auth: WebhookAuth, payload: unknown): Promise {\n if (!auth.clientId || !auth.clientSecret || !auth.accountId) {\n throw new Error('webhook client rejected incomplete auth');\n }\n await fetch('/internal/dispatch', {\n method: 'POST',\n headers: { 'x-client-id': auth.clientId },\n body: JSON.stringify(payload),\n });\n}\n"}},"note":"finalizeRelease only checks env.HOOK_CLIENT_ID before calling sendWebhook, but sendWebhook requires clientSecret and accountId too and throws when either is missing. A repo with only HOOK_CLIENT_ID configured takes the dispatch branch and crashes the release step instead of skipping the notification cleanly.","difficulty":"clear","provenance":"mined","source":{"repo":"benord-labs/frink","pr":38,"url":"https://github.com/benord-labs/frink/pull/38#discussion_r3501128595"},"caseId":"corr-pr38-check-the-full-r2-credential","sourcePr":38,"variantOf":null,"holdout":true,"outcomeEvidence":"resolved+line-touched","scopeConfirmed":"unverifiable"} +{"id":"corr-pr38-require-the-full-azure-signing-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"api/publish/package-build.ts":"import { packageUnsigned } from './packager';\n\nexport async function packageBuild(): Promise {\n await packageUnsigned();\n}\n","api/publish/signer.ts":"export interface SigningConfig {\n account: string;\n profile: string;\n endpoint: string;\n publisher: string;\n}\n\nexport async function signArtifact(config: SigningConfig): Promise {\n if (!config.account || !config.profile || !config.endpoint || !config.publisher) {\n throw new Error('signing service rejected incomplete credentials');\n }\n await fetch('/internal/sign', { method: 'POST', body: JSON.stringify(config) });\n}\n"},"staged":{"api/publish/package-build.ts":"import { packageUnsigned } from './packager';\nimport { signArtifact, type SigningConfig } from './signer';\n\nexport async function packageBuild(env: NodeJS.ProcessEnv): Promise {\n if (env.SIGN_ACCOUNT && env.SIGN_PROFILE && env.SIGN_ENDPOINT && env.SIGN_PUBLISHER) {\n const config: SigningConfig = {\n account: env.SIGN_ACCOUNT,\n profile: env.SIGN_PROFILE,\n endpoint: env.SIGN_ENDPOINT,\n publisher: env.SIGN_PUBLISHER,\n };\n await signArtifact(config);\n } else {\n await packageUnsigned();\n }\n}\n","api/publish/signer.ts":"export interface SigningConfig {\n account: string;\n profile: string;\n endpoint: string;\n publisher: string;\n}\n\nexport async function signArtifact(config: SigningConfig): Promise {\n if (!config.account || !config.profile || !config.endpoint || !config.publisher) {\n throw new Error('signing service rejected incomplete credentials');\n }\n await fetch('/internal/sign', { method: 'POST', body: JSON.stringify(config) });\n}\n"}},"note":"packageBuild now requires every signing input (account, profile, endpoint, publisher) before taking the signing branch, so a partially provisioned environment falls back to the unsigned package instead of crashing mid-release. The gate condition and signArtifact's own requirements agree.","difficulty":"clear","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":38,"url":"https://github.com/benord-labs/frink/pull/38#discussion_r3501128582"},"caseId":"corr-pr38-require-the-full-azure-signing","sourcePr":38,"variantOf":"corr-pr38-require-the-full-azure-signing","holdout":false} +{"id":"corr-pr38-require-the-full-azure-signing","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"credential|partial|incomplete|signing|missing|fallback|precondition|requires|throw|unsigned","repo":{"base":{"api/publish/package-build.ts":"import { packageUnsigned } from './packager';\n\nexport async function packageBuild(): Promise {\n await packageUnsigned();\n}\n","api/publish/signer.ts":"export interface SigningConfig {\n account: string;\n profile: string;\n endpoint: string;\n publisher: string;\n}\n\nexport async function signArtifact(config: SigningConfig): Promise {\n if (!config.account || !config.profile || !config.endpoint || !config.publisher) {\n throw new Error('signing service rejected incomplete credentials');\n }\n await fetch('/internal/sign', { method: 'POST', body: JSON.stringify(config) });\n}\n"},"staged":{"api/publish/package-build.ts":"import { packageUnsigned } from './packager';\nimport { signArtifact, type SigningConfig } from './signer';\n\nexport async function packageBuild(env: NodeJS.ProcessEnv): Promise {\n if (env.SIGN_ACCOUNT) {\n const config: SigningConfig = {\n account: env.SIGN_ACCOUNT,\n profile: env.SIGN_PROFILE ?? '',\n endpoint: env.SIGN_ENDPOINT ?? '',\n publisher: env.SIGN_PUBLISHER ?? '',\n };\n await signArtifact(config);\n } else {\n await packageUnsigned();\n }\n}\n","api/publish/signer.ts":"export interface SigningConfig {\n account: string;\n profile: string;\n endpoint: string;\n publisher: string;\n}\n\nexport async function signArtifact(config: SigningConfig): Promise {\n if (!config.account || !config.profile || !config.endpoint || !config.publisher) {\n throw new Error('signing service rejected incomplete credentials');\n }\n await fetch('/internal/sign', { method: 'POST', body: JSON.stringify(config) });\n}\n"}},"note":"packageBuild only checks env.SIGN_ACCOUNT before taking the signing branch, but signArtifact requires the full credential set (profile, endpoint, publisher) and throws when any of them is missing. A repo with only SIGN_ACCOUNT provisioned takes the signing branch and crashes the release instead of falling back to the unsigned package. The gate must require every signing input, not just one.","difficulty":"clear","provenance":"mined","source":{"repo":"benord-labs/frink","pr":38,"url":"https://github.com/benord-labs/frink/pull/38#discussion_r3501128582"},"caseId":"corr-pr38-require-the-full-azure-signing","sourcePr":38,"variantOf":null,"holdout":false,"outcomeEvidence":"resolved+line-touched","scopeConfirmed":"unverifiable"} +{"id":"corr-pr48-cn-ts-src-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"web/ui/button.tsx":"import { cn } from './cn';\n\nconst VARIANT_CLASSES: Record<'default' | 'ghost', string> = {\n default: 'bg-brand text-brand-fg',\n ghost: 'bg-transparent text-foreground',\n};\n\nconst SIZE_CLASSES: Record<'default' | 'sm', string> = {\n default: 'h-9 rounded-md px-4',\n sm: 'h-7 rounded-md px-3',\n};\n\nexport function buttonClassName(\n variant: keyof typeof VARIANT_CLASSES,\n size: keyof typeof SIZE_CLASSES,\n className?: string,\n): string {\n const base = 'inline-flex items-center justify-center rounded-md text-sm font-medium';\n return cn(base, VARIANT_CLASSES[variant], SIZE_CLASSES[size], className ?? '');\n}\n","web/ui/cn.ts":"// Combine class lists into a single deduped string.\nexport function cn(...lists: string[]): string {\n const resolved = new Map();\n for (const list of lists) {\n for (const cls of list.split(/\\s+/).filter(Boolean)) {\n const key = cls.replace(/-[^-]+$/, '');\n resolved.set(key, cls);\n }\n }\n return [...resolved.values()].join(' ');\n}\n"},"staged":{"web/ui/button.tsx":"import { cn } from './cn';\n\nconst VARIANT_CLASSES: Record<'default' | 'ghost', string> = {\n default: 'bg-brand rounded-full text-brand-fg',\n ghost: 'bg-transparent text-foreground',\n};\n\nconst SIZE_CLASSES: Record<'default' | 'sm', string> = {\n default: 'h-9 px-4',\n sm: 'h-7 px-3',\n};\n\nexport function buttonClassName(\n variant: keyof typeof VARIANT_CLASSES,\n size: keyof typeof SIZE_CLASSES,\n className?: string,\n): string {\n const base = 'inline-flex items-center justify-center rounded-md text-sm font-medium';\n return cn(base, VARIANT_CLASSES[variant], SIZE_CLASSES[size], className ?? '');\n}\n","web/ui/cn.ts":"// Combine class lists into a single deduped string.\nexport function cn(...lists: string[]): string {\n const resolved = new Map();\n for (const list of lists) {\n for (const cls of list.split(/\\s+/).filter(Boolean)) {\n const key = cls.replace(/-[^-]+$/, '');\n resolved.set(key, cls);\n }\n }\n return [...resolved.values()].join(' ');\n}\n"}},"note":"SIZE_CLASSES no longer declares a rounded utility at all, so the default variant's rounded-full class is the last (and only) write to that group and survives the cn merge; non-default variants still fall back untouched to the base rounded-md. Writer (variant) and reader (merge order) now agree.","difficulty":"borderline","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":48,"url":"https://github.com/benord-labs/frink/pull/48#discussion_r3505778670"},"caseId":"corr-pr48-cn-ts-src","sourcePr":48,"variantOf":"corr-pr48-cn-ts-src","holdout":false} +{"id":"corr-pr48-cn-ts-src","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["writer-reader-contracts"],"reasonPattern":"rounded|override|precedence|merge order|last.?write|conflict|variant|pill|class order|shape","repo":{"base":{"web/ui/button.tsx":"import { cn } from './cn';\n\nconst VARIANT_CLASSES: Record<'default' | 'ghost', string> = {\n default: 'bg-brand text-brand-fg',\n ghost: 'bg-transparent text-foreground',\n};\n\nconst SIZE_CLASSES: Record<'default' | 'sm', string> = {\n default: 'h-9 rounded-md px-4',\n sm: 'h-7 rounded-md px-3',\n};\n\nexport function buttonClassName(\n variant: keyof typeof VARIANT_CLASSES,\n size: keyof typeof SIZE_CLASSES,\n className?: string,\n): string {\n const base = 'inline-flex items-center justify-center rounded-md text-sm font-medium';\n return cn(base, VARIANT_CLASSES[variant], SIZE_CLASSES[size], className ?? '');\n}\n","web/ui/cn.ts":"// Combine class lists into a single deduped string.\nexport function cn(...lists: string[]): string {\n const resolved = new Map();\n for (const list of lists) {\n for (const cls of list.split(/\\s+/).filter(Boolean)) {\n const key = cls.replace(/-[^-]+$/, '');\n resolved.set(key, cls);\n }\n }\n return [...resolved.values()].join(' ');\n}\n"},"staged":{"web/ui/button.tsx":"import { cn } from './cn';\n\nconst VARIANT_CLASSES: Record<'default' | 'ghost', string> = {\n default: 'bg-brand rounded-full text-brand-fg',\n ghost: 'bg-transparent text-foreground',\n};\n\nconst SIZE_CLASSES: Record<'default' | 'sm', string> = {\n default: 'h-9 rounded-md px-4',\n sm: 'h-7 rounded-md px-3',\n};\n\nexport function buttonClassName(\n variant: keyof typeof VARIANT_CLASSES,\n size: keyof typeof SIZE_CLASSES,\n className?: string,\n): string {\n const base = 'inline-flex items-center justify-center rounded-md text-sm font-medium';\n return cn(base, VARIANT_CLASSES[variant], SIZE_CLASSES[size], className ?? '');\n}\n","web/ui/cn.ts":"// Combine class lists into a single deduped string.\nexport function cn(...lists: string[]): string {\n const resolved = new Map();\n for (const list of lists) {\n for (const cls of list.split(/\\s+/).filter(Boolean)) {\n const key = cls.replace(/-[^-]+$/, '');\n resolved.set(key, cls);\n }\n }\n return [...resolved.values()].join(' ');\n}\n"}},"note":"buttonClassName merges class lists with cn, which keeps only the LAST class in any dash-prefixed group; SIZE_CLASSES sets rounded-md for every size and is applied after VARIANT_CLASSES, so the default variant's newly declared rounded-full pill shape is always overwritten back to rounded-md. Every default-size default-variant button silently renders square instead of the intended pill.","difficulty":"borderline","provenance":"mined","source":{"repo":"benord-labs/frink","pr":48,"url":"https://github.com/benord-labs/frink/pull/48#discussion_r3505778670"},"caseId":"corr-pr48-cn-ts-src","sourcePr":48,"variantOf":null,"holdout":true,"outcomeEvidence":"addressed-marker","scopeConfirmed":"unverifiable"} +{"id":"corr-pr59-guard-config-json-pair","reviewer":"correctness-reviewer","expected":"PASS","repo":{"base":{"src/jobs/execute.ts":"export interface Job {\n id: string;\n type: string;\n payload: Record;\n}\n\n// Executes a job. Payload validation currently happens upstream, at intake.\nexport async function executeJob(job: Job): Promise {\n await runJob(job);\n}\n\nasync function runJob(job: Job): Promise {\n console.log(`running ${job.id}`);\n}\n","src/jobs/rules-gate.ts":"export interface Rule {\n name: string;\n check: (job: unknown) => boolean;\n}\n\n// Rule definitions live in a separate package, shared with other internal tools.\nexport async function resolveValidationRules(): Promise {\n const mod = await import('@internal/job-rules');\n return mod.rules;\n}\n"},"staged":{"src/jobs/execute.ts":"import { resolveValidationRules } from './rules-gate';\n\nexport interface Job {\n id: string;\n type: string;\n payload: Record;\n}\n\n// Executes a job, routing it through the shared validation gate first.\nexport async function executeJob(job: Job): Promise {\n const rules = await resolveValidationRules();\n for (const rule of rules) {\n if (!rule.check(job)) throw new Error(`rule ${rule.name} failed`);\n }\n await runJob(job);\n}\n\nasync function runJob(job: Job): Promise {\n console.log(`running ${job.id}`);\n}\n","src/jobs/rules-gate.ts":"export interface Rule {\n name: string;\n check: (job: unknown) => boolean;\n}\n\n// Rule definitions ship bundled with this app, so resolution never depends on the runtime\n// environment the app happens to be installed into.\nexport function resolveValidationRules(): Rule[] {\n return BUNDLED_RULES;\n}\n\nconst BUNDLED_RULES: Rule[] = [];\n"}},"note":"Correct: resolveValidationRules no longer depends on an environment-sensitive dynamic import that can fail differently depending on how the package is installed. Rules are read from a value bundled directly with the app, so there is no failure path to collapse with \"no rules configured\" — the two cases can no longer be confused, and executeJob's gate behaves the same regardless of deployment shape.","difficulty":"clear","provenance":"adapted","source":{"repo":"benord-labs/frink","pr":59,"url":"https://github.com/benord-labs/frink/pull/59#discussion_r3523720574"},"caseId":"corr-pr59-guard-config-json","sourcePr":59,"variantOf":"corr-pr59-guard-config-json","holdout":false} +{"id":"corr-pr59-guard-config-json","reviewer":"correctness-reviewer","expected":"FAIL","expectItems":["error-and-edge-classification"],"reasonPattern":"swallow|catch|resolve|import|rules|no.?op|skip|indistinguish|empty|configured|silently","repo":{"base":{"src/jobs/execute.ts":"export interface Job {\n id: string;\n type: string;\n payload: Record;\n}\n\n// Executes a job. Payload validation currently happens upstream, at intake.\nexport async function executeJob(job: Job): Promise {\n await runJob(job);\n}\n\nasync function runJob(job: Job): Promise {\n console.log(`running ${job.id}`);\n}\n","src/jobs/rules-gate.ts":"export interface Rule {\n name: string;\n check: (job: unknown) => boolean;\n}\n\n// Rule definitions live in a separate package, shared with other internal tools.\nexport async function resolveValidationRules(): Promise {\n const mod = await import('@internal/job-rules');\n return mod.rules;\n}\n"},"staged":{"src/jobs/execute.ts":"import { resolveValidationRules } from './rules-gate';\n\nexport interface Job {\n id: string;\n type: string;\n payload: Record;\n}\n\n// Executes a job, routing it through the shared validation gate first.\nexport async function executeJob(job: Job): Promise {\n const rules = await resolveValidationRules();\n if (rules.length > 0) {\n for (const rule of rules) {\n if (!rule.check(job)) throw new Error(`rule ${rule.name} failed`);\n }\n }\n await runJob(job);\n}\n\nasync function runJob(job: Job): Promise {\n console.log(`running ${job.id}`);\n}\n","src/jobs/rules-gate.ts":"export interface Rule {\n name: string;\n check: (job: unknown) => boolean;\n}\n\n// Rule definitions live in a separate package, shared with other internal tools.\nexport async function resolveValidationRules(): Promise {\n try {\n const mod = await import('@internal/job-rules');\n return mod.rules;\n } catch {\n return [];\n }\n}\n"}},"note":"resolveValidationRules wraps the package import in a catch that returns an empty array on ANY failure, including the package being unresolvable at runtime (e.g. only installed globally, not as a local dependency of this project). executeJob then reads an empty array as \"no rules configured\" and skips validation entirely, so the newly wired gate silently never runs instead of surfacing the resolution failure. A resolution error and a legitimately empty rule set are different conditions and must not collapse to the same no-op path.","difficulty":"clear","provenance":"mined","source":{"repo":"benord-labs/frink","pr":59,"url":"https://github.com/benord-labs/frink/pull/59#discussion_r3523720574"},"caseId":"corr-pr59-guard-config-json","sourcePr":59,"holdout":false,"outcomeEvidence":"resolved+line-touched","scopeConfirmed":"unverifiable"} diff --git a/gate-engine/review/eval/reviewers/finalize.mts b/gate-engine/review/eval/reviewers/finalize.mts new file mode 100644 index 00000000..6c35d098 --- /dev/null +++ b/gate-engine/review/eval/reviewers/finalize.mts @@ -0,0 +1,419 @@ +#!/usr/bin/env node +// @ts-nocheck — BENCH-ONLY (excluded from tsc, see tsconfig.json exclude); loose types deliberate. + +/** + * finalize — the proposal checker + corpus appender that turns a hand-authored, ANONYMIZED + * proposal (raw/proposals/.json — a full cases-*.jsonl row a human built from a + * propose.mts queue entry) into a real corpus row, without ever touching the immutable raw file. + * + * bun finalize.mts --check + * 0 LLM calls. Runs the same corpus lint + fixture validation the bench itself trusts + * (lintRows + validateRow) against the single proposed row. Prints problems (hard fails, + * exit 1) and warnings (leakage tells, non-fatal). + * + * bun finalize.mts --append --suite [--max N] + * Reads every raw/proposals/*.json whose `reviewer` matches the suite, applies + * raw/audit-overlay.jsonl corrections, skips ids already in cases-.jsonl, assigns + * holdout deterministically, packs up to --max rows without splitting a caseId across + * batches, re-verifies the ≥3-holdout-per-class invariant, and appends — one JSON line per + * row, in the existing corpus's key order — to cases-.jsonl. raw/proposals/*.json and + * raw/audit-overlay.jsonl are NEVER written by this script. + * + * Both modes: raw/ is gitignored — the enriched material (`gh api` file contents, full mined-bot + * comment bodies) never reaches the public repo until a human distills it into an anonymized, + * generic-identifier fixture inside a proposal's `repo.base`/`repo.staged`. + */ + +import { + appendFileSync, + existsSync, + readdirSync, + readFileSync, + statSync, + unlinkSync, + writeFileSync, +} from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { BenchAbort, parseCasesText } from '../../../decisions/eval/bench.mts'; +import { BENCH_REVIEWERS, validateRow } from './bench.mts'; +import { casesFile, lintRows, loadRows } from './corpus.mts'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const RAW_DIR = path.join(here, 'raw'); +const PROPOSALS_DIR = path.join(RAW_DIR, 'proposals'); +const OVERLAY_FILE = path.join(RAW_DIR, 'audit-overlay.jsonl'); + +// Forbidden identifiers a real (private-repo) mined snippet could still be carrying — a proposal's +// fixture content must be a rewritten, generic-identifier minimal repro, never verbatim source. +// Checked here (not in bench.mts's shared validateRow) because it is specific to the +// candidates.jsonl→proposal provenance path, not every corpus row. Deliberately a bare substring +// (no \b anchors): word boundaries never fire inside identifiers, so `frinkClient`/`FRINK_API_KEY` +// would slip an anchored pattern — a leak scan should over-match, never under-match. +const LEAK_RE = /frink/i; + +// ─── --check ──────────────────────────────────────────────────────────────────────── + +function readProposal(file) { + let text: string; + try { + text = readFileSync(file, 'utf8'); + } catch (e) { + throw new BenchAbort(2, `finalize: cannot read ${file} — ${e?.message ?? e}`); + } + try { + return JSON.parse(text); + } catch (e) { + throw new BenchAbort(2, `finalize: ${file} is not valid JSON — ${e?.message ?? e}`); + } +} + +function leakScan(row) { + const hits = []; + for (const [p, content] of Object.entries({ ...row.repo?.base, ...row.repo?.staged })) { + if (content && LEAK_RE.test(content)) hits.push(p); + } + return hits; +} + +function checkProposal(file) { + const row = readProposal(file); + const problems = []; + const warnings = []; + + if (!row.reviewer || typeof row.reviewer !== 'string') { + problems.push('missing/invalid "reviewer" field — cannot determine which corpus this targets'); + } else { + try { + lintRows([row], row.reviewer); + } catch (e) { + problems.push(e instanceof BenchAbort ? e.message : String(e?.message ?? e)); + } + // validateRow materializes the fixture and reads row.repo.* unguarded — only meaningful (and + // only safe) once the structural lint above passed; a lint-failed row reports cleanly instead + // of crashing on a missing repo.base/staged. + if (problems.length === 0) { + const { problems: vProblems, warnings: vWarnings } = validateRow(row); + problems.push(...vProblems); + warnings.push(...vWarnings); + } + } + + const leaks = leakScan(row); + if (leaks.length) + problems.push( + `fixture content still references a private-repo identifier in: ${leaks.join(', ')}`, + ); + + console.log(`finalize --check ${path.basename(file)} (id=${row.id ?? ''})`); + if (problems.length === 0) console.log(' OK — no problems'); + else for (const p of problems) console.log(` PROBLEM ${p}`); + for (const w of warnings) console.log(` ${w}`); + + if (problems.length > 0) throw new BenchAbort(1, `finalize: ${problems.length} problem(s)`); +} + +// ─── --append ─────────────────────────────────────────────────────────────────────── + +function listProposalFiles() { + if (!existsSync(PROPOSALS_DIR)) return []; + return readdirSync(PROPOSALS_DIR) + .filter((f) => f.endsWith('.json')) + .sort() + .map((f) => path.join(PROPOSALS_DIR, f)); +} + +function loadOverlay() { + if (!existsSync(OVERLAY_FILE)) return []; + return parseCasesText(readFileSync(OVERLAY_FILE, 'utf8')); +} + +/** Canonical key order for an appended row — matches the shape hand-written rows already use, so + * a human diffing cases-*.jsonl sees a consistent layout regardless of a proposal's key order. */ +const KEY_ORDER = [ + 'id', + 'reviewer', + 'expected', + 'expectItems', + 'reasonPattern', + 'repo', + 'note', + 'difficulty', + 'provenance', + 'source', + 'caseId', + 'sourcePr', + 'variantOf', + 'holdout', +]; + +function reorderRow(row) { + const out = {}; + for (const k of KEY_ORDER) if (row[k] !== undefined) out[k] = row[k]; + for (const k of Object.keys(row)) if (!(k in out)) out[k] = row[k]; + return out; +} + +/** expected PASS rows DEFAULT to dev (holdout: false — decoys are starved there); expected FAIL + * rows alternate true/false in sorted-id order, so re-running the same batch is stable. The + * ≥3-holdout-per-class floor outranks the dev-bias: enforceHoldoutFloor may flip the minimal + * number of PASS rows back to holdout when the class would otherwise fall short (only PASS rows + * can satisfy the PASS-class floor), and reports every flip. */ +function assignHoldout(rows) { + const byExpected = { FAIL: [], PASS: [] }; + for (const r of rows) (byExpected[r.expected] ?? byExpected.FAIL).push(r); + for (const bucket of Object.values(byExpected)) + bucket.sort((a, b) => (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)); + for (const r of byExpected.PASS) r.holdout = false; + byExpected.FAIL.forEach((r, i) => { + r.holdout = i % 2 === 0; + }); +} + +/** Groups rows by caseId (own id when absent — an unclustered row is its own one-row group), + * greedily packs whole groups (sorted by each group's minimum id) into a --max row budget without + * ever splitting a caseId across the accepted/deferred boundary. */ +function packByMax(rows, max) { + if (max === null) return { accepted: rows, deferred: [] }; + const groups = new Map(); + for (const r of rows) { + const key = r.caseId ?? r.id; + if (!groups.has(key)) groups.set(key, []); + groups.get(key).push(r); + } + const ordered = [...groups.values()].sort((a, b) => { + const ai = a.reduce((m, r) => (r.id < m ? r.id : m), a[0].id); + const bi = b.reduce((m, r) => (r.id < m ? r.id : m), b[0].id); + return ai < bi ? -1 : ai > bi ? 1 : 0; + }); + const accepted = []; + const deferred = []; + let budget = max; + for (const group of ordered) { + if (group.length <= budget) { + accepted.push(...group); + budget -= group.length; + } else { + deferred.push(...group); + } + } + return { accepted, deferred }; +} + +/** Re-verifies the ≥3-holdout-per-expected-class invariant across (existing + accepted) rows; + * flips the minimal number of ACCEPTED rows (deterministic, sorted by id) to holdout:true when a + * class would otherwise fall short, and reports every flip. */ +function enforceHoldoutFloor(existingRows, acceptedRows) { + const flipped = []; + for (const expected of ['FAIL', 'PASS']) { + const have = [...existingRows, ...acceptedRows].filter( + (r) => r.expected === expected && r.holdout, + ).length; + if (have >= 3) continue; + const need = 3 - have; + const candidates = acceptedRows + .filter((r) => r.expected === expected && !r.holdout) + .sort((a, b) => (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)); + for (const r of candidates.slice(0, need)) { + r.holdout = true; + flipped.push(r.id); + } + } + return flipped; +} + +/** Exclusive append lock beside the cases file — appendSuite reads the corpus, then appends; two + * concurrent --append invocations would both read pre-append state and write duplicate ids, + * corrupting the corpus for every later loadRows caller. `wx` creation is the atomicity; a lock + * older than 10 min is treated as stale (a crashed run) and stolen with a warning. */ +const LOCK_STALE_MS = 10 * 60 * 1000; +function acquireAppendLock(reviewer) { + const lockPath = `${casesFile(reviewer)}.lock`; + for (let attempt = 0; attempt < 2; attempt++) { + try { + writeFileSync(lockPath, `${process.pid} ${new Date().toISOString()}\n`, { flag: 'wx' }); + return lockPath; + } catch (e) { + if ((e as { code?: string }).code !== 'EEXIST') throw e; + const age = Date.now() - statSync(lockPath).mtimeMs; + if (age > LOCK_STALE_MS) { + console.error(`finalize: stealing stale append lock (${Math.round(age / 1000)}s old)`); + unlinkSync(lockPath); + continue; + } + throw new BenchAbort( + 2, + `finalize: another --append holds ${path.basename(lockPath)} (age ${Math.round(age / 1000)}s) — retry when it finishes`, + ); + } + } + throw new BenchAbort(2, 'finalize: could not acquire append lock'); +} + +function appendSuite(suite, max) { + const reviewer = BENCH_REVIEWERS.find((r) => r.skill === suite); + if (!reviewer) { + throw new BenchAbort( + 2, + `finalize: unknown --suite ${suite} (want one of ${BENCH_REVIEWERS.map((r) => r.skill).join(', ')})`, + ); + } + + // Held for the remainder of the process (this is a one-shot CLI); released on every exit path + // via the 'exit' hook, and by the stale-lock rule if the process is SIGKILLed. + const lockPath = acquireAppendLock(reviewer); + process.on('exit', () => { + try { + unlinkSync(lockPath); + } catch { + /* already gone — fine */ + } + }); + + const proposalFiles = listProposalFiles(); + const allProposals = proposalFiles.map((f) => ({ file: f, row: readProposal(f) })); + + const overlay = loadOverlay(); + const matchedRefs = new Set(); + for (const { ref, set } of overlay) { + const hit = allProposals.find((p) => p.row.id === ref); + if (hit) { + Object.assign(hit.row, set); + matchedRefs.add(ref); + } + } + for (const { ref } of overlay) + if (!matchedRefs.has(ref)) + console.log(`finalize: audit-overlay ref "${ref}" matched no proposal — ignored`); + + const suiteRows = allProposals.filter((p) => p.row.reviewer === reviewer.name).map((p) => p.row); + if (suiteRows.length === 0) { + console.log(`finalize: no raw/proposals/*.json targets ${reviewer.name} — nothing to append`); + return; + } + + const existingRows = loadRows(reviewer); + const existingIds = new Set(existingRows.map((r) => r.id)); + const fresh = []; + for (const row of suiteRows) { + if (existingIds.has(row.id)) { + console.log(`finalize: ${row.id} already in ${path.basename(casesFile(reviewer))} — skipped`); + continue; + } + fresh.push(row); + } + if (fresh.length === 0) { + console.log('finalize: every proposal for this suite is already in the corpus — nothing to do'); + return; + } + + // Same structural lint --check runs on a single proposal before it's ever hand-approved — run it + // again here, on the post-overlay rows, right before anything is written. A row can reach this + // point without ever having gone through --check (nothing enforces that ordering), and the + // audit-overlay's Object.assign above can itself reintroduce a broken field on a row that DID + // pass --check earlier. lintRows throws BenchAbort on the first bad row, aborting the whole + // batch before a single line is appended — a refused append is safer than a corpus row that + // crashes every downstream loadRows() (bench/gate) for this reviewer. + lintRows(fresh, reviewer.name); + // Same reasoning for the leak scan: --append is the last line of defense before private-repo + // identifiers reach the public corpus, and nothing guarantees --check ran (or ran after the + // overlay was applied). + for (const row of fresh) { + const leaks = leakScan(row); + if (leaks.length) + throw new BenchAbort( + 2, + `finalize: ${row.id} fixture content references a private-repo identifier in: ${leaks.join(', ')} — fix the proposal (or overlay) before appending`, + ); + } + + assignHoldout(fresh); + + const { accepted, deferred } = packByMax(fresh, max); + if (accepted.length === 0) { + console.log( + `finalize: --max ${max} is smaller than every remaining caseId group — nothing appended this batch`, + ); + return; + } + + const flipped = enforceHoldoutFloor(existingRows, accepted); + if (flipped.length) + console.log( + `finalize: flipped ${flipped.length} row(s) to holdout:true to hold the ≥3-per-class floor — ${flipped.join(', ')}`, + ); + + const file = casesFile(reviewer); + for (const row of accepted) appendFileSync(file, `${JSON.stringify(reorderRow(row))}\n`); + + const allNow = [...existingRows, ...accepted]; + const countFor = (expected, holdout) => + allNow.filter((r) => r.expected === expected && !!r.holdout === holdout).length; + + console.log( + [ + `finalize: appended ${accepted.length} row(s) to ${path.basename(file)}: ${accepted.map((r) => r.id).join(', ')}`, + deferred.length + ? ` deferred (exceeded --max, caseId kept whole): ${deferred.map((r) => r.id).join(', ')}` + : null, + ` corpus now: ${countFor('FAIL', false) + countFor('FAIL', true)} gold ` + + `(${countFor('FAIL', true)} holdout) / ${countFor('PASS', false) + countFor('PASS', true)} decoy ` + + `(${countFor('PASS', true)} holdout)`, + ] + .filter(Boolean) + .join('\n'), + ); +} + +// ─── CLI ────────────────────────────────────────────────────────────────────────── + +function printHelp() { + console.log( + [ + 'usage: finalize.mts --check ', + ' finalize.mts --append --suite [--max N]', + ].join('\n'), + ); +} + +function main() { + const argv = process.argv.slice(2); + if (argv.includes('--help') || argv.includes('-h') || argv.length === 0) { + printHelp(); + return; + } + if (argv.includes('--check')) { + const idx = argv.indexOf('--check'); + const file = argv[idx + 1]; + if (!file) throw new BenchAbort(2, 'finalize: --check needs a path'); + checkProposal(path.resolve(file)); + return; + } + if (argv.includes('--append')) { + const suiteIdx = argv.indexOf('--suite'); + const suite = suiteIdx !== -1 ? argv[suiteIdx + 1] : null; + if (!suite) throw new BenchAbort(2, 'finalize: --append needs --suite '); + const maxIdx = argv.indexOf('--max'); + const max = maxIdx !== -1 ? Number.parseInt(argv[maxIdx + 1], 10) : null; + if (max !== null && (!Number.isFinite(max) || max <= 0)) + throw new BenchAbort(2, 'finalize: --max must be a positive integer'); + appendSuite(suite, max); + return; + } + printHelp(); + process.exit(2); +} + +const invokedDirectly = + process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url); +if (invokedDirectly) { + try { + main(); + } catch (e) { + if (e instanceof BenchAbort) { + console.error(e.message); + process.exit(e.code); + } + throw e; + } +} diff --git a/gate-engine/review/eval/reviewers/propose.mts b/gate-engine/review/eval/reviewers/propose.mts new file mode 100644 index 00000000..21c982a4 --- /dev/null +++ b/gate-engine/review/eval/reviewers/propose.mts @@ -0,0 +1,278 @@ +#!/usr/bin/env node +// @ts-nocheck — BENCH-ONLY (excluded from tsc, see tsconfig.json exclude); loose types deliberate. + +/** + * propose — deterministic triage of candidates.jsonl (mine-bots' output) into a per-suite review + * queue. No LLM calls: hard drops, a fixed crCategory→suite router, a priority sort, then a + * network-only enrichment step that fetches each surviving comment's BASE file content so a human + * (or a later agent) can turn it into an ANONYMIZED corpus fixture — nothing here writes to + * cases-*.jsonl directly, and nothing here is exempt from the "never copy private-repo source + * verbatim" rule: raw/ is gitignored precisely so this real, un-anonymized material never reaches + * the public repo. + * + * bun propose.mts --suite [--max N] + * + * Output: raw/queue-.jsonl (gitignored), one JSON line per surviving candidate — + * { queueId, suite, candidate: , baseFileContent } + * + * Pipeline: HARD DROPS (counted) → ROUTE (crCategory → suite, path splits security/performance + * frontend vs backend) → SORT (outcome, severity, scope, recency) → ENRICH (gh api contents, up to + * --max successes — a fetch failure drops that entry and the next-ranked one is tried instead) → + * WRITE. A histogram of every drop reason prints to stderr so triage counts are auditable. + */ + +import { execFileSync } from 'node:child_process'; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const CANDIDATES_FILE = path.join(here, 'candidates.jsonl'); +const RAW_DIR = path.join(here, 'raw'); + +const MAX_HUNK_LEN = 4000; + +// ─── Suite routing ────────────────────────────────────────────────────────────────── + +const CORRECTNESS_CATEGORIES = new Set([ + 'Functional Correctness', + 'Data Integrity & Integration', + 'Stability & Availability', +]); + +const FRONTEND_PATH_RE = /^src\/(renderer|preload)\//; + +/** Returns the suite a candidate belongs to, or null for out-of-charter (Maintainability, + * null, 'Potential issue', or any crCategory not in the router). */ +function routeSuite(candidate) { + const cat = candidate.crCategory; + if (CORRECTNESS_CATEGORIES.has(cat)) return 'correctness'; + if (cat === 'Security & Privacy') + return FRONTEND_PATH_RE.test(candidate.path ?? '') ? 'frontend-security' : 'api-security'; + if (cat === 'Performance & Scalability') + return FRONTEND_PATH_RE.test(candidate.path ?? '') + ? 'frontend-performance' + : 'backend-performance'; + return null; +} + +const SUITE_PREFIX = { + correctness: 'corr', + 'api-security': 'api-sec', + 'frontend-security': 'fe-sec', + 'backend-performance': 'be-perf', + 'frontend-performance': 'fe-perf', +}; +const VALID_SUITES = Object.keys(SUITE_PREFIX); + +// ─── Hard drops ───────────────────────────────────────────────────────────────────── + +/** Returns a drop reason string, or null if the candidate survives. */ +function hardDropReason(c) { + if (c.alreadyInCorpus) return 'already-in-corpus'; + if (c.outcome !== 'fixed' && c.outcome !== 'rebutted') return `bad-outcome:${c.outcome}`; + if (!c.originalCommitId) return 'missing-original-commit'; + if (c.line === null && c.originalLine === null) return 'no-line'; + if (c.outcomeEvidence === 'outdated-only') return 'outdated-only'; + if (!c.diffHunk || c.diffHunk.length === 0) return 'empty-hunk'; + if ((c.hunkLen ?? c.diffHunk.length) > MAX_HUNK_LEN) return 'hunk-too-long'; + return null; +} + +// ─── Sort ─────────────────────────────────────────────────────────────────────────── + +const OUTCOME_RANK = { fixed: 0, rebutted: 1 }; +const SEVERITY_RANK = { Critical: 0, Major: 1, Minor: 2 }; +const severityRank = (s) => SEVERITY_RANK[s] ?? 3; +const scopeRank = (s) => (s === 'confirmed' ? 0 : 1); + +function compareCandidates(a, b) { + const byOutcome = (OUTCOME_RANK[a.outcome] ?? 2) - (OUTCOME_RANK[b.outcome] ?? 2); + if (byOutcome !== 0) return byOutcome; + const bySeverity = severityRank(a.crSeverity) - severityRank(b.crSeverity); + if (bySeverity !== 0) return bySeverity; + const byScope = scopeRank(a.scopeConfirmed) - scopeRank(b.scopeConfirmed); + if (byScope !== 0) return byScope; + // Newest createdAt sorts last: prefer older (more time to accrue outcome evidence) comments. + return Date.parse(a.createdAt ?? 0) - Date.parse(b.createdAt ?? 0); +} + +// ─── Enrichment (gh api) ──────────────────────────────────────────────────────────── + +function gh(args) { + return execFileSync('gh', args, { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }); +} + +function preflightGh() { + try { + execFileSync('gh', ['--version'], { encoding: 'utf8', timeout: 15000 }); + } catch { + console.error('propose: `gh` CLI not available — cannot enrich candidates'); + process.exit(2); + } +} + +/** Fetches a file's content at a specific commit via the GitHub contents API. Throws on any + * failure (missing file, bad ref, oversized blob, non-file content type) — caller drops the row. */ +function fetchBaseFileContent(repo, filePath, ref) { + const encodedPath = filePath + .split('/') + .map((seg) => encodeURIComponent(seg)) + .join('/'); + const b64 = gh([ + 'api', + `repos/${repo}/contents/${encodedPath}?ref=${ref}`, + '--jq', + '.content', + ]).trim(); + if (!b64) throw new Error('empty .content (not a regular file, or missing at this ref)'); + return Buffer.from(b64, 'base64').toString('utf8'); +} + +// ─── queueId slug ─────────────────────────────────────────────────────────────────── + +function slugify(text, maxWords = 5) { + return ( + String(text ?? '') + .toLowerCase() + .replace(/`/g, '') + .match(/[a-z0-9]+/g) + ?.slice(0, maxWords) + .join('-') + ?.slice(0, 40) || 'finding' + ); +} + +/** A short kebab defect hint from the bot comment's bolded headline (CodeRabbit always leads with + * one), falling back to the crCategory when no bold text is present. */ +const BOLD_HEADLINE_RE = /\*\*([^*]{3,90})\*\*/; +function defectHint(candidate) { + const m = BOLD_HEADLINE_RE.exec(candidate.body ?? ''); + return slugify(m ? m[1] : candidate.crCategory); +} + +function makeQueueId(suite, candidate, seen) { + const base = `${SUITE_PREFIX[suite]}-pr${candidate.pr}-${defectHint(candidate)}`; + let id = base; + let n = 2; + while (seen.has(id)) { + id = `${base}-${n}`; + n += 1; + } + seen.add(id); + return id; +} + +// ─── Main ─────────────────────────────────────────────────────────────────────────── + +function parseArgs(argv) { + const suiteIdx = argv.indexOf('--suite'); + const suite = suiteIdx !== -1 ? argv[suiteIdx + 1] : null; + const maxIdx = argv.indexOf('--max'); + const max = maxIdx !== -1 ? Number.parseInt(argv[maxIdx + 1], 10) : 20; + return { suite, max }; +} + +function printHelp() { + console.log( + 'usage: propose.mts --suite [--max N]', + ); +} + +async function main() { + const argv = process.argv.slice(2); + if (argv.includes('--help') || argv.includes('-h')) { + printHelp(); + return; + } + const { suite, max } = parseArgs(argv); + if (!suite || !VALID_SUITES.includes(suite)) { + console.error(`propose: --suite must be one of ${VALID_SUITES.join(', ')}`); + process.exit(2); + } + if (!Number.isFinite(max) || max <= 0) { + console.error('propose: --max must be a positive integer'); + process.exit(2); + } + if (!existsSync(CANDIDATES_FILE)) { + console.error(`propose: missing ${path.basename(CANDIDATES_FILE)} — run mine-bots.mts first`); + process.exit(2); + } + preflightGh(); + + const candidates = readFileSync(CANDIDATES_FILE, 'utf8') + .split('\n') + .filter((l) => l.trim()) + .map((l) => JSON.parse(l)); + + const drops = {}; + const bump = (reason, n = 1) => { + drops[reason] = (drops[reason] ?? 0) + n; + }; + + const routed = []; + for (const c of candidates) { + const dropReason = hardDropReason(c); + if (dropReason) { + bump(dropReason); + continue; + } + const target = routeSuite(c); + if (!target) { + bump('out-of-charter'); + continue; + } + if (target !== suite) { + bump(`routed-elsewhere:${target}`); + continue; + } + routed.push(c); + } + + routed.sort(compareCandidates); + + const seenIds = new Set(); + const enriched = []; + let enrichFailures = 0; + for (const c of routed) { + if (enriched.length >= max) break; + let baseFileContent: string; + try { + baseFileContent = fetchBaseFileContent(c.repo, c.path, c.originalCommitId); + } catch (e) { + enrichFailures += 1; + console.error( + `propose: enrich failed for ${c.repo}#${c.pr} ${c.path}@${c.originalCommitId.slice(0, 8)} — ${e.message?.split('\n')[0] ?? e}`, + ); + continue; + } + enriched.push({ + queueId: makeQueueId(suite, c, seenIds), + suite, + candidate: c, + baseFileContent, + }); + } + if (enrichFailures) bump('enrich-failed', enrichFailures); + + mkdirSync(RAW_DIR, { recursive: true }); + const outFile = path.join(RAW_DIR, `queue-${suite}.jsonl`); + writeFileSync(outFile, `${enriched.map((r) => JSON.stringify(r)).join('\n')}\n`); + + console.error( + [ + `propose: ${candidates.length} candidates → ${routed.length} routed to ${suite} (of which ${enriched.length} enriched, ${enrichFailures} enrich failures)`, + ` drops: ${ + Object.entries(drops) + .sort((a, b) => b[1] - a[1]) + .map(([k, v]) => `${k}:${v}`) + .join(', ') || '—' + }`, + ` → ${path.relative(here, outFile)}`, + ].join('\n'), + ); +} + +const invokedDirectly = + process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url); +if (invokedDirectly) await main();