From f343ab3e99506bcac7c524d9413b7764ca00b74a Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Sat, 13 Jun 2026 01:56:29 +0000 Subject: [PATCH 1/2] Port the PR-review benchmark harness as a mock-mode regression suite All 30 fixtures from the previous product line's harness, adapted to the rebuilt CLI contract: the release binary runs postil review --repo --pr --output-json against a per-case mock GitHub API and mock OpenAI-compatible endpoint, and the v1 envelope plus forge interactions are scored against ground truth. Isolation (own HOME/TMPDIR, no inherited environment) and prompt-leakage guardrails are kept. Mock mode measures pipeline fidelity, not detection ability; live-model mode is a later slice. --- ROADMAP.md | 4 + bench/.gitignore | 2 + bench/README.md | 44 ++ bench/bun.lock | 26 ++ bench/fixtures/cases.ts | 632 ++++++++++++++++++++++++++++ bench/package.json | 15 + bench/src/harness.ts | 897 ++++++++++++++++++++++++++++++++++++++++ bench/src/run.ts | 40 ++ bench/tsconfig.json | 12 + 9 files changed, 1672 insertions(+) create mode 100644 bench/.gitignore create mode 100644 bench/README.md create mode 100644 bench/bun.lock create mode 100644 bench/fixtures/cases.ts create mode 100644 bench/package.json create mode 100644 bench/src/harness.ts create mode 100644 bench/src/run.ts create mode 100644 bench/tsconfig.json diff --git a/ROADMAP.md b/ROADMAP.md index 5193e44..0b52962 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -61,3 +61,7 @@ Port plan: bring the harness into this repo against the envelope v1 contract, keep mock mode as a regression suite, add a live-model mode (real inference, same fixtures) to measure detection and silence rate, then run peers on the identical fixture set before publishing any comparison. + +Status: mock mode is ported and lives in `bench/` (all 30 fixtures, isolation +and prompt-leakage guardrails kept), runs as the `bench` job in CI against a +release build. Live-model mode and peer runs remain open. diff --git a/bench/.gitignore b/bench/.gitignore new file mode 100644 index 0000000..f5855fb --- /dev/null +++ b/bench/.gitignore @@ -0,0 +1,2 @@ +node_modules/ +.runs/ diff --git a/bench/README.md b/bench/README.md new file mode 100644 index 0000000..869d87c --- /dev/null +++ b/bench/README.md @@ -0,0 +1,44 @@ +# postil bench + +Hermetic PR-review regression suite for the `postil` CLI, ported from the +previous product line's benchmark harness. Each of the 30 fixtures (seeded +defects across languages and change classes, plus clean PRs where the correct +review is silence) runs the release binary against a per-case mock GitHub API +and a mock OpenAI-compatible model endpoint in an isolated run directory, then +scores the v1 envelope and the forge interactions against ground truth. + +## What it measures + +Pipeline fidelity, not detection ability: grounding (no ungrounded findings, +`counts.ungrounded == 0`), gating (gate fails exactly on error-severity +findings, exit codes match), statusline correctness (`postil/review` and +`postil/gate` check-runs created and completed with the right conclusions), +silence on clean PRs (no review comment posted), and prompt-leakage guardrails +(fixture metadata such as policy phrasing must never appear in the prompt or +any pipeline output). + +From the roadmap, verbatim — the caveats that gate any public use of these +numbers: + +> - The mock model echoes output generated from the same spec as the ground +> truth, so the run measures pipeline fidelity (grounding, gating, statusline +> correctness) — not detection ability. +> - No competitor has been run on the same fixtures, so comparative claims are +> not defensible; site comparisons stay qualitative and sourced until peer +> runs on identical fixtures exist. + +A live-model mode (real inference, same fixtures) is a later slice; see +ROADMAP.md. + +## Running + +```sh +cargo build --quiet --release # from the repo root +cd bench +bun install +bun run bench # add --json or --json-out report.json for machine output +``` + +`POSTIL_BIN` points the suite at a different binary; +`POSTIL_BENCH_KEEP_RUNS=1` keeps per-case run directories under `.runs/` +(failing cases are always kept). diff --git a/bench/bun.lock b/bench/bun.lock new file mode 100644 index 0000000..fdc0eca --- /dev/null +++ b/bench/bun.lock @@ -0,0 +1,26 @@ +{ + "lockfileVersion": 1, + "configVersion": 1, + "workspaces": { + "": { + "name": "postil-bench", + "dependencies": { + "zod": "^4.4.3", + }, + "devDependencies": { + "@types/bun": "^1.3.14", + }, + }, + }, + "packages": { + "@types/bun": ["@types/bun@1.3.14", "", { "dependencies": { "bun-types": "1.3.14" } }, "sha512-h1hFqFVcvAvD9j9K7ZW7vd82aSA+rTdznZa+5bwvCwqSB1jmmfLcbIWhOLx1/+boy/xmjgCs/OMUL8hRJSmnPw=="], + + "@types/node": ["@types/node@25.9.3", "", { "dependencies": { "undici-types": ">=7.24.0 <7.24.7" } }, "sha512-603BddQMv3pUcr4U2dhujk83N2tTDVr/34wII2B6bJy6g+8WD6yUb11jszNs0gdi4PesVWl7ABt8nYMVpnLUcg=="], + + "bun-types": ["bun-types@1.3.14", "", { "dependencies": { "@types/node": "*" } }, "sha512-4N0ig0fEomHt5R0KCFWjovxow98rIoRwKolrYdCcknNwMekCXRnWEUvgu5soYV8QXtVsrUD8B95MBOZGPvr6KQ=="], + + "undici-types": ["undici-types@7.24.6", "", {}, "sha512-WRNW+sJgj5OBN4/0JpHFqtqzhpbnV0GuB+OozA9gCL7a993SmU+1JBZCzLNxYsbMfIeDL+lTsphD5jN5N+n0zg=="], + + "zod": ["zod@4.4.3", "", {}, "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ=="], + } +} diff --git a/bench/fixtures/cases.ts b/bench/fixtures/cases.ts new file mode 100644 index 0000000..24d796a --- /dev/null +++ b/bench/fixtures/cases.ts @@ -0,0 +1,632 @@ +// The 30-case fixture set carried over from the previous product line's +// benchmark harness: 24 seeded defects across languages and change classes, +// 6 clean PRs where the correct review is silence. +// +// Each spec compiles into a hermetic case: a single-hunk diff, the changed +// file as allowed context, a recorded model response generated from the same +// spec as the ground truth (mock mode measures pipeline fidelity, not +// detection), and `disallowedSources` — distinctive policy phrasing that must +// never leak into the prompt or any pipeline output. + +import type { BenchmarkCaseInput } from "../src/harness"; + +type FixtureSpec = { + id: string; + name: string; + pullNumber: number; + path: string; + line: number; + before: string; + after: string; + allowedFileContent: string; + policy: string; + scoringLabels: string[]; + finding?: { + severity: "info" | "warn" | "error"; + body: string; + bodyIncludes: string; + }; + disallowedSources?: string[]; + maxFindings?: number; +}; + +const repoFullName = "benchmark/example-fixtures"; + +function makeHeadSha(seed: number): string { + return seed.toString(16).padStart(2, "0").repeat(20).slice(0, 40); +} + +function makeDiff(path: string, line: number, before: string, after: string): string { + return [ + `diff --git a/${path} b/${path}`, + "index 1111111..2222222 100644", + `--- a/${path}`, + `+++ b/${path}`, + `@@ -${line},1 +${line},1 @@`, + `- ${before}`, + `+ ${after}`, + "", + ].join("\n"); +} + +function buildCase(spec: FixtureSpec): BenchmarkCaseInput { + const expected = spec.finding + ? [ + { + path: spec.path, + line: spec.line, + severity: spec.finding.severity, + bodyIncludes: spec.finding.bodyIncludes, + }, + ] + : []; + + return { + id: spec.id, + name: spec.name, + repo: repoFullName, + pullNumber: spec.pullNumber, + headSha: makeHeadSha(spec.pullNumber), + diff: makeDiff(spec.path, spec.line, spec.before, spec.after), + allowedContext: { + files: [{ path: spec.path, content: spec.allowedFileContent }], + docs: [{ path: "review-policy.md", content: spec.policy }], + }, + disallowedSources: spec.disallowedSources ?? [], + scoringLabels: spec.scoringLabels, + groundTruth: { findings: expected }, + guardrails: { forbiddenPromptSubstrings: [] }, + modelOutput: { + // Contract: a clean review is an empty summary plus an empty findings + // array; anything else fails closed as narrated risk. + summary: spec.finding ? `Recorded model response for ${spec.id}.` : "", + findings: spec.finding + ? [ + { + path: spec.path, + line: spec.line, + severity: spec.finding.severity, + kind: "risk" as const, + confidence: 0.9, + title: spec.name, + body: spec.finding.body, + }, + ] + : [], + }, + expectations: { + minFindings: spec.finding ? 1 : 0, + maxFindings: spec.maxFindings ?? (spec.finding ? 1 : 0), + requiredFindings: expected, + }, + }; +} + +const fixtureSpecs: FixtureSpec[] = [ + { + id: "billing-double-charge", + name: "Double charge on retry", + pullNumber: 1, + path: "src/billing/charge.ts", + line: 18, + before: "return amount;", + after: "return amount + amount;", + allowedFileContent: "export function charge(amount: number) { return amount + amount; }", + policy: "Billing changes must preserve single-charge semantics.", + scoringLabels: ["billing", "retry", "error"], + finding: { + severity: "error", + body: "The amount is added twice, so a retry can bill the customer twice.", + bodyIncludes: "bill the customer twice", + }, + disallowedSources: ["single-charge semantics"], + }, + { + id: "billing-refund-replay", + name: "Refund replay without idempotency", + pullNumber: 2, + path: "src/billing/refunds.ts", + line: 22, + before: "await ledger.refund(invoiceId);", + after: "await ledger.refund(invoiceId); await ledger.refund(invoiceId);", + allowedFileContent: + "export async function refund(invoiceId: string) { await ledger.refund(invoiceId); await ledger.refund(invoiceId); }", + policy: "Refund paths should be idempotent across retries.", + scoringLabels: ["billing", "idempotency", "error"], + finding: { + severity: "error", + body: "The refund path can replay the same payout twice.", + bodyIncludes: "replay the same payout twice", + }, + disallowedSources: ["refund path should be idempotent"], + }, + { + id: "security-admin-delete", + name: "Delete user without an authorization check", + pullNumber: 3, + path: "src/users/delete-user.ts", + line: 27, + before: 'if (!user.isAdmin) throw new Error("Forbidden");', + after: "await db.user.delete({ where: { id: userId } });", + allowedFileContent: + "export async function deleteUser(userId: string) { await db.user.delete({ where: { id: userId } }); }", + policy: "Destructive actions require authorization checks before the write happens.", + scoringLabels: ["security", "authorization", "error"], + finding: { + severity: "error", + body: "The destructive path no longer checks authorization before deleting the user.", + bodyIncludes: "checks authorization before deleting the user", + }, + disallowedSources: ["authorization checks before the write"], + }, + { + id: "security-public-export", + name: "Public export endpoint skips permission gating", + pullNumber: 4, + path: "src/reports/export.ts", + line: 19, + before: 'if (!session.user.canExport) throw new Error("Forbidden");', + after: "return await buildExport(reportId);", + allowedFileContent: + "export async function exportReport(reportId: string) { return await buildExport(reportId); }", + policy: "Export endpoints should guard access before materializing sensitive data.", + scoringLabels: ["security", "authorization", "error"], + finding: { + severity: "error", + body: "The export path is reachable without the permission gate.", + bodyIncludes: "permission gate", + }, + disallowedSources: ["guard access before materializing sensitive data"], + }, + { + id: "race-double-enqueue", + name: "Duplicate queue enqueue on retry", + pullNumber: 5, + path: "src/queue/schedule.ts", + line: 31, + before: "await queue.enqueue(jobId);", + after: "await queue.enqueue(jobId); await queue.enqueue(jobId);", + allowedFileContent: + "export async function schedule(jobId: string) { await queue.enqueue(jobId); await queue.enqueue(jobId); }", + policy: "Queue scheduling should remain idempotent under retries.", + scoringLabels: ["race", "queue", "error"], + finding: { + severity: "error", + body: "The job can be queued twice when the request retries.", + bodyIncludes: "queued twice", + }, + disallowedSources: ["idempotent under retries"], + }, + { + id: "race-non-atomic-counter", + name: "Counter increment loses the lock", + pullNumber: 6, + path: "src/metrics/increment.ts", + line: 14, + before: "await lock.run(() => stats.increment(key));", + after: "stats.increment(key);", + allowedFileContent: "export function bump(key: string) { stats.increment(key); }", + policy: "Shared counters must stay protected by the lock to avoid lost updates.", + scoringLabels: ["race", "concurrency", "warn"], + finding: { + severity: "warn", + body: "Removing the lock makes the counter update vulnerable to lost writes.", + bodyIncludes: "lost writes", + }, + disallowedSources: ["protected by the lock"], + }, + { + id: "migration-drop-column", + name: "Drop a column without preserving the old values", + pullNumber: 7, + path: "db/migrations/0012_drop_legacy_email.sql", + line: 9, + before: "INSERT INTO archived_users SELECT id, email FROM users;", + after: "ALTER TABLE users DROP COLUMN email;", + allowedFileContent: + "INSERT INTO archived_users SELECT id, email FROM users;\nALTER TABLE users DROP COLUMN email;", + policy: "Schema migrations must preserve data before dropping a column.", + scoringLabels: ["migrations", "data-loss", "error"], + finding: { + severity: "error", + body: "The column is dropped before the old values are preserved anywhere else.", + bodyIncludes: "dropped before the old values are preserved", + }, + disallowedSources: ["preserve data before dropping a column"], + }, + { + id: "migration-nullability-tighten", + name: "Tighten nullability with no backfill", + pullNumber: 8, + path: "db/migrations/0013_require_invoice_status.sql", + line: 10, + before: "UPDATE invoices SET status = 'pending' WHERE status IS NULL;", + after: "ALTER TABLE invoices ALTER COLUMN status SET NOT NULL;", + allowedFileContent: + "UPDATE invoices SET status = 'pending' WHERE status IS NULL;\nALTER TABLE invoices ALTER COLUMN status SET NOT NULL;", + policy: "Nullability tightening needs a backfill step before the constraint changes.", + scoringLabels: ["migrations", "data-loss", "error"], + finding: { + severity: "error", + body: "The NOT NULL constraint is added without backfilling the missing rows first.", + bodyIncludes: "backfilling the missing rows", + }, + disallowedSources: ["backfill step before the constraint changes"], + }, + { + id: "cache-tenant-key-omission", + name: "Cache key drops tenant scope", + pullNumber: 9, + path: "src/cache/key.ts", + line: 12, + before: "return `${tenantId}:${path}`;", + after: "return path;", + allowedFileContent: "export function cacheKey(tenantId: string, path: string) { return path; }", + policy: "Cache keys must retain tenant scope or data can bleed across accounts.", + scoringLabels: ["cache", "multi-tenant", "warn"], + finding: { + severity: "warn", + body: "The cache key ignores tenant scope, so entries can bleed across accounts.", + bodyIncludes: "tenant scope", + }, + disallowedSources: ["retain tenant scope"], + }, + { + id: "cache-missing-invalidation", + name: "Update path stops clearing the cache", + pullNumber: 10, + path: "src/cache/invalidate.ts", + line: 16, + before: "cache.delete(key);", + after: "return result;", + allowedFileContent: "export function updateRecord() { return result; }", + policy: "Writes should invalidate any cached copy of the updated record.", + scoringLabels: ["cache", "invalidation", "warn"], + finding: { + severity: "warn", + body: "The update path no longer clears the cached record after the write.", + bodyIncludes: "clears the cached record", + }, + disallowedSources: ["invalidate any cached copy"], + }, + { + id: "dependency-major-bump", + name: "Dependency jumps to a new major", + pullNumber: 11, + path: "package.json", + line: 9, + before: '"ui-kit": "^1.4.0"', + after: '"ui-kit": "^2.0.0"', + allowedFileContent: '{ "dependencies": { "ui-kit": "^2.0.0" } }', + policy: "Major dependency upgrades need compatibility review before merge.", + scoringLabels: ["dependency", "upgrade", "warn"], + finding: { + severity: "warn", + body: "This dependency jumps to a new major without a compatibility check.", + bodyIncludes: "compatibility check", + }, + disallowedSources: ["compatibility review before merge"], + }, + { + id: "dependency-vulnerable-pin", + name: "Pinned dependency stays on the vulnerable line", + pullNumber: 12, + path: "package.json", + line: 10, + before: '"yaml-parser": "1.2.2"', + after: '"yaml-parser": "1.2.3"', + allowedFileContent: '{ "dependencies": { "yaml-parser": "1.2.3" } }', + policy: "Keep pinned dependencies off the affected release line when the policy says to move.", + scoringLabels: ["dependency", "security", "error"], + finding: { + severity: "error", + body: "The dependency remains in the affected release line and should move off it.", + bodyIncludes: "affected release line", + }, + disallowedSources: ["move off the affected release line"], + }, + { + id: "deletion-hard-delete", + name: "Soft delete turns into a hard delete", + pullNumber: 13, + path: "src/users/archive.ts", + line: 18, + before: "await db.user.update({ where: { id: userId }, data: { deletedAt: new Date() } });", + after: "await db.user.delete({ where: { id: userId } });", + allowedFileContent: + "export async function archiveUser(userId: string) { await db.user.delete({ where: { id: userId } }); }", + policy: + "User removal flows should preserve auditability unless the product explicitly says otherwise.", + scoringLabels: ["deletion", "audit", "error"], + finding: { + severity: "error", + body: "This now hard-deletes user records and removes the audit trail.", + bodyIncludes: "hard-deletes user records", + }, + disallowedSources: ["preserving the soft-delete trail"], + }, + { + id: "deletion-no-archive", + name: "Delete path skips the archive copy", + pullNumber: 14, + path: "src/files/remove.ts", + line: 24, + before: "await archiveFile(fileId);", + after: "await storage.delete(fileId);", + allowedFileContent: + "export async function removeFile(fileId: string) { await storage.delete(fileId); }", + policy: "Deletion flows should preserve the only recovery copy before the removal happens.", + scoringLabels: ["deletion", "recovery", "error"], + finding: { + severity: "error", + body: "The delete path skips the archive table and loses the only recovery copy.", + bodyIncludes: "archive table", + }, + disallowedSources: ["preserve the only recovery copy"], + }, + { + id: "ui-button-missing-label", + name: "Icon button loses its accessible name", + pullNumber: 15, + path: "src/app/settings/page.tsx", + line: 42, + before: '", + allowedFileContent: "export function Settings() { return ; }", + policy: "Icon-only buttons need an accessible name for assistive technologies.", + scoringLabels: ["ui", "a11y", "warn"], + finding: { + severity: "warn", + body: "The icon button has no accessible name for assistive tech.", + bodyIncludes: "accessible name", + }, + disallowedSources: ["accessible name for assistive technologies"], + }, + { + id: "ui-input-missing-label", + name: "Text input loses its visible label", + pullNumber: 16, + path: "src/app/profile/page.tsx", + line: 48, + before: '', + after: '', + allowedFileContent: 'export function Profile() { return ; }', + policy: "Form fields should keep a visible or programmatic label.", + scoringLabels: ["ui", "a11y", "warn"], + finding: { + severity: "warn", + body: "The input has no visible label or aria-label.", + bodyIncludes: "visible label", + }, + disallowedSources: ["visible or programmatic label"], + }, + { + id: "a11y-low-contrast-status", + name: "Status text drops below the contrast floor", + pullNumber: 17, + path: "src/app/dashboard/status.tsx", + line: 25, + before: 'className="text-zinc-700"', + after: 'className="text-zinc-400"', + allowedFileContent: + 'export function Status() { return Ready; }', + policy: "Text must stay readable against the page background.", + scoringLabels: ["a11y", "contrast", "warn"], + finding: { + severity: "warn", + body: "The status text falls below the contrast floor against the page background.", + bodyIncludes: "contrast floor", + }, + disallowedSources: ["stay readable against the page background"], + }, + { + id: "a11y-icon-only-action", + name: "Close control becomes icon-only", + pullNumber: 18, + path: "src/components/menu.tsx", + line: 20, + before: '", + allowedFileContent: "export function Menu() { return ; }", + policy: "Icon-only actions must still be announced meaningfully.", + scoringLabels: ["a11y", "ui", "warn"], + finding: { + severity: "warn", + body: "This control is icon-only, so assistive tech does not get a usable label.", + bodyIncludes: "icon-only", + }, + disallowedSources: ["announced meaningfully"], + }, + { + id: "api-contract-field-removed", + name: "Response drops a client-facing field", + pullNumber: 19, + path: "src/api/orders/route.ts", + line: 31, + before: "return Response.json({ id, currency, total });", + after: "return Response.json({ id, total });", + allowedFileContent: "export function GET() { return Response.json({ id, total }); }", + policy: "API responses should preserve fields that downstream clients already rely on.", + scoringLabels: ["api", "contract", "error"], + finding: { + severity: "error", + body: "The response no longer includes currency, which breaks downstream clients.", + bodyIncludes: "downstream clients", + }, + disallowedSources: ["preserve fields that downstream clients already rely on"], + }, + { + id: "api-contract-status-drift", + name: "Validation error becomes a success response", + pullNumber: 20, + path: "src/api/invoices/route.ts", + line: 38, + before: 'return Response.json({ error: "bad input" }, { status: 400 });', + after: 'return Response.json({ error: "bad input" }, { status: 200 });', + allowedFileContent: + 'export function POST() { return Response.json({ error: "bad input" }, { status: 200 }); }', + policy: "Status codes are part of the contract and should not drift silently.", + scoringLabels: ["api", "contract", "error"], + finding: { + severity: "error", + body: "The endpoint now returns success for validation failures, hiding the contract signal.", + bodyIncludes: "contract signal", + }, + disallowedSources: ["status codes are part of the contract"], + }, + { + id: "ci-secret-in-log", + name: "Workflow logs a secret value", + pullNumber: 21, + path: ".github/workflows/ci.yml", + line: 23, + before: 'echo "build ok"', + after: 'echo "$DEPLOY_TOKEN"', + allowedFileContent: 'name: CI\njobs:\n build:\n steps:\n - run: echo "$DEPLOY_TOKEN"', + policy: "CI logs must not print secret values or anything that can be replayed later.", + scoringLabels: ["ci", "secrets", "error"], + finding: { + severity: "error", + body: "The workflow logs a secret value during the build.", + bodyIncludes: "logs a secret value", + }, + disallowedSources: ["must not print secret values"], + }, + { + id: "ci-unpinned-action", + name: "Workflow action is no longer pinned", + pullNumber: 22, + path: ".github/workflows/release.yml", + line: 18, + before: "uses: actions/checkout@v4", + after: "uses: actions/checkout@main", + allowedFileContent: "name: Release\njobs:\n publish:\n steps:\n - uses: actions/checkout@main", + policy: "Workflow actions should stay pinned to a reviewed release reference.", + scoringLabels: ["ci", "config", "warn"], + finding: { + severity: "warn", + body: "The action is no longer pinned to a release reference.", + bodyIncludes: "no longer pinned", + }, + disallowedSources: ["pinned to a reviewed release reference"], + }, + { + id: "config-debug-enabled", + name: "Production config turns debugging on", + pullNumber: 23, + path: ".postil.yaml", + line: 7, + before: "debug: false", + after: "debug: true", + allowedFileContent: "enabled: true\ndebug: true", + policy: "Production config should keep debug output off unless explicitly approved.", + scoringLabels: ["config", "runtime", "warn"], + finding: { + severity: "warn", + body: "The production configuration enables debug output.", + bodyIncludes: "debug output", + }, + disallowedSources: ["keep debug output off"], + }, + { + id: "config-review-disabled", + name: "Review gate is disabled in the default config", + pullNumber: 24, + path: ".postil.yaml", + line: 8, + before: "review.enabled: true", + after: "review.enabled: false", + allowedFileContent: "review:\n enabled: false", + policy: "The default review gate should stay on unless a release policy says otherwise.", + scoringLabels: ["config", "review", "error"], + finding: { + severity: "error", + body: "The review gate is disabled in the default configuration.", + bodyIncludes: "review gate is disabled", + }, + disallowedSources: ["default review gate should stay on"], + }, + { + id: "clean-docs-only", + name: "Docs-only formatting update", + pullNumber: 25, + path: "docs/usage.md", + line: 12, + before: "Use the CLI to review pull requests.", + after: "Use the CLI to review pull requests and keep the docs in sync.", + allowedFileContent: "Use the CLI to review pull requests and keep the docs in sync.", + policy: "Docs-only changes should not trigger merge-blocking review comments.", + scoringLabels: ["clean", "docs", "silence"], + }, + { + id: "clean-refactor-no-behavior-change", + name: "Refactor without a behavior delta", + pullNumber: 26, + path: "src/lib/format.ts", + line: 14, + before: "const result = normalize(value);", + after: "const normalized = normalize(value);", + allowedFileContent: + "export function format(value: string) { const normalized = normalize(value); return normalized; }", + policy: "Benign refactors should stay silent when the behavior does not change.", + scoringLabels: ["clean", "refactor", "silence"], + }, + { + id: "clean-comment-only", + name: "Comment-only cleanup", + pullNumber: 27, + path: "src/lib/logger.ts", + line: 11, + before: "// keep the log prefix stable", + after: "// keep the log prefix stable for tests", + allowedFileContent: 'export const prefix = "app"; // keep the log prefix stable for tests', + policy: "Comment-only churn should not create review noise.", + scoringLabels: ["clean", "comments", "silence"], + }, + { + id: "clean-readme-update", + name: "README wording update", + pullNumber: 28, + path: "README.md", + line: 5, + before: "Postil keeps noisy review out of the way.", + after: "Postil keeps noisy review out of the way for small teams.", + allowedFileContent: "Postil keeps noisy review out of the way for small teams.", + policy: "Plain language copy changes should not be flagged as code defects.", + scoringLabels: ["clean", "copy", "silence"], + }, + { + id: "clean-rename-only", + name: "Local variable rename only", + pullNumber: 29, + path: "src/lib/cache.ts", + line: 20, + before: "const payload = buildPayload(input);", + after: "const cachedPayload = buildPayload(input);", + allowedFileContent: + "export function cache(input: string) { const cachedPayload = buildPayload(input); return cachedPayload; }", + policy: "Rename-only changes should stay silent when behavior is unchanged.", + scoringLabels: ["clean", "rename", "silence"], + }, + { + id: "node-event-listener-leak", + name: "Node event listener leak", + pullNumber: 30, + path: "src/server.js", + line: 1, + before: "source.on('close', () => cleanup());", + after: "source.addEventListener('abort', () => target.abort());", + allowedFileContent: + "export function forwardAbort(source, target) { source.addEventListener('abort', () => target.abort()); }", + policy: "Retained event listeners should be cleaned up when the request completes.", + scoringLabels: ["performance", "memory", "node"], + finding: { + severity: "warn", + body: "The event listener is added for each request; remove it when the request completes so it does not keep growing.", + bodyIncludes: "removeEventListener", + }, + disallowedSources: ["retained event listeners"], + }, +]; + +export const cases: BenchmarkCaseInput[] = fixtureSpecs.map((spec) => buildCase(spec)); diff --git a/bench/package.json b/bench/package.json new file mode 100644 index 0000000..173c562 --- /dev/null +++ b/bench/package.json @@ -0,0 +1,15 @@ +{ + "name": "postil-bench", + "private": true, + "type": "module", + "description": "Hermetic PR-review regression suite for the postil CLI (mock forge + mock model).", + "scripts": { + "bench": "bun run src/run.ts" + }, + "dependencies": { + "zod": "^4.4.3" + }, + "devDependencies": { + "@types/bun": "^1.3.14" + } +} diff --git a/bench/src/harness.ts b/bench/src/harness.ts new file mode 100644 index 0000000..1f2b343 --- /dev/null +++ b/bench/src/harness.ts @@ -0,0 +1,897 @@ +// Hermetic PR-review benchmark harness for the postil CLI. +// +// Each case runs the release binary against a per-case mock GitHub API and a +// mock OpenAI-compatible model endpoint, inside an isolated run directory +// (own HOME/TMPDIR/XDG dirs, no inherited environment), then scores the +// envelope (contract v1) and the forge interactions against the case's ground +// truth. The mock model replays a recorded response generated from the same +// spec as the ground truth, so a green run demonstrates pipeline fidelity +// (grounding, gating, statusline correctness, silence on clean PRs) — not +// detection ability. + +import { execFile as execFileCb } from "node:child_process"; +import { createHash } from "node:crypto"; +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; +import type { AddressInfo } from "node:net"; +import { dirname, join, resolve } from "node:path"; +import { promisify } from "node:util"; +import { z } from "zod"; + +const execFile = promisify(execFileCb); + +// --------------------------------------------------------------------------- +// Case schema + +export const severity = z.enum(["info", "warn", "error"]); + +const expectedFinding = z.object({ + path: z.string(), + line: z.number().int().positive().optional(), + severity: severity.optional(), + bodyIncludes: z.string().optional(), +}); + +const fixtureFile = z.object({ + path: z.string().min(1), + content: z.string(), +}); + +const allowedContext = z.object({ + files: z.array(fixtureFile).default([]), + docs: z.array(fixtureFile).default([]), +}); + +// The recorded response the mock model endpoint replays, shaped for the +// rebuilt CLI contract: findings need kind/confidence/title to survive +// filtering, and an empty findings array must come with an empty summary or +// the CLI fails closed on narrated risk. +const modelFinding = z.object({ + path: z.string(), + line: z.number().int().positive(), + severity, + kind: z.enum(["risk", "humanEscalation", "guardrail", "uncertainty"]), + confidence: z.number().min(0).max(1), + title: z.string(), + body: z.string(), +}); + +const modelOutput = z + .object({ + summary: z.string(), + findings: z.array(modelFinding), + }) + .refine((m) => m.findings.length > 0 || m.summary === "", { + message: "a clean recorded response must have an empty summary (CLI contract)", + }); + +export const benchmarkCase = z.object({ + id: z.string().min(1), + name: z.string().min(1), + repo: z.string().regex(/^[^/]+\/[^/]+$/u), + pullNumber: z.number().int().positive(), + headSha: z.string().min(1), + diff: z.string().min(1), + allowedContext: allowedContext.default({ files: [], docs: [] }), + /// Fixture metadata (policy phrasing, ground-truth labels) that must never + /// leak into the prompt or any pipeline output. + disallowedSources: z.array(z.string().min(1)).default([]), + scoringLabels: z.array(z.string().min(1)).default([]), + groundTruth: z.object({ findings: z.array(expectedFinding).default([]) }).default({ + findings: [], + }), + guardrails: z + .object({ forbiddenPromptSubstrings: z.array(z.string().min(1)).default([]) }) + .default({ forbiddenPromptSubstrings: [] }), + modelOutput, + expectations: z.object({ + minFindings: z.number().int().nonnegative().default(0), + maxFindings: z.number().int().nonnegative().optional(), + requiredFindings: z.array(expectedFinding).default([]), + }), +}); + +export type BenchmarkCase = z.infer; +export type BenchmarkCaseInput = z.input; + +// --------------------------------------------------------------------------- +// Envelope contract (v1) — mirrors src/envelope.rs (camelCase serialization). + +const envelopeFinding = z.object({ + path: z.string(), + line: z.number().int(), + endLine: z.number().int().optional(), + severity, + kind: z.string(), + confidence: z.number(), + title: z.string(), + body: z.string(), +}); + +export const envelopeV1 = z.object({ + version: z.literal(1), + summary: z.string(), + silent: z.boolean(), + findings: z.array(envelopeFinding), + resolved: z.array(envelopeFinding), + counts: z.object({ + info: z.number().int().nonnegative(), + warn: z.number().int().nonnegative(), + error: z.number().int().nonnegative(), + suppressed: z.number().int().nonnegative(), + ungrounded: z.number().int().nonnegative(), + }), + confidenceBuckets: z.array(z.number().int().nonnegative()).length(5), + gate: z.object({ failOn: z.string(), failing: z.boolean() }), + modelUsed: z.string(), + usage: z.object({ + promptTokens: z.number().int().nonnegative(), + completionTokens: z.number().int().nonnegative(), + }), + durationMs: z.number().int().nonnegative(), + baseSha: z.string().nullable(), + headSha: z.string().nullable(), + sinceSha: z.string().nullable(), +}); + +export type Envelope = z.infer; + +// --------------------------------------------------------------------------- +// Results + +export interface BenchmarkOptions { + /** Path to the postil binary (a release build). */ + binary: string; + /** Keep run directories after a green run (failures are always kept). */ + keepRuns?: boolean; + /** Root directory for per-case run dirs. Defaults to bench/.runs. */ + rootDir?: string; + timeoutMs?: number; +} + +export interface BenchmarkMetrics { + truePositives: number; + falsePositives: number; + falseNegatives: number; + severityMatches: number; + fileLineMatches: number; + commentUsefulness: number; +} + +export interface CaseResult { + id: string; + name: string; + ok: boolean; + runDir: string; + findings: number; + scoringLabels: string[]; + metrics: BenchmarkMetrics; + failures: string[]; + envelope?: Envelope; +} + +export interface BenchmarkReport { + ok: boolean; + total: number; + passed: number; + failed: number; + metrics: BenchmarkMetrics; + results: CaseResult[]; +} + +// --------------------------------------------------------------------------- +// Entry point + +export async function runBenchmark( + inputs: BenchmarkCaseInput[], + options: BenchmarkOptions, +): Promise { + const cases = inputs.map((input) => benchmarkCase.parse(input)); + validateUniqueCaseIds(cases); + await assertBinary(options.binary); + + const rootDir = options.rootDir ?? resolve(import.meta.dir, "..", ".runs"); + const results: CaseResult[] = []; + for (const [index, c] of cases.entries()) { + const result = await runCase(c, index, rootDir, options); + results.push(result); + if (result.ok && !options.keepRuns) { + await rm(result.runDir, { recursive: true, force: true }); + } + } + + const passed = results.filter((r) => r.ok).length; + return { + ok: passed === results.length, + total: results.length, + passed, + failed: results.length - passed, + metrics: sumMetrics(results.map((r) => r.metrics)), + results, + }; +} + +async function assertBinary(binary: string) { + const ok = await readFile(binary) + .then(() => true) + .catch(() => false); + if (!ok) { + throw new Error( + `postil binary not found at ${binary} — build it first: cargo build --quiet --release ` + + `(or point POSTIL_BIN at a binary)`, + ); + } +} + +// --------------------------------------------------------------------------- +// Per-case execution + +async function runCase( + c: BenchmarkCase, + index: number, + rootDir: string, + options: BenchmarkOptions, +): Promise { + const runDir = join(rootDir, caseRunDirName(index, c.id)); + await rm(runDir, { recursive: true, force: true }); + const homeDir = join(runDir, "home"); + const tmpDir = join(runDir, "tmp"); + const artifactsDir = join(runDir, "artifacts"); + await mkdir(homeDir, { recursive: true, mode: 0o700 }); + await mkdir(tmpDir, { recursive: true, mode: 0o700 }); + await mkdir(artifactsDir, { recursive: true, mode: 0o700 }); + await materializeAllowedContext(c, runDir); + await writeFile(join(artifactsDir, "pull.diff"), c.diff, { mode: 0o600 }); + + // Authoring guardrails, before anything runs. + const preFailures = [ + ...validateAllowedContext(c), + ...scanForForbidden(c, "fixture diff", c.diff), + ]; + if (preFailures.length > 0) { + return failedCase(c, runDir, preFailures); + } + + const github = await startMockGithub(c); + const model = await startMockModel(c, artifactsDir); + + let exitCode: number | undefined; + let stdout = ""; + let stderr = ""; + try { + const out = await execFile( + options.binary, + ["review", "--repo", c.repo, "--pr", String(c.pullNumber), "--output-json"], + { + cwd: runDir, + env: isolatedEnv(homeDir, tmpDir, github.baseUrl, model.baseUrl), + timeout: options.timeoutMs ?? 120_000, + maxBuffer: 4 * 1024 * 1024, + }, + ); + exitCode = 0; + stdout = out.stdout; + stderr = out.stderr; + } catch (err) { + const e = err as { code?: unknown; stdout?: string; stderr?: string }; + exitCode = typeof e.code === "number" ? e.code : undefined; + stdout = e.stdout ?? ""; + stderr = e.stderr ?? ""; + } finally { + await github.close(); + await model.close(); + } + await writeFile(join(artifactsDir, "stdout.json"), stdout, { mode: 0o600 }); + await writeFile(join(artifactsDir, "stderr.log"), stderr, { mode: 0o600 }); + + const parsed = envelopeV1.safeParse(safeJson(stdout)); + if (!parsed.success) { + const detail = exitCode === undefined ? firstLine(stderr) : `exit code ${exitCode}`; + return failedCase(c, runDir, [ + `reviewer did not emit a valid v1 envelope on stdout (${detail}): ${firstIssue(parsed.error)}`, + ]); + } + const envelope = parsed.data; + + // Prompt-leakage guardrails: fixture metadata (policy phrasing, ground-truth + // labels) must not appear in anything the pipeline sent or produced. + const leakFailures = [ + ...model.requestBodies.flatMap((body, i) => scanForForbidden(c, `model request #${i + 1}`, body)), + ...scanForForbidden(c, "envelope output", stdout), + ...github.requests + .filter((r) => r.body.length > 0) + .flatMap((r) => scanForForbidden(c, `forge ${r.method} ${r.path}`, r.body)), + ]; + + const metrics = scoreFindings(c.groundTruth.findings, envelope.findings); + const failures = [ + ...leakFailures, + ...evaluateEnvelope(c, envelope, exitCode), + ...evaluateForgeInteractions(c, envelope, github), + ...evaluateExpectations(c, envelope, metrics), + ]; + + return { + id: c.id, + name: c.name, + ok: failures.length === 0, + runDir, + findings: envelope.findings.length, + scoringLabels: c.scoringLabels, + metrics, + failures, + envelope, + }; +} + +function failedCase(c: BenchmarkCase, runDir: string, failures: string[]): CaseResult { + return { + id: c.id, + name: c.name, + ok: false, + runDir, + findings: 0, + scoringLabels: c.scoringLabels, + metrics: emptyMetrics(), + failures, + }; +} + +// --------------------------------------------------------------------------- +// Isolation + +function isolatedEnv( + homeDir: string, + tmpDir: string, + githubBaseUrl: string, + modelBaseUrl: string, +): NodeJS.ProcessEnv { + // A fresh environment, never the parent's: no developer keys, no repo + // config discovery beyond the run directory. + return { + PATH: process.env.PATH, + CI: "true", + NO_COLOR: "1", + HOME: homeDir, + TMPDIR: tmpDir, + XDG_CACHE_HOME: join(homeDir, ".cache"), + XDG_CONFIG_HOME: join(homeDir, ".config"), + XDG_DATA_HOME: join(homeDir, ".local", "share"), + GIT_CONFIG_NOSYSTEM: "1", + GIT_TERMINAL_PROMPT: "0", + POSTIL_API_BASE: modelBaseUrl, + POSTIL_API_KEY: "benchmark-api-key", + GITHUB_API_URL: githubBaseUrl, + GITHUB_TOKEN: "benchmark-github-token", + REVIEW_MODEL: "postil-bench/recorded", + }; +} + +// --------------------------------------------------------------------------- +// Mock servers + +interface RecordedRequest { + method: string; + path: string; + accept: string; + body: string; +} + +async function startMockGithub(c: BenchmarkCase) { + const requests: RecordedRequest[] = []; + const checkRunNames = new Map(); // id -> check name + let nextCheckRunId = 1001; + const pullPath = `/repos/${c.repo}/pulls/${c.pullNumber}`; + const checkRunsPath = `/repos/${c.repo}/check-runs`; + const contentsPrefix = `/repos/${c.repo}/contents/`; + const allowedContent = allowedContextByPath(c); + const baseSha = "0".repeat(40); + + const server = createServer(async (req: IncomingMessage, res: ServerResponse) => { + const url = new URL(req.url ?? "/", "http://127.0.0.1"); + const accept = String(req.headers.accept ?? ""); + const body = await readRequestBody(req); + requests.push({ method: req.method ?? "", path: url.pathname, accept, body }); + + if (req.method === "GET" && url.pathname === pullPath) { + if (accept.includes("diff")) { + res.writeHead(200, { "content-type": "text/plain; charset=utf-8" }); + res.end(c.diff); + } else { + res.writeHead(200, { "content-type": "application/json" }); + res.end( + JSON.stringify({ + title: c.name, + body: "", + head: { sha: c.headSha }, + base: { sha: baseSha }, + }), + ); + } + return; + } + + if (req.method === "GET" && url.pathname.startsWith(contentsPrefix)) { + const requested = decodeURIComponent(url.pathname.slice(contentsPrefix.length)); + const content = allowedContent.get(requested); + if (content !== undefined) { + res.writeHead(200, { "content-type": "text/plain; charset=utf-8" }); + res.end(content); + return; + } + res.writeHead(404, { "content-type": "application/json" }); + res.end(JSON.stringify({ message: "Not Found" })); + return; + } + + if (req.method === "POST" && url.pathname === checkRunsPath) { + const id = nextCheckRunId; + nextCheckRunId += 1; + const name = (safeJson(body) as { name?: string } | undefined)?.name ?? ""; + checkRunNames.set(String(id), name); + res.writeHead(201, { "content-type": "application/json" }); + res.end(JSON.stringify({ id })); + return; + } + + if (req.method === "PATCH" && /^.*\/check-runs\/\d+$/u.test(url.pathname)) { + res.writeHead(200, { "content-type": "application/json" }); + res.end("{}"); + return; + } + + if (req.method === "POST" && url.pathname === `${pullPath}/reviews`) { + res.writeHead(200, { "content-type": "application/json" }); + res.end("{}"); + return; + } + + res.writeHead(404, { "content-type": "application/json" }); + res.end(JSON.stringify({ message: "Not Found" })); + }); + + await listen(server); + return { + baseUrl: serverBaseUrl(server), + requests, + checkRunNames, + baseSha, + pullPath, + checkRunsPath, + close: () => closeServer(server), + }; +} + +async function startMockModel(c: BenchmarkCase, artifactsDir: string) { + const requestBodies: string[] = []; + const server = createServer(async (req: IncomingMessage, res: ServerResponse) => { + if (req.method === "POST" && req.url === "/chat/completions") { + const body = await readRequestBody(req); + requestBodies.push(body); + await writeFile(join(artifactsDir, `model-request-${requestBodies.length}.json`), body, { + mode: 0o600, + }); + res.writeHead(200, { "content-type": "application/json" }); + res.end( + JSON.stringify({ + choices: [{ message: { content: JSON.stringify(c.modelOutput) } }], + usage: { prompt_tokens: 10, completion_tokens: 5, total_tokens: 15 }, + }), + ); + return; + } + res.writeHead(404, { "content-type": "application/json" }); + res.end(JSON.stringify({ error: "not found" })); + }); + + await listen(server); + return { + baseUrl: serverBaseUrl(server), + requestBodies, + close: () => closeServer(server), + }; +} + +function listen(server: ReturnType): Promise { + return new Promise((resolvePromise, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", () => { + server.off("error", reject); + resolvePromise(); + }); + }); +} + +function closeServer(server: ReturnType): Promise { + return new Promise((resolvePromise, reject) => { + server.close((err) => (err ? reject(err) : resolvePromise())); + }); +} + +function serverBaseUrl(server: ReturnType): string { + const address = server.address() as AddressInfo; + return `http://127.0.0.1:${address.port}`; +} + +function readRequestBody(req: IncomingMessage): Promise { + return new Promise((resolvePromise, reject) => { + const chunks: Buffer[] = []; + req.on("data", (chunk) => chunks.push(Buffer.from(chunk))); + req.on("end", () => resolvePromise(Buffer.concat(chunks).toString("utf8"))); + req.on("error", reject); + }); +} + +// --------------------------------------------------------------------------- +// Fixture materialization and authoring guardrails + +async function materializeAllowedContext(c: BenchmarkCase, runDir: string) { + const repo = join(runDir, "context", "repo"); + const docs = join(runDir, "context", "docs"); + await mkdir(repo, { recursive: true, mode: 0o700 }); + await mkdir(docs, { recursive: true, mode: 0o700 }); + for (const file of c.allowedContext.files) { + await writeFixtureFile(repo, file); + } + for (const doc of c.allowedContext.docs) { + await writeFixtureFile(docs, doc); + } +} + +async function writeFixtureFile(rootDir: string, file: z.infer) { + const safePath = file.path + .split(/[\\/]+/u) + .filter((part) => part && part !== "." && part !== "..") + .join("/"); + const destination = join(rootDir, safePath); + await mkdir(dirname(destination), { recursive: true, mode: 0o700 }); + await writeFile(destination, file.content, { mode: 0o600 }); +} + +function allowedContextByPath(c: BenchmarkCase): Map { + const context = new Map(); + for (const file of c.allowedContext.files) { + context.set(file.path, file.content); + } + for (const doc of c.allowedContext.docs) { + context.set(doc.path, doc.content); + } + return context; +} + +function validateAllowedContext(c: BenchmarkCase): string[] { + const allowedFiles = new Set(c.allowedContext.files.map((file) => file.path)); + if (allowedFiles.size === 0) return []; + + const diffPaths = new Set(); + for (const line of c.diff.split("\n")) { + const match = /^(?:diff --git a\/(.+) b\/(.+)|--- a\/(.+)|\+\+\+ b\/(.+))$/u.exec(line); + if (!match) continue; + for (const path of match.slice(1).filter(Boolean)) { + if (path !== "/dev/null") diffPaths.add(path); + } + } + + return [...diffPaths] + .filter((path) => !allowedFiles.has(path)) + .map((path) => `diff references ${path}, which is not declared as allowed context`); +} + +function scanForForbidden(c: BenchmarkCase, where: string, content: string): string[] { + const forbidden = [...c.disallowedSources, ...c.guardrails.forbiddenPromptSubstrings]; + const failures: string[] = []; + for (const token of forbidden) { + if (content.includes(token)) { + failures.push(`guardrail: fixture metadata leaked into ${where}`); + } + } + return [...new Set(failures)]; +} + +// --------------------------------------------------------------------------- +// Evaluation + +/** The default gate (failOn: error) should fail exactly when the ground truth + * contains an error-severity finding. */ +function expectedGateFailing(c: BenchmarkCase): boolean { + return c.groundTruth.findings.some((f) => f.severity === "error"); +} + +function evaluateEnvelope(c: BenchmarkCase, env: Envelope, exitCode: number | undefined): string[] { + const failures: string[] = []; + const clean = c.groundTruth.findings.length === 0; + + if (env.headSha !== c.headSha) { + failures.push(`envelope headSha ${env.headSha} != PR head ${c.headSha}`); + } + if (env.counts.ungrounded !== 0) { + failures.push(`counts.ungrounded is ${env.counts.ungrounded}: the pipeline dropped grounded findings`); + } + if (env.silent !== (env.findings.length === 0)) { + failures.push(`silent=${env.silent} contradicts ${env.findings.length} finding(s)`); + } + if (clean) { + if (!env.silent) failures.push("clean case: envelope is not silent"); + if (env.summary !== "") failures.push("clean case: summary is not empty"); + } + const gateShouldFail = expectedGateFailing(c); + if (env.gate.failing !== gateShouldFail) { + failures.push(`gate.failing=${env.gate.failing}, expected ${gateShouldFail} (failOn: ${env.gate.failOn})`); + } + const expectedExit = gateShouldFail ? 1 : 0; + if (exitCode !== expectedExit) { + failures.push(`exit code ${exitCode ?? "unknown"}, expected ${expectedExit}`); + } + // Synthetic operational findings mean the pipeline did not trust the run. + for (const f of env.findings) { + if (f.path.startsWith(".postil/")) { + failures.push(`synthetic finding ${f.path}: ${f.title}`); + } + } + return failures; +} + +/** Statusline correctness: both checks created and completed with the right + * conclusions, and review comments posted exactly when there are findings. */ +function evaluateForgeInteractions( + c: BenchmarkCase, + env: Envelope, + github: Awaited>, +): string[] { + const failures: string[] = []; + + const creates = github.requests.filter( + (r) => r.method === "POST" && r.path === github.checkRunsPath, + ); + const names = new Set( + creates.map((r) => (safeJson(r.body) as { name?: string } | undefined)?.name ?? ""), + ); + if (creates.length !== 2 || !names.has("postil/review") || !names.has("postil/gate")) { + failures.push( + `expected check-run creations for postil/review and postil/gate, got [${[...names].join(", ")}]`, + ); + } + + const patches = github.requests.filter( + (r) => r.method === "PATCH" && /\/check-runs\/\d+$/u.test(r.path), + ); + if (patches.length !== 2) { + failures.push(`expected 2 check-run completions, got ${patches.length}`); + } + const conclusionByName = new Map(); + for (const patch of patches) { + const id = patch.path.split("/").pop() ?? ""; + const name = github.checkRunNames.get(id) ?? `unknown-${id}`; + const conclusion = (safeJson(patch.body) as { conclusion?: string } | undefined)?.conclusion; + conclusionByName.set(name, conclusion ?? "missing"); + } + const wantGate = env.gate.failing ? "failure" : "success"; + if (conclusionByName.size > 0) { + if (conclusionByName.get("postil/gate") !== wantGate) { + failures.push( + `postil/gate concluded ${conclusionByName.get("postil/gate")}, expected ${wantGate}`, + ); + } + if (conclusionByName.get("postil/review") !== "success") { + failures.push( + `postil/review concluded ${conclusionByName.get("postil/review")}, expected success`, + ); + } + } + + const reviews = github.requests.filter( + (r) => r.method === "POST" && r.path === `${github.pullPath}/reviews`, + ); + if (c.groundTruth.findings.length === 0) { + // Silence is the product: a clean PR gets no review comment at all. + if (reviews.length !== 0) { + failures.push(`clean case: ${reviews.length} review comment(s) posted, expected silence`); + } + } else { + if (reviews.length !== 1) { + failures.push(`expected exactly 1 posted review, got ${reviews.length}`); + } + const comments = reviews.flatMap( + (r) => (safeJson(r.body) as { comments?: { path?: string; line?: number }[] })?.comments ?? [], + ); + for (const expected of c.groundTruth.findings) { + const anchored = comments.some( + (cm) => cm.path === expected.path && (expected.line === undefined || cm.line === expected.line), + ); + if (!anchored) { + failures.push(`no inline comment anchored at ${expected.path}:${expected.line ?? "?"}`); + } + } + } + + return failures; +} + +function evaluateExpectations( + c: BenchmarkCase, + env: Envelope, + metrics: BenchmarkMetrics, +): string[] { + const failures: string[] = []; + const { expectations } = c; + if (env.findings.length < expectations.minFindings) { + failures.push( + `expected at least ${expectations.minFindings} finding(s), got ${env.findings.length}`, + ); + } + if (expectations.maxFindings !== undefined && env.findings.length > expectations.maxFindings) { + failures.push( + `expected at most ${expectations.maxFindings} finding(s), got ${env.findings.length}`, + ); + } + + for (const required of expectations.requiredFindings) { + const match = env.findings.some((finding) => { + if (finding.path !== required.path) return false; + if (required.line !== undefined && finding.line !== required.line) return false; + if (required.severity !== undefined && finding.severity !== required.severity) return false; + return commentMatchesExpectation(finding.body, required.bodyIncludes); + }); + if (!match) { + failures.push(`missing required finding in ${required.path}`); + } + } + if (metrics.falseNegatives > 0) { + failures.push(`missed ${metrics.falseNegatives} ground truth finding(s)`); + } + return failures; +} + +// --------------------------------------------------------------------------- +// Scoring + +function scoreFindings( + expected: z.infer[], + actual: Envelope["findings"], +): BenchmarkMetrics { + const matchedActual = new Set(); + let truePositives = 0; + let severityMatches = 0; + let fileLineMatches = 0; + let commentUsefulness = 0; + + for (const want of expected) { + const matchIndex = actual.findIndex( + (finding, index) => + !matchedActual.has(index) && + finding.path === want.path && + (want.line === undefined || finding.line === want.line), + ); + if (matchIndex === -1) continue; + + matchedActual.add(matchIndex); + truePositives += 1; + const finding = actual[matchIndex]!; + if (want.severity === undefined || finding.severity === want.severity) { + severityMatches += 1; + } + if (want.line !== undefined && finding.line === want.line) { + fileLineMatches += 1; + } + if (commentMatchesExpectation(finding.body, want.bodyIncludes)) { + commentUsefulness += 1; + } + } + + return { + truePositives, + falsePositives: actual.length - matchedActual.size, + falseNegatives: expected.length - truePositives, + severityMatches, + fileLineMatches, + commentUsefulness, + }; +} + +function commentMatchesExpectation(comment: string, bodyIncludes: string | undefined): boolean { + if (bodyIncludes === undefined) return true; + if (comment.includes(bodyIncludes)) return true; + + const expectedTokens = tokenizeForUsefulness(bodyIncludes); + if (expectedTokens.length === 0) return true; + + const commentTokens = new Set(tokenizeForUsefulness(comment)); + return expectedTokens.every((token) => commentTokens.has(token)); +} + +function tokenizeForUsefulness(value: string): string[] { + return value + .replace(/([a-z0-9])([A-Z])/gu, "$1 $2") + .toLowerCase() + .split(/[^a-z0-9]+/u) + .map(normalizeUsefulnessToken) + .filter((token) => token.length > 2); +} + +function normalizeUsefulnessToken(token: string): string { + if (token.startsWith("remov")) return "remove"; + return token; +} + +function sumMetrics(metrics: BenchmarkMetrics[]): BenchmarkMetrics { + return metrics.reduce( + (sum, item) => ({ + truePositives: sum.truePositives + item.truePositives, + falsePositives: sum.falsePositives + item.falsePositives, + falseNegatives: sum.falseNegatives + item.falseNegatives, + severityMatches: sum.severityMatches + item.severityMatches, + fileLineMatches: sum.fileLineMatches + item.fileLineMatches, + commentUsefulness: sum.commentUsefulness + item.commentUsefulness, + }), + emptyMetrics(), + ); +} + +function emptyMetrics(): BenchmarkMetrics { + return { + truePositives: 0, + falsePositives: 0, + falseNegatives: 0, + severityMatches: 0, + fileLineMatches: 0, + commentUsefulness: 0, + }; +} + +function validateUniqueCaseIds(cases: BenchmarkCase[]) { + const ids = new Set(); + for (const c of cases) { + if (ids.has(c.id)) { + throw new Error(`duplicate benchmark case id: ${c.id}`); + } + ids.add(c.id); + } +} + +// --------------------------------------------------------------------------- +// Reporting + +export function formatReport(report: BenchmarkReport): string { + const lines = [ + `postil bench (mock mode): ${report.passed}/${report.total} passed`, + `TP ${report.metrics.truePositives} | FP ${report.metrics.falsePositives} | FN ${report.metrics.falseNegatives}`, + "Note: the mock model replays recorded output; this measures pipeline fidelity, not detection ability.", + ]; + for (const result of report.results) { + const status = result.ok ? "PASS" : "FAIL"; + lines.push(`${status} ${result.id}: ${result.findings} finding(s)`); + for (const failure of result.failures) { + lines.push(` - ${failure}`); + } + if (!result.ok) { + lines.push(` run dir kept at ${result.runDir}`); + } + } + return lines.join("\n"); +} + +// --------------------------------------------------------------------------- +// Small utilities + +function safeJson(raw: string): unknown { + try { + return JSON.parse(raw); + } catch { + return undefined; + } +} + +function firstLine(text: string): string { + return text.split("\n").find((l) => l.trim()) ?? "no output"; +} + +function firstIssue(error: z.ZodError): string { + const issue = error.issues[0]; + if (!issue) return "unknown validation error"; + return `${issue.path.join(".") || "(root)"}: ${issue.message}`; +} + +function caseRunDirName(index: number, id: string): string { + const digest = createHash("sha256").update(id).digest("hex").slice(0, 12); + return `case-${index + 1}-${digest}`; +} diff --git a/bench/src/run.ts b/bench/src/run.ts new file mode 100644 index 0000000..2f22293 --- /dev/null +++ b/bench/src/run.ts @@ -0,0 +1,40 @@ +#!/usr/bin/env bun +// CLI entry point: run all benchmark cases against a release build. +// +// bun run bench [--json] [--json-out ] +// +// Environment: +// POSTIL_BIN path to the postil binary (default ../target/release/postil) +// POSTIL_BENCH_KEEP_RUNS set to 1 to keep run directories after a green run + +import { writeFile } from "node:fs/promises"; +import { resolve } from "node:path"; +import { cases } from "../fixtures/cases"; +import { formatReport, runBenchmark } from "./harness"; + +async function main() { + const args = process.argv.slice(2); + const json = args.includes("--json"); + const jsonOutIndex = args.indexOf("--json-out"); + const jsonOut = jsonOutIndex === -1 ? undefined : args[jsonOutIndex + 1]; + + const report = await runBenchmark(cases, { + binary: process.env.POSTIL_BIN ?? resolve(import.meta.dir, "..", "..", "target", "release", "postil"), + keepRuns: process.env.POSTIL_BENCH_KEEP_RUNS === "1", + }); + + const jsonReport = JSON.stringify(report, null, 2); + if (jsonOut) { + await writeFile(jsonOut, `${jsonReport}\n`); + } + console.log(json ? jsonReport : formatReport(report)); + + if (!report.ok) { + process.exitCode = 1; + } +} + +main().catch((err) => { + console.error(err instanceof Error ? err.message : String(err)); + process.exitCode = 1; +}); diff --git a/bench/tsconfig.json b/bench/tsconfig.json new file mode 100644 index 0000000..85a9dc7 --- /dev/null +++ b/bench/tsconfig.json @@ -0,0 +1,12 @@ +{ + "compilerOptions": { + "target": "ESNext", + "module": "ESNext", + "moduleResolution": "bundler", + "strict": true, + "noUncheckedIndexedAccess": true, + "noEmit": true, + "types": ["bun"] + }, + "include": ["src", "fixtures"] +} From 96abe6b9d0ea5a805a6979c621cd6c338813cf8d Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Sat, 13 Jun 2026 01:56:35 +0000 Subject: [PATCH 2/2] Run the benchmark suite in CI A separate bench job builds the release binary and runs the 30-case mock-mode suite with a frozen lockfile. --- .github/workflows/ci.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed6d71a..3ed4f84 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,3 +21,16 @@ jobs: - run: cargo clippy --quiet --all-targets -- -D warnings - run: cargo test --quiet - run: cargo build --quiet --release + + bench: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@v2 + - uses: oven-sh/setup-bun@v2 + - run: cargo build --quiet --release + - run: bun install --frozen-lockfile + working-directory: bench + - run: bun run bench + working-directory: bench