diff --git a/Cargo.lock b/Cargo.lock index 9108fb7..54b18c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1104,7 +1104,7 @@ checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" [[package]] name = "postil-cli" -version = "0.8.17" +version = "0.8.18" dependencies = [ "aho-corasick", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index aeddbcb..610c53c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "postil-cli" -version = "0.8.17" +version = "0.8.18" edition = "2024" description = "Postil: a low-noise AI review gate. Silent on clean PRs, hard gate on real risk." license = "Apache-2.0" diff --git a/bench/README.md b/bench/README.md index f0ccad1..70e06ae 100644 --- a/bench/README.md +++ b/bench/README.md @@ -454,9 +454,13 @@ classification), mean provider cost per case, and p95 review latency. Each metric has its own tolerance (see the exported `*_MAX_*` constants at the top of `compare-baseline.ts`). -Three of those metrics block a release: detection rate, mean cost per case, and -p95 latency. False/unrelated findings and gate-verdict correctness are reported -but never block, because one run cannot measure them precisely enough to act on. +Detection rate and p95 latency always block a release when they cross their +tolerances. Mean provider cost blocks only when the baseline and current run +share the same enforced provider profile; without that identity, the comparison +is a trend report. False/unrelated findings and gate-verdict correctness are +reported but never block, because one run cannot measure them precisely enough +to act on. The CLI's per-operation cost cap remains the deterministic spending +boundary. Six runs of a single unchanged binary against this corpus, four on managed routing and two pinned to the qualified upstream provider, spanned 4 to 7 false findings and 12.9 percentage points of gate-verdict correctness, against @@ -466,7 +470,7 @@ the provider did not remove it: the widest false-finding count came from a pinned run. Detection rate over the same six runs spanned 3.5 points, which leaves a threshold that still catches a real regression. -Read the two reported metrics across releases rather than acting on a single +Read the non-blocking metrics across releases rather than acting on a single run. Making them blocking again means reducing the noise rather than tightening the number: comparing a median across repeated runs is the direct fix, at proportionally more cost and wall-clock per release. A gate that fails at diff --git a/bench/baseline.json b/bench/baseline.json index 6ac6bfb..d199daf 100644 --- a/bench/baseline.json +++ b/bench/baseline.json @@ -10,6 +10,9 @@ "generatedAt": "2026-08-13T10:05:48.461Z", "reviewMode": "exhaustive", "sourceRunAt": "2026-08-13T10:04:27.827Z", + "providerContractEnforced": false, + "screeningProfileSha256": null, + "upstreamProviderIdentity": null, "totalCases": 70, "scoredCases": 68, "detectionRate": 0.9473684210526315, diff --git a/bench/src/compare-baseline.test.ts b/bench/src/compare-baseline.test.ts index 9a7cf72..da55adf 100644 --- a/bench/src/compare-baseline.test.ts +++ b/bench/src/compare-baseline.test.ts @@ -38,6 +38,7 @@ function fakeReport(overrides: { observedProviderCostUsdDecimal?: string; results?: LiveReportForComparison["results"]; detected?: number; + providerContractEnforced?: boolean; } = {}): LiveReportForComparison { const results: LiveReportForComparison["results"] = overrides.results ?? [ { id: "must-block-1", type: "defect", scored: true, truthSeverity: "error", durationMs: 1000, exitCode: 1 }, @@ -50,6 +51,9 @@ function fakeReport(overrides: { summary: { model: "z-ai/glm-5.2", reviewMode: "exhaustive", + providerContractEnforced: overrides.providerContractEnforced ?? false, + screeningProfileSha256: overrides.providerContractEnforced === true ? "profile-sha" : null, + upstreamProviderIdentity: overrides.providerContractEnforced === true ? "PinnedProvider" : null, fixtureCorpusSha256: "corpus-sha", evaluatorSha256: "evaluator-sha", totalCases: results.length, @@ -69,6 +73,9 @@ const populatedBaseline: Extract = { generatedAt: "2026-01-01T00:00:00.000Z", reviewMode: "exhaustive", sourceRunAt: "2026-01-01T00:00:00.000Z", + providerContractEnforced: false, + screeningProfileSha256: null, + upstreamProviderIdentity: null, totalCases: 70, scoredCases: 70, detectionRate: 0.95, @@ -235,10 +242,51 @@ describe("compareMetrics", () => { expect(comparison.ok).toBe(true); }); - test("fails when mean cost rises past the ratio budget", () => { + test("reports mean cost past the ratio budget without blocking", () => { const observed = { ...extractObservedMetrics(fakeReport()), meanCostUsdPerCase: populatedBaseline.meanCostUsdPerCase * (1 + MEAN_COST_MAX_INCREASE_RATIO) + 0.001 }; const comparison = compareMetrics(populatedBaseline, observed); - expect(comparison.rows.find((row) => row.metric === "mean cost per case")?.verdict).toBe("FAIL"); + const row = comparison.rows.find((candidate) => candidate.metric === "mean cost per case"); + expect(row?.verdict).toBe("FAIL"); + expect(row?.informational).toBe(true); + expect(comparison.ok).toBe(true); + }); + + test("blocks a cost regression under the same enforced provider contract", () => { + const baseline = { + ...populatedBaseline, + providerContractEnforced: true, + screeningProfileSha256: "profile-sha", + upstreamProviderIdentity: "PinnedProvider", + }; + const observed = { + ...extractObservedMetrics(fakeReport({ providerContractEnforced: true })), + meanCostUsdPerCase: + baseline.meanCostUsdPerCase * (1 + MEAN_COST_MAX_INCREASE_RATIO) + 0.001, + }; + const comparison = compareMetrics(baseline, observed); + const row = comparison.rows.find((candidate) => candidate.metric === "mean cost per case"); + expect(row?.verdict).toBe("FAIL"); + expect(row?.informational).toBe(false); + expect(comparison.ok).toBe(false); + }); + + test("does not treat missing provider identity as a comparable contract", () => { + const baseline = { + ...populatedBaseline, + providerContractEnforced: true, + screeningProfileSha256: null, + upstreamProviderIdentity: null, + }; + const observed = { + ...extractObservedMetrics(fakeReport()), + providerContractEnforced: true, + meanCostUsdPerCase: + baseline.meanCostUsdPerCase * (1 + MEAN_COST_MAX_INCREASE_RATIO) + 0.001, + }; + const comparison = compareMetrics(baseline, observed); + const row = comparison.rows.find((candidate) => candidate.metric === "mean cost per case"); + expect(row?.informational).toBe(true); + expect(comparison.ok).toBe(true); }); test("fails when p95 latency rises past the ratio budget", () => { diff --git a/bench/src/compare-baseline.ts b/bench/src/compare-baseline.ts index ed3d416..af9c4d7 100644 --- a/bench/src/compare-baseline.ts +++ b/bench/src/compare-baseline.ts @@ -80,9 +80,10 @@ export const MEASURED_RUN_TO_RUN_SPREAD = { gateVerdictPp: 12.9, } as const; -/** Mean provider cost per case may rise at most this fraction above baseline - * before the gate fails. Provider pricing and routing drift more than model - * behavior does, so this tolerance is wider than the detection tolerances. */ +/** Mean provider cost per case above this fraction is reported for inspection. + * One live run cannot separate variable provider output length and routing + * from a call-plan regression, while the CLI's per-operation cap remains the + * deterministic release safety boundary. */ export const MEAN_COST_MAX_INCREASE_RATIO = 0.25; /** p95 review latency may rise at most this fraction above baseline. Wide @@ -107,6 +108,9 @@ const liveReportSchema = z.object({ summary: z.object({ model: z.string(), reviewMode: z.enum(["exhaustive", "bounded"]), + providerContractEnforced: z.boolean(), + screeningProfileSha256: z.string().nullable(), + upstreamProviderIdentity: z.string().nullable(), fixtureCorpusSha256: z.string(), evaluatorSha256: z.string(), totalCases: z.number().int().nonnegative(), @@ -135,6 +139,9 @@ const baselineProfileSchema = z.discriminatedUnion("populated", [ generatedAt: z.string(), reviewMode: z.enum(["exhaustive", "bounded"]), sourceRunAt: z.string(), + providerContractEnforced: z.boolean(), + screeningProfileSha256: z.string().nullable(), + upstreamProviderIdentity: z.string().nullable(), totalCases: z.number().int().positive(), scoredCases: z.number().int().positive(), detectionRate: z.number().min(0).max(1), @@ -166,6 +173,9 @@ export type BaselineProfile = z.infer; export interface ObservedMetrics { model: string; reviewMode: "exhaustive" | "bounded"; + providerContractEnforced: boolean; + screeningProfileSha256: string | null; + upstreamProviderIdentity: string | null; fixtureCorpusSha256: string; evaluatorSha256: string; ranAt: string; @@ -224,6 +234,9 @@ export function extractObservedMetrics(report: LiveReportForComparison): Observe return { model: s.model, reviewMode: s.reviewMode, + providerContractEnforced: s.providerContractEnforced, + screeningProfileSha256: s.screeningProfileSha256, + upstreamProviderIdentity: s.upstreamProviderIdentity, fixtureCorpusSha256: s.fixtureCorpusSha256, evaluatorSha256: s.evaluatorSha256, ranAt: s.ranAt, @@ -303,12 +316,21 @@ export function compareMetrics(baseline: Extract row.metric).join(", ")}. ` + - `One run cannot separate this from provider nondeterminism (${MEASURED_RUN_TO_RUN_SPREAD.runs} runs ` + - `of one unchanged binary spanned ${MEASURED_RUN_TO_RUN_SPREAD.gateVerdictPp}pp of gate verdict ` + - `correctness). Worth reading across several releases, not acting on alone.`, + "One run cannot separate these signals from provider nondeterminism. Worth reading across " + + "several releases, not acting on alone.", ); } return lines.join("\n"); @@ -415,6 +436,9 @@ async function main() { generatedAt: new Date().toISOString(), reviewMode: observed.reviewMode, sourceRunAt: observed.ranAt, + providerContractEnforced: observed.providerContractEnforced, + screeningProfileSha256: observed.screeningProfileSha256, + upstreamProviderIdentity: observed.upstreamProviderIdentity, totalCases: observed.totalCases, scoredCases: observed.scoredCases, detectionRate: observed.detectionRate, diff --git a/src/prompt.rs b/src/prompt.rs index 797ae40..3dd2385 100644 --- a/src/prompt.rs +++ b/src/prompt.rs @@ -144,10 +144,14 @@ pub fn review_contract(cfg: &Config) -> String { `.postil/change-metadata`. Findings citing other lines are discarded as \ ungrounded.\n\ \n\ - Every finding MUST include `repositoryContext`: `none` is diff-local; `absence` names the \ - missing construct; `mismatch` names a target in resources, paths, or identifiers and an \ - expected value in values or versions. Populated arrays are conjunctive and refute only \ - when matched in one file. Repository claims require the complete reviewed head. Public \ + Every finding MUST include `repositoryContext`. Use `none` whenever the exact cited \ + changed line is sufficient to establish the defect, even if unchanged code elsewhere \ + could also be relevant. Use `absence` only when the conclusion depends on a construct \ + missing from the complete reviewed head. Use `mismatch` only when the conclusion depends \ + on a repository target having an expected value not established by the cited changed \ + line; name that target in resources, paths, or identifiers and the expected value in \ + values or versions. Populated arrays are conjunctive and refute only when matched in one \ + file. Repository claims require the complete reviewed head. Public \ text names the concrete construct \ and correction, never review-input boundaries such as `in the diff`, retrieval mechanics, \ delegated evidence collection, or guessed files.\n", @@ -541,6 +545,20 @@ mod tests { } } + #[test] + fn generator_reserves_repository_claims_for_repository_dependent_conclusions() { + let prompt = system_prompt(&Config::default(), trusted_date()); + assert!(prompt.contains( + "Use `none` whenever the exact cited changed line is sufficient to establish the defect" + )); + assert!(prompt.contains( + "Use `absence` only when the conclusion depends on a construct missing from the complete reviewed head" + )); + assert!(prompt.contains( + "Use `mismatch` only when the conclusion depends on a repository target having an expected value" + )); + } + #[test] fn system_prompt_injects_guardrails() { let mut cfg = Config::default(); diff --git a/src/review.rs b/src/review.rs index bc07d7c..25b4fae 100644 --- a/src/review.rs +++ b/src/review.rs @@ -3297,12 +3297,12 @@ mod tests { let below_floor = format!("Benchmark pull request{}", "x".repeat(442)); let local_budgets = batch_budgets_for_title(&local_edge); - assert_eq!(local_budgets.synthesis, 4_495); - assert_eq!(local_budgets.source, 4_496); + assert_eq!(local_budgets.synthesis, 4_177); + assert_eq!(local_budgets.source, 4_178); assert!(review_batch_budgets_are_usable(local_budgets)); let ci_budgets = batch_budgets_for_title(&ci_edge); - assert_eq!(ci_budgets.synthesis, 4_479); - assert_eq!(ci_budgets.source, 4_480); + assert_eq!(ci_budgets.synthesis, 4_161); + assert_eq!(ci_budgets.source, 4_162); assert!(review_batch_budgets_are_usable(ci_budgets)); assert_eq!( ci_budgets.stabilized_for_rendering(), @@ -3314,7 +3314,7 @@ mod tests { let below_floor_budgets = batch_budgets_for_title(&below_floor); assert_eq!( below_floor_budgets.synthesis, - diff::MIN_REVIEW_BATCH_BYTES - 1 + diff::MIN_REVIEW_BATCH_BYTES - 319 ); assert!(!review_batch_budgets_are_usable(below_floor_budgets)); }