From 04ed8ad52880ed03b20fb8780f139d7f31cfe42c Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Tue, 25 Aug 2026 18:58:13 +0000 Subject: [PATCH 1/7] Suppress unsupported lockfile platform claims --- src/envelope.rs | 3 + src/forge/mod.rs | 3 + src/review.rs | 300 ++++++++++++++++++++++++++++++++++++++++++++++- tests/e2e.rs | 177 ++++++++++++++++++++++++++++ 4 files changed, 482 insertions(+), 1 deletion(-) diff --git a/src/envelope.rs b/src/envelope.rs index 061d808..9e545a2 100644 --- a/src/envelope.rs +++ b/src/envelope.rs @@ -1303,6 +1303,9 @@ pub enum SuppressionReason { MachineClaimRefuted, /// The source verifier could not produce a bounded conclusive proof. MachineClaimUnverified, + /// Compact lockfile metadata cannot establish the claimed platform, OS, + /// CPU architecture, ABI, or runtime support conclusion. + LockfilePlatformEvidenceInsufficient, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/forge/mod.rs b/src/forge/mod.rs index b8fe49c..75e1a01 100644 --- a/src/forge/mod.rs +++ b/src/forge/mod.rs @@ -2028,6 +2028,9 @@ fn suppression_reason(reason: SuppressionReason) -> &'static str { SuppressionReason::MachineClaimUnverified => { "source premise could not be verified within deterministic bounds" } + SuppressionReason::LockfilePlatformEvidenceInsufficient => { + "compact lockfile metadata cannot establish platform support" + } } } diff --git a/src/review.rs b/src/review.rs index 3434dc0..c3cd5ab 100644 --- a/src/review.rs +++ b/src/review.rs @@ -2532,6 +2532,7 @@ async fn review_diff_at( None } }; + let mut lockfile_platform_policy_allowed = false; let mut application = match adjudicated { Some(Ok(adjudicated)) => { add_usage(&mut usage, adjudicated.usage); @@ -2548,7 +2549,10 @@ async fn review_diff_at( &diff_receipt, receipt, ) { - Ok(application) => application, + Ok(application) => { + lockfile_platform_policy_allowed = true; + application + } Err(error) => { adjudication_incomplete = true; review_trust = filter::ReviewTrust::Failed; @@ -2599,6 +2603,13 @@ async fn review_diff_at( &mut application, fresh_candidate_count, ); + if lockfile_platform_policy_allowed { + suppress_fresh_lockfile_platform_claims( + &mut application, + fresh_candidate_count, + &index, + ); + } for (candidate_index, finding) in application .kept_indices .iter() @@ -3525,6 +3536,190 @@ fn suppress_fresh_unresolved_repository_claims( .retain(|index| application.kept_indices.contains(index)); } +fn suppress_fresh_lockfile_platform_claims( + application: &mut crate::adjudication::AdjudicationApplication, + fresh_candidate_count: usize, + index: &diff::DiffIndex, +) { + let mut kept = Vec::with_capacity(application.kept.len()); + let mut kept_indices = Vec::with_capacity(application.kept_indices.len()); + for (candidate_index, finding) in application + .kept_indices + .drain(..) + .zip(application.kept.drain(..)) + { + if candidate_index < fresh_candidate_count + && is_unverifiable_lockfile_platform_claim(index, &finding) + { + application.suppressed.push(SuppressedFinding { + finding, + reason: SuppressionReason::LockfilePlatformEvidenceInsufficient, + }); + } else { + kept_indices.push(candidate_index); + kept.push(finding); + } + } + application.kept = kept; + application.kept_indices = kept_indices; + application + .unresolved_indices + .retain(|index| application.kept_indices.contains(index)); +} + +fn is_unverifiable_lockfile_platform_claim(index: &diff::DiffIndex, finding: &Finding) -> bool { + index.contains_exact_evidence(finding) + && is_compact_lockfile_metadata(finding) + && is_platform_support_claim(finding) +} + +fn is_compact_lockfile_metadata(finding: &Finding) -> bool { + if finding.path != crate::envelope::CHANGE_METADATA_PATH { + return false; + } + let Some(evidence) = finding.evidence.as_deref() else { + return false; + }; + let evidence = evidence.trim(); + if let Some(path) = evidence + .strip_prefix("- ") + .and_then(|path| path.strip_suffix(" [lockfile summary]")) + { + return diff::canonical_prompt_path(path) + .is_some_and(|path| diff::is_known_lockfile(&path)); + } + + const DELIMITER: &str = ": lockfile changed, "; + evidence.match_indices(DELIMITER).any(|(delimiter, _)| { + is_compact_lockfile_change_details( + &evidence[..delimiter], + &evidence[delimiter + DELIMITER.len()..], + ) + }) +} + +fn is_compact_lockfile_change_details(path: &str, details: &str) -> bool { + let Some((additions, details)) = details.split_once(" additions, ") else { + return false; + }; + let Some((deletions, changes)) = details.split_once(" deletions; ") else { + return false; + }; + additions.parse::().is_ok() + && deletions.parse::().is_ok() + && !changes.trim().is_empty() + && diff::canonical_prompt_path(path).is_some_and(|path| diff::is_known_lockfile(&path)) +} + +fn is_platform_support_claim(finding: &Finding) -> bool { + let prose = normalize_classifier_prose(&format!("{} {}", finding.title, finding.body)); + if [ + "security", + "vulnerab", + "cve", + "checksum", + "integrity", + "malware", + "install script", + "preinstall", + "postinstall", + "prepare script", + "lifecycle script", + ] + .iter() + .any(|term| prose.contains(term)) + { + return false; + } + + let has_platform_term = [ + "platform", + "operating system", + "architecture", + "arch", + "cpu", + "abi", + "runtime", + "ia32", + "i386", + "x86", + "amd64", + "x86_64", + "arm64", + "aarch64", + "armv5", + "armv6", + "armv7", + "armv8", + "armel", + "armhf", + "x64", + "ppc64le", + "loong64", + "mips64", + "ppc64", + "s390x", + "riscv", + "win32", + "windows", + "linux", + "darwin", + "macos", + "freebsd", + "android", + "musl", + "glibc", + ] + .iter() + .any(|term| prose_contains_term(&prose, term)); + if !has_platform_term { + return false; + } + + ["support", "compatib", "regress"] + .iter() + .any(|prefix| prose_contains_word_prefix(&prose, prefix)) + || ["break", "breaks", "breaking", "broken"] + .iter() + .any(|term| prose_contains_term(&prose, term)) + || ["works on", "runs on", "available on"] + .iter() + .any(|phrase| prose.contains(phrase)) +} + +fn prose_contains_term(prose: &str, term: &str) -> bool { + if term.contains(' ') { + prose.contains(term) + } else { + prose + .split(|character: char| !character.is_alphanumeric()) + .any(|word| word == term) + } +} + +fn prose_contains_word_prefix(prose: &str, prefix: &str) -> bool { + prose + .split(|character: char| !character.is_alphanumeric()) + .any(|word| word.starts_with(prefix)) +} + +fn normalize_classifier_prose(prose: &str) -> String { + let mut normalized = String::with_capacity(prose.len()); + for character in prose.chars() { + let compatible = if ('\u{ff01}'..='\u{ff5e}').contains(&character) { + char::from_u32(character as u32 - 0xfee0).unwrap_or(character) + } else { + character + }; + for lowercase in compatible.to_lowercase() { + if lowercase != '\u{0307}' { + normalized.push(lowercase); + } + } + } + normalized +} + fn error_envelope( cfg: &Config, err: &anyhow::Error, @@ -4552,6 +4747,109 @@ mod tests { ); } + #[test] + fn only_fresh_unverifiable_lockfile_platform_claims_are_suppressed() { + let evidence = + "pnpm-lock.yaml: lockfile changed, 1 additions, 1 deletions; optional packages changed"; + let mut index = diff::DiffIndex::default(); + index.add_change_metadata(1); + index.add_rendered_evidence(&format!( + "### {}\n1 + {evidence}\n", + crate::envelope::CHANGE_METADATA_PATH + )); + + let mut fresh_platform = finding_with_title( + crate::envelope::CHANGE_METADATA_PATH, + 1, + "Preserve Windows IA32 support", + "The lockfile change breaks Windows IA32 runtime support.", + ); + fresh_platform.evidence = Some(evidence.into()); + let mut direct_removal = finding_with_title( + crate::envelope::CHANGE_METADATA_PATH, + 1, + "The lockfile removes an optional dependency", + "The dependency resolution no longer contains the package.", + ); + direct_removal.evidence = Some(evidence.into()); + let mut security_finding = finding_with_title( + crate::envelope::CHANGE_METADATA_PATH, + 1, + "Windows IA32 checksum verification regresses", + "The lockfile change breaks checksum verification for this package.", + ); + security_finding.evidence = Some(evidence.into()); + let baseline_platform = fresh_platform.clone(); + let mut application = crate::adjudication::AdjudicationApplication { + kept: vec![ + fresh_platform.clone(), + direct_removal.clone(), + security_finding.clone(), + baseline_platform.clone(), + ], + kept_indices: vec![0, 1, 2, 3], + unresolved_indices: vec![0, 1, 2, 3], + resolved_indices: Vec::new(), + suppressed: Vec::new(), + }; + + suppress_fresh_lockfile_platform_claims(&mut application, 3, &index); + + assert_eq!(application.kept_indices, vec![1, 2, 3]); + assert_eq!(application.unresolved_indices, vec![1, 2, 3]); + assert_eq!(application.kept.len(), 3); + assert!(same_visible_finding(&application.kept[0], &direct_removal)); + assert!(same_visible_finding( + &application.kept[1], + &security_finding + )); + assert!(same_visible_finding( + &application.kept[2], + &baseline_platform + )); + assert_eq!(application.suppressed.len(), 1); + assert!(same_visible_finding( + &application.suppressed[0].finding, + &fresh_platform + )); + assert_eq!( + application.suppressed[0].reason, + SuppressionReason::LockfilePlatformEvidenceInsufficient + ); + + let breakpoint = finding_with_title( + crate::envelope::CHANGE_METADATA_PATH, + 1, + "Keep the Linux breakpoint package", + "The lockfile removes the Linux breakpoint package required by diagnostics.", + ); + assert!(!is_platform_support_claim(&breakpoint)); + + for body in [ + "The lockfile removes the armv8 package and breaks ARMv8 support.", + "The lockfile removes arm64_support and breaks arm64_support builds.", + "The lockfile change breaks Ｗindows support.", + "The lockfile change breaks WİNDOWS support.", + ] { + assert!(is_platform_support_claim(&finding( + crate::envelope::CHANGE_METADATA_PATH, + 1, + body, + ))); + } + + let mut quoted_path = finding( + crate::envelope::CHANGE_METADATA_PATH, + 1, + "The lockfile change breaks Windows support.", + ); + quoted_path.evidence = Some( + "\"deps/a: lockfile changed, x/pnpm-lock.yaml\": lockfile changed, 1 additions, 1 deletions; optional packages changed" + .into(), + ); + assert!(is_compact_lockfile_metadata("ed_path)); + } + #[test] fn adjudication_preserved_baseline_survives_a_deterministic_refutation_receipt() { let snapshot_id = "a".repeat(40); diff --git a/tests/e2e.rs b/tests/e2e.rs index 51b9472..f5055fa 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -5958,6 +5958,183 @@ async fn malformed_known_lockfile_uses_bounded_source_review() { assert!(body.contains(r#"source = \"new-four\""#)); } +#[tokio::test] +async fn compact_pnpm_lockfile_ia32_platform_claim_is_suppressed_but_dependency_removal_survives() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with(|request: &Request| { + let request_body: Value = request.body_json().unwrap(); + let system = request_body["messages"][0]["content"] + .as_str() + .unwrap_or_default(); + if system.contains("single finding adjudicator") { + let user = request_body["messages"][1]["content"].as_str().unwrap(); + let adjudication: Value = serde_json::from_str(user).unwrap(); + let results = adjudication["candidates"] + .as_array() + .unwrap() + .iter() + .map(|candidate| { + json!({ + "candidateId": candidate["candidateId"], + "status": "confirmed", + "revisedTitle": candidate["title"], + "revisedBody": candidate["body"], + "evidence": candidate["citedEvidence"], + "duplicateOf": null + }) + }) + .collect::>(); + return ResponseTemplate::new(200) + .set_body_json(scorer_text(&Value::Array(results).to_string())); + } + let evidence = prompt_evidence( + request, + ".postil/change-metadata", + 1, + "pnpm-lock.yaml: lockfile changed", + ); + ResponseTemplate::new(200).set_body_json(llm_content(json!([ + { + "path": ".postil/change-metadata", + "line": 1, + "severity": "error", + "kind": "risk", + "confidence": 0.99, + "title": "Preserve Windows IA32 support", + "body": "The lockfile change removes the IA32 package and breaks Windows IA32 runtime support; preserve the package so 32-bit Windows remains supported.", + "evidence": evidence + }, + { + "path": ".postil/change-metadata", + "line": 1, + "severity": "error", + "kind": "risk", + "confidence": 0.98, + "title": "The lockfile removes the IA32 optional dependency", + "body": "The lockfile removes `@rollup/rollup-win32-ia32-msvc@4.34.8`, so the dependency resolution no longer contains that package.", + "evidence": evidence + } + ]))) + }) + .with_priority(1) + .mount(&server) + .await; + + let dir = tempfile::tempdir().unwrap(); + let diff = dir.path().join("pnpm-ia32-regression.diff"); + std::fs::write( + &diff, + "diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml\n--- a/pnpm-lock.yaml\n+++ b/pnpm-lock.yaml\n@@ -1,3 +1,3 @@\n lockfileVersion: '9.0'\n packages:\n- '@rollup/rollup-win32-ia32-msvc@4.34.8':\n+ '@rollup/rollup-win32-x64-msvc@4.34.8':\n", + ) + .unwrap(); + + let out = postil() + .current_dir(dir.path()) + .env("POSTIL_API_BASE", server.uri()) + .env("POSTIL_DISABLE_SCORER", "1") + .args(["review", "--diff-file"]) + .arg(&diff) + .args(["--output", "json"]) + .assert() + .code(1); + let envelope: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); + assert_eq!(envelope["findings"].as_array().unwrap().len(), 1); + assert_eq!( + envelope["findings"][0]["title"], + "The lockfile removes the IA32 optional dependency" + ); + assert_eq!(envelope["suppressedFindings"].as_array().unwrap().len(), 1); + assert_eq!( + envelope["suppressedFindings"][0]["reason"], + "lockfilePlatformEvidenceInsufficient" + ); + assert_eq!( + envelope["suppressedFindings"][0]["finding"]["title"], + "Preserve Windows IA32 support" + ); + assert_eq!(envelope["gate"]["failing"], true); +} + +#[tokio::test] +async fn compact_lockfile_platform_claim_survives_failed_adjudication() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .and(body_string_contains("single finding adjudicator")) + .respond_with(ResponseTemplate::new(503)) + .with_priority(1) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with(|request: &Request| { + let evidence = prompt_evidence( + request, + ".postil/change-metadata", + 1, + "pnpm-lock.yaml: lockfile changed", + ); + ResponseTemplate::new(200).set_body_json(llm_content(json!([{ + "path": ".postil/change-metadata", + "line": 1, + "severity": "error", + "kind": "risk", + "confidence": 0.99, + "title": "Preserve Windows IA32 support", + "body": "The lockfile change removes the IA32 package and breaks Windows IA32 runtime support.", + "evidence": evidence + }]))) + }) + .with_priority(10) + .mount(&server) + .await; + + let dir = tempfile::tempdir().unwrap(); + let diff = dir.path().join("pnpm-ia32-adjudication-failure.diff"); + std::fs::write( + &diff, + "diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml\n--- a/pnpm-lock.yaml\n+++ b/pnpm-lock.yaml\n@@ -1,3 +1,3 @@\n lockfileVersion: '9.0'\n packages:\n- '@rollup/rollup-win32-ia32-msvc@4.34.8':\n+ '@rollup/rollup-win32-x64-msvc@4.34.8':\n", + ) + .unwrap(); + + let out = postil() + .current_dir(dir.path()) + .env("POSTIL_API_BASE", server.uri()) + .env("POSTIL_DISABLE_SCORER", "1") + .args(["review", "--diff-file"]) + .arg(&diff) + .args(["--output", "json"]) + .assert() + .code(1); + let envelope: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); + assert!( + envelope["findings"] + .as_array() + .unwrap() + .iter() + .any(|finding| { finding["title"] == "Preserve Windows IA32 support" }) + ); + assert!( + envelope["findings"] + .as_array() + .unwrap() + .iter() + .any(|finding| { finding["path"] == ".postil/provider" }) + ); + assert!( + envelope["suppressedFindings"] + .as_array() + .is_none_or(|findings| { + !findings + .iter() + .any(|finding| finding["reason"] == "lockfilePlatformEvidenceInsufficient") + }) + ); + assert_eq!(envelope["gate"]["failing"], true); +} + async fn mock_review_model(server: &MockServer, model: &str, findings: Value) { Mock::given(method("POST")) .and(path("/chat/completions")) From e2a2ae200087e5117eb1492ba4baad5eb0e62a74 Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Tue, 25 Aug 2026 18:59:10 +0000 Subject: [PATCH 2/7] Require three complete release benchmark samples --- .github/workflows/release.yml | 112 ++-- Cargo.lock | 2 +- Cargo.toml | 2 +- bench/README.md | 135 +++-- bench/baseline.json | 4 +- bench/src/compare-baseline.test.ts | 752 ++++++++++++++++------- bench/src/compare-baseline.ts | 932 +++++++++++++++++++++++++---- bench/src/live.ts | 6 +- bench/src/livemodels.test.ts | 32 +- 9 files changed, 1529 insertions(+), 448 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c30be0a..7b2beec 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -3,15 +3,7 @@ name: Release on: push: tags: ["v*"] - workflow_dispatch: - inputs: - bench_live_override_reason: - description: >- - Publish even when the live benchmark misses its floor, recording this - reason on the run. Leave empty to require the benchmark to pass. The - baseline is never rewritten; an accepted miss stays visible as a miss. - required: false - default: "" + workflow_dispatch: {} jobs: validate-tag: @@ -52,7 +44,7 @@ jobs: bench-live: needs: validate-tag runs-on: ubuntu-latest - timeout-minutes: 90 + timeout-minutes: 270 permissions: contents: read steps: @@ -63,9 +55,8 @@ jobs: with: bun-version: 1.3.14 - # A present-but-rejected key fails all 70 cases about three minutes later - # with nothing that names the credential. Probing it costs one request and - # turns that into an immediate, accurate failure. + # A present-but-rejected key fails every benchmark case without naming the + # credential. This bounded probe makes that failure immediate and explicit. - name: Require a working OpenRouter credential env: COMPLETION_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} @@ -93,7 +84,7 @@ jobs: if jq -e '.data.limit_remaining != null and .data.limit_remaining < 1' \ /tmp/openrouter-key.json >/dev/null; then remaining="$(jq -r '.data.limit_remaining' /tmp/openrouter-key.json)" - echo "::error::OPENROUTER_API_KEY has ${remaining} credit remaining, below the cost of one benchmark run." + echo "::error::OPENROUTER_API_KEY has ${remaining} credit remaining, below the release benchmark reserve." exit 1 fi remaining="$(jq -r '.data.limit_remaining // "unlimited"' /tmp/openrouter-key.json)" @@ -137,65 +128,80 @@ jobs: if-no-files-found: warn retention-days: 30 - - name: Run diff-file live benchmark + - name: Run diff-file live benchmark sample 1 + id: live-sample-1 + continue-on-error: true + working-directory: bench + env: + OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} + POSTIL_BIN: ${{ github.workspace }}/target/release/postil + run: >- + bun run bench:live -- --screen-profile ../provisional-models.json + --run-id "release-${{ github.ref_name }}-sample-1" + --json-out "${{ runner.temp }}/bench-live-report-1.json" + + - name: Run diff-file live benchmark sample 2 + id: live-sample-2 + continue-on-error: true + working-directory: bench + env: + OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} + POSTIL_BIN: ${{ github.workspace }}/target/release/postil + run: >- + bun run bench:live -- --screen-profile ../provisional-models.json + --run-id "release-${{ github.ref_name }}-sample-2" + --json-out "${{ runner.temp }}/bench-live-report-2.json" + + - name: Run diff-file live benchmark sample 3 + id: live-sample-3 + continue-on-error: true working-directory: bench env: OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} + POSTIL_BIN: ${{ github.workspace }}/target/release/postil run: >- bun run bench:live -- --screen-profile ../provisional-models.json - --run-id "release-${{ github.ref_name }}" - --json-out "${{ runner.temp }}/bench-live-report.json" + --run-id "release-${{ github.ref_name }}-sample-3" + --json-out "${{ runner.temp }}/bench-live-report-3.json" - - name: Upload the diff-file live report + - name: Upload the diff-file live reports if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: bench-live-report + name: bench-live-reports path: | - ${{ runner.temp }}/bench-live-report.json - ${{ runner.temp }}/bench-live-report.json.partial + ${{ runner.temp }}/bench-live-report-1.json + ${{ runner.temp }}/bench-live-report-2.json + ${{ runner.temp }}/bench-live-report-3.json if-no-files-found: warn retention-days: 30 - name: Compare against the recorded baseline - id: compare-baseline - # Failure is decided by the next step so an operator-supplied reason can - # accept a miss. Recording the miss and rewriting the baseline are - # different acts, and only the first one happens here. - continue-on-error: true + if: always() working-directory: bench - run: bun run bench:compare -- --result "${{ runner.temp }}/bench-live-report.json" - - - name: Enforce the benchmark floor env: - OVERRIDE_REASON: ${{ inputs.bench_live_override_reason }} - COMPARISON: ${{ steps.compare-baseline.outcome }} + SAMPLE_1_OUTCOME: ${{ steps.live-sample-1.outcome }} + SAMPLE_2_OUTCOME: ${{ steps.live-sample-2.outcome }} + SAMPLE_3_OUTCOME: ${{ steps.live-sample-3.outcome }} run: | set -euo pipefail - if [[ "${COMPARISON}" == "success" ]]; then - echo "The live benchmark met its floor." - exit 0 - fi - if [[ -z "${OVERRIDE_REASON}" ]]; then - echo "::error::Release blocked by a benchmark regression. See the 'Compare against the recorded baseline' step above for the failing metric(s)." - { - echo "## Release blocked" - echo - echo "The live benchmark regressed past tolerance against \`bench/baseline.json\`." - echo "See the 'Compare against the recorded baseline' step log for the failing metric(s)." - } >> "$GITHUB_STEP_SUMMARY" + comparison=0 + bun run bench:compare -- \ + --binary "${{ github.workspace }}/target/release/postil" \ + --screen-profile ../provisional-models.json \ + --expected-run-id "release-${{ github.ref_name }}-sample-1" \ + --expected-run-id "release-${{ github.ref_name }}-sample-2" \ + --expected-run-id "release-${{ github.ref_name }}-sample-3" \ + --result "${{ runner.temp }}/bench-live-report-1.json" \ + --result "${{ runner.temp }}/bench-live-report-2.json" \ + --result "${{ runner.temp }}/bench-live-report-3.json" || comparison=$? + if [[ "${SAMPLE_1_OUTCOME}" != "success" || \ + "${SAMPLE_2_OUTCOME}" != "success" || \ + "${SAMPLE_3_OUTCOME}" != "success" ]]; then + echo "::error::At least one live benchmark sample failed before comparison." exit 1 fi - echo "::warning title=Benchmark floor missed and accepted::${OVERRIDE_REASON}" - { - echo "## Benchmark floor missed and accepted" - echo - echo "The live benchmark missed its floor and this release was published anyway." - echo - echo "**Reason:** ${OVERRIDE_REASON}" - echo - echo "\`bench/baseline.json\` is unchanged, so the next release faces the same floor." - } >> "$GITHUB_STEP_SUMMARY" + exit "${comparison}" build: needs: [validate-tag, bench-live] diff --git a/Cargo.lock b/Cargo.lock index 0a96cc4..104efc4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1114,7 +1114,7 @@ checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" [[package]] name = "postil-cli" -version = "0.9.2" +version = "0.9.3" dependencies = [ "aho-corasick", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index d801d6e..4f124a0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "postil-cli" -version = "0.9.2" +version = "0.9.3" 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 e41afa5..6725102 100644 --- a/bench/README.md +++ b/bench/README.md @@ -376,9 +376,9 @@ exercise the scorer and must retain its exact identity and usage record. It refuses to run without `POSTIL_API_KEY`, `OPENROUTER_API_KEY`, `MODEL_API_KEY`, or `LLM_API_KEY` and never logs or prints the key value. Live mode spends real tokens and depends on an external provider, so it is **not run on ordinary -pushes or pull requests**; the release pipeline runs a full-corpus live pass -against `bench/baseline.json` before every tagged release (see "Release gate" -below). Every live run writes its JSON report under +pushes or pull requests**. The release pipeline runs three sequential +full-corpus live samples against `bench/baseline.json` before every tagged +release (see "Release gate" below). Every live run writes its JSON report under `.runs/live//` (gitignored), beside raw per-attempt stdout and stderr. Set `--run-id ` or `POSTIL_BENCH_SCREEN_RUN_ID` to name the immutable namespace; an omitted ID gets a unique generated value. Reusing an ID fails @@ -458,62 +458,85 @@ nondeterministic. Treat them as internal evidence, not a published benchmark. ## Release gate -The `Release` workflow runs a full-corpus diff-file live pass against the Luna -profile in `provisional-models.json` and the exact release binary before it -builds any other target, then checks the result with `bun run bench:compare`. -A material regression blocks the release: `build` depends on the `bench-live` -job. - -`compare-baseline.ts` computes five metrics from the live report and compares -each against the matching model entry in `bench/baseline.json`: authored-target -detection rate, false/unrelated finding count, gate-verdict correctness (does -the CLI's exit code agree with the authored must-block/advisory/clean -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`). - -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 -thresholds of 2 findings and 2 points. Every request goes out at temperature 0, -so this is the provider's own nondeterminism rather than sampling, and pinning -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 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 -random is worse than no gate, because it teaches everyone to bypass it. - -The baseline -also records the fixture-corpus and evaluator-source SHA-256 digests the live -report already computes; a mismatch fails loudly instead of comparing metrics -across an unrelated fixture set. +The `Release` workflow runs three uniquely named, sequential full-corpus +diff-file samples against the Luna profile in `provisional-models.json`. Every +sample uses the same preserved release binary. All three samples are attempted +even when one fails, and the workflow uploads every completed raw report before +comparison. A failed sample, missing report, or failed comparison blocks the +release because `build` depends on the `bench-live` job. + +The comparator accepts exactly one or three distinct `--result` paths. Three +paths must also contain byte-distinct reports with distinct immutable run IDs +and timestamps. Byte identity is established by SHA-256 over each raw file. +Every report must be exhaustive full-corpus evidence with an empty +`selectedCaseIds`, an enforced provider contract, no operational errors, and +all cases scored. Summary and per-result cost accounting must be complete. +Every cost is a canonical nonnegative decimal, and the summary cost must equal +the exact sum of result costs. Binary, fixture-corpus, evaluator, screening +profile, and provider-contract hashes are exact 64-character lowercase SHA-256 +values. Provider identity, route, API, and scorer configuration fields must be +present and internally consistent. The comparator requires explicit release +binary and screening-profile paths, then recomputes the binary, fixture corpus, +evaluator source, screening profile, provider contract, and exact case cohort +instead of trusting hashes asserted by the reports. + +A three-report comparison also requires identical binary, corpus, evaluator, +model, provider, API, scorer, route, profile, contract, timeout, fixture +identity, and case-count fields across the cohort. Structural, operational, +digest, and cohort failures block before metric comparison. The one-report +mode applies the same fail-closed report validation. + +`compare-baseline.ts` compares five aggregate metrics against the matching +model entry in `bench/baseline.json`: median authored-target detection rate, +median false/unrelated finding count, median gate-verdict correctness, maximum +per-run mean provider cost per case, and median per-run nearest-rank p95 review +latency. Each metric has its own tolerance in the exported `*_MAX_*` constants. +Detection rate and p95 latency block when they cross their tolerances. Maximum +mean cost always blocks and is compared as an exact decimal ratio; a provider +profile mismatch invalidates the comparison instead of making cost +informational. False/unrelated findings and gate-verdict correctness remain +informational; their medians and complete observed ranges appear in the table. +The CLI's per-operation cost cap remains the deterministic spending boundary. + +The baseline records fixture-corpus and evaluator-source SHA-256 digests, the +expected complete case count, exact provider profile, and the maximum sampled +run cost as a canonical decimal with its case count. A mismatch blocks +comparison across unrelated or incomplete evidence. ```sh -# Compare an existing report against the committed baseline. -bun run bench:compare -- --result -bun run bench:compare -- --run-id # resolves .runs/live//report.json +# Compare one complete report for a local check. +bun run bench:compare -- \ + --binary \ + --screen-profile ../provisional-models.json \ + --expected-run-id \ + --result -# Re-baseline deliberately, after confirming the new numbers are an accepted -# tradeoff (a model change, a fixture change, an intentional pipeline change): -REVIEW_MODEL=openai/gpt-5.6-luna bun run bench:live -- \ +# Run the release comparison over three complete reports. +bun run bench:compare -- \ + --binary \ + --screen-profile ../provisional-models.json \ + --expected-run-id \ + --expected-run-id \ + --expected-run-id \ + --result \ + --result \ + --result + +# Re-baseline explicitly from three independent complete reports with the same +# binary, profile, and corpus. +bun run bench:compare -- \ + --binary \ --screen-profile ../provisional-models.json \ - --json-out live-report.json -bun run bench:compare -- --result live-report.json --record + --expected-run-id \ + --expected-run-id \ + --expected-run-id \ + --result \ + --result \ + --result \ + --record ``` -`--record` is the only thing that writes `bench/baseline.json`; nothing else -updates it implicitly. When the gate fails in CI, the printed table shows -baseline vs. observed vs. verdict for every metric, so the failing metric and -by how much is visible directly in the job log without downloading the report -artifact. +`--record` accepts exactly three reports and is the only operation that writes +`bench/baseline.json`. The release workflow never records a baseline. The +comparison table shows baseline, aggregate observation, verdict, and sample +range directly in the job log. diff --git a/bench/baseline.json b/bench/baseline.json index 8d9824f..1ccc426 100644 --- a/bench/baseline.json +++ b/bench/baseline.json @@ -2,7 +2,7 @@ "schemaVersion": 1, "corpus": { "fixtureCorpusSha256": "8e4c2cb9ad5a7efdfe6a875566d20133e905155b6f693a873595adf6c069e065", - "evaluatorSha256": "02f4ad84bd86d1f65aafc7dcc077981972a95fdd0f5ab5140f5ea98ba9372797" + "evaluatorSha256": "1d6f9fecda01e6e8066f8fefd4a1b2955014575ce07542819b6f506d48dae595" }, "profiles": { "z-ai/glm-5.2": { @@ -38,6 +38,8 @@ "falsePositives": 0, "gateVerdictCorrectness": 0.8428571428571429, "meanCostUsdPerCase": 0.0008315965428571429, + "maximumRunCostUsdDecimal": "0.058211758", + "costCaseCount": 70, "latencyMs": { "p50": 5659, "p95": 9530 diff --git a/bench/src/compare-baseline.test.ts b/bench/src/compare-baseline.test.ts index da55adf..f1630bb 100644 --- a/bench/src/compare-baseline.test.ts +++ b/bench/src/compare-baseline.test.ts @@ -5,19 +5,36 @@ import { resolve } from "node:path"; import { cases } from "../fixtures/cases"; import { DETECTION_RATE_MAX_DROP_PP, - FALSE_FINDINGS_MAX_INCREASE, - GATE_VERDICT_MAX_DROP_PP, - LATENCY_P95_MAX_INCREASE_RATIO, - MEAN_COST_MAX_INCREASE_RATIO, + aggregateObservedMetrics, + assertDistinctRunIdentities, + assertDistinctRawReportDigests, + assertExpectedRunIdentities, + assertReportsBoundToInputs, + assertValidReleaseReport, compareMetrics, + exactMeanCostWithinTolerance, extractObservedMetrics, formatComparisonTable, + formatRebaselineGuidance, + median, + parseCliArguments, percentile, type BaselineProfile, type LiveReportForComparison, } from "./compare-baseline"; import { benchmarkCase } from "./harness"; -import { evaluatorSourceSha256 } from "./live"; +import { + ADMISSION_API_BASE, + evaluatorSourceSha256, + screeningProfileMetadata, +} from "./live"; +import { + formatCanonicalDecimal, + parseCanonicalDecimal, + providerContractSha256, + sumCanonicalDecimals, + type ProviderContractEvidence, +} from "./livemodels-score"; test("committed baseline authority matches the current benchmark sources", async () => { const baseline = JSON.parse( @@ -33,268 +50,583 @@ test("committed baseline authority matches the current benchmark sources", async expect(baseline.corpus.evaluatorSha256).toBe(await evaluatorSourceSha256()); }); -function fakeReport(overrides: { - falsePositives?: number; - observedProviderCostUsdDecimal?: string; - results?: LiveReportForComparison["results"]; +const PROVIDER_CONTRACT: ProviderContractEvidence = { + version: 1, + benchmarkProviderIdentity: "openrouter:managed-routing", + upstreamProviderIdentity: "Azure", + upstreamProviderRoute: "azure/eu", + dataCollection: "deny", + zeroDataRetention: true, + allowFallbacks: false, + generatorRequireParameters: false, + scorerRequireParameters: true, + maxPricePinned: true, + maxPriceUnits: "USD per million tokens", + modelPriceBounds: [{ + model: "openai/gpt-5.6-luna", + roles: ["generator", "scorer"], + inputMicrosPerMillionTokens: 400_000, + outputMicrosPerMillionTokens: 1_600_000, + }], +}; + +const HASHES = { + binary: "1".repeat(64), + corpus: "2".repeat(64), + evaluator: "3".repeat(64), + profile: "4".repeat(64), + contract: providerContractSha256(PROVIDER_CONTRACT), +} as const; + +interface FakeReportOptions { 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 }, - { id: "must-block-2", type: "defect", scored: true, truthSeverity: "error", durationMs: 2000, exitCode: 1 }, - { id: "advisory-1", type: "defect", scored: true, truthSeverity: "warn", durationMs: 1500, exitCode: 0 }, - { id: "clean-1", type: "clean", scored: true, truthSeverity: null, durationMs: 500, exitCode: 0 }, - ]; - const defectCases = results.filter((r) => r.type === "defect").length; + falsePositives?: number; + costPerCase?: string; + durationMultiplier?: number; + ranAt?: string; + runId?: string; +} + +let fakeRunSequence = 0; + +function fakeReport(options: FakeReportOptions = {}): LiveReportForComparison { + fakeRunSequence += 1; + const detected = options.detected ?? 54; + const falsePositives = options.falsePositives ?? 0; + const costPerCase = options.costPerCase ?? "0.001"; + const durationMultiplier = options.durationMultiplier ?? 100; + const results: LiveReportForComparison["results"] = Array.from({ length: 70 }, (_, index) => { + const defect = index < 57; + const truthSeverity = defect ? (index < 47 ? "error" : "warn") : null; + const caseDetected = defect ? index < detected : null; + return { + id: `${defect ? "defect" : "clean"}-${String(index + 1).padStart(2, "0")}`, + type: defect ? "defect" : "clean", + scored: true, + detected: caseDetected, + truthSeverity, + falsePositives: index === 0 ? falsePositives : 0, + durationMs: (index + 1) * durationMultiplier, + observedProviderCostUsdDecimal: costPerCase, + costAccountingComplete: true, + exitCode: truthSeverity === "error" && caseDetected === true ? 1 : 0, + }; + }); + const observedProviderCostUsdDecimal = formatCanonicalDecimal(sumCanonicalDecimals( + results.map(() => parseCanonicalDecimal(costPerCase)), + )); + return { summary: { - model: "z-ai/glm-5.2", + runId: options.runId ?? `fixture-run-${fakeRunSequence}`, + model: "openai/gpt-5.6-luna", + binarySha256: HASHES.binary, + providerIdentity: "openrouter:managed-routing", + apiBase: ADMISSION_API_BASE, + apiFormat: "openai-compatible", + scorerMode: "disabled", + scorerModel: null, 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, - scoredCases: results.filter((r) => r.scored).length, - defectCases, - detected: overrides.detected ?? defectCases, - falsePositives: overrides.falsePositives ?? 0, - observedProviderCostUsdDecimal: overrides.observedProviderCostUsdDecimal ?? "0.004", - ranAt: "2026-01-01T00:00:00.000Z", + evidenceScope: "full-corpus", + selectedCaseIds: [], + providerContractEnforced: true, + screeningProfileSha256: HASHES.profile, + upstreamProviderIdentity: "Azure", + upstreamProviderRoute: "azure/eu", + providerContractSha256: HASHES.contract, + providerContract: structuredClone(PROVIDER_CONTRACT), + fixtureCorpusSha256: HASHES.corpus, + evaluatorSha256: HASHES.evaluator, + timeoutOverrides: { + requestSeconds: null, + totalSeconds: null, + caseProcessMilliseconds: 180_000, + }, + totalCases: 70, + scoredCases: 70, + defectCases: 57, + cleanCases: 13, + detected, + falsePositives, + detectionRate: `${detected}/57`, + observedProviderCostUsdDecimal, + costAccountingComplete: true, + errors: 0, + ranAt: options.ranAt ?? new Date(Date.UTC(2026, 7, 25, 0, 0, fakeRunSequence)).toISOString(), }, results, }; } +function cloneReport(report: LiveReportForComparison): LiveReportForComparison { + return structuredClone(report); +} + +async function inputBoundReport(): Promise { + const report = fakeReport(); + const parsedCases = cases.map((input) => benchmarkCase.parse(input)); + let detected = 0; + report.results = parsedCases.map((fixture, index) => { + const finding = fixture.groundTruth.findings[0]; + const defect = finding !== undefined; + const caseDetected = defect && detected < 54; + if (caseDetected) detected += 1; + return { + id: fixture.id, + type: defect ? "defect" : "clean", + scored: true, + detected: defect ? caseDetected : null, + truthSeverity: finding?.severity ?? null, + falsePositives: 0, + durationMs: (index + 1) * 100, + observedProviderCostUsdDecimal: "0.001", + costAccountingComplete: true, + exitCode: caseDetected && finding?.severity === "error" ? 1 : 0, + }; + }); + const screeningProfilePath = resolve(import.meta.dir, "..", "..", "provisional-models.json"); + const [binary, evaluatorSha256, profile] = await Promise.all([ + readFile(process.execPath), + evaluatorSourceSha256(), + screeningProfileMetadata(screeningProfilePath), + ]); + Object.assign(report.summary, { + binarySha256: createHash("sha256").update(binary).digest("hex"), + fixtureCorpusSha256: createHash("sha256") + .update(JSON.stringify(parsedCases)) + .digest("hex"), + evaluatorSha256, + screeningProfileSha256: profile.sha256, + upstreamProviderIdentity: profile.upstreamProviderIdentity, + upstreamProviderRoute: profile.upstreamProviderRoute, + providerContractSha256: profile.providerContractSha256, + providerContract: profile.providerContract, + totalCases: parsedCases.length, + scoredCases: parsedCases.length, + defectCases: parsedCases.filter((fixture) => fixture.groundTruth.findings.length > 0).length, + cleanCases: parsedCases.filter((fixture) => fixture.groundTruth.findings.length === 0).length, + detected, + falsePositives: 0, + detectionRate: `${detected}/${parsedCases.filter((fixture) => fixture.groundTruth.findings.length > 0).length}`, + observedProviderCostUsdDecimal: formatCanonicalDecimal(sumCanonicalDecimals( + parsedCases.map(() => parseCanonicalDecimal("0.001")), + )), + errors: 0, + }); + return report; +} + const populatedBaseline: Extract = { populated: true, - generatedAt: "2026-01-01T00:00:00.000Z", + generatedAt: "2026-08-25T00:00:00.000Z", reviewMode: "exhaustive", - sourceRunAt: "2026-01-01T00:00:00.000Z", - providerContractEnforced: false, - screeningProfileSha256: null, - upstreamProviderIdentity: null, + sourceRunAt: "2026-08-25T00:00:00.000Z", + providerContractEnforced: true, + screeningProfileSha256: HASHES.profile, + upstreamProviderIdentity: "Azure", totalCases: 70, scoredCases: 70, - detectionRate: 0.95, - falsePositives: 2, - gateVerdictCorrectness: 0.97, - meanCostUsdPerCase: 0.003, - latencyMs: { p50: 4000, p95: 9000 }, + detectionRate: 54 / 57, + falsePositives: 0, + gateVerdictCorrectness: 1, + meanCostUsdPerCase: 0.001, + maximumRunCostUsdDecimal: "0.07", + costCaseCount: 70, + latencyMs: { p50: 3500, p95: 6700 }, }; -describe("percentile", () => { - test("nearest-rank over a sorted sample", () => { - const sorted = [10, 20, 30, 40, 50]; - expect(percentile(sorted, 50)).toBe(30); - expect(percentile(sorted, 95)).toBe(50); - expect(percentile(sorted, 1)).toBe(10); +describe("sample math", () => { + test("uses nearest-rank percentiles", () => { + expect(percentile([10, 20, 30, 40, 50], 50)).toBe(30); + expect(percentile([10, 20, 30, 40, 50], 95)).toBe(50); + expect(percentile([42], 95)).toBe(42); + expect(() => percentile([], 50)).toThrow("empty sample"); }); - test("single-element sample returns that element at any percentile", () => { - expect(percentile([42], 50)).toBe(42); - expect(percentile([42], 95)).toBe(42); + test("computes a median without depending on input order", () => { + expect(median([52, 54, 53])).toBe(53); + expect(median([54, 52, 53])).toBe(53); + expect(() => median([])).toThrow("empty sample"); }); - test("rejects an empty sample", () => { - expect(() => percentile([], 50)).toThrow("empty sample"); + test("uses the median per-run nearest-rank p95 and maximum per-run mean cost", () => { + const observed = aggregateObservedMetrics([ + fakeReport({ durationMultiplier: 1, costPerCase: "0.001" }), + fakeReport({ durationMultiplier: 100, costPerCase: "0.003" }), + fakeReport({ durationMultiplier: 10, costPerCase: "0.002" }), + ]); + + expect(observed.latencyMs.p95).toBe(670); + expect(observed.ranges.p95LatencyMs).toEqual({ min: 67, max: 6700 }); + expect(observed.meanCostUsdPerCase).toBeCloseTo(0.003, 10); + expect(observed.ranges.meanCostUsdPerCase.min).toBeCloseTo(0.001, 10); + expect(observed.ranges.meanCostUsdPerCase.max).toBeCloseTo(0.003, 10); + expect( + compareMetrics(populatedBaseline, observed).rows.find( + (row) => row.metric === "maximum mean cost per case", + )?.verdict, + ).toBe("FAIL"); }); }); -describe("extractObservedMetrics", () => { - test("derives detection rate, gate-verdict correctness, cost, and latency from a live report", () => { - const metrics = extractObservedMetrics(fakeReport()); - // 2 defects, 2 detected. - expect(metrics.detectionRate).toBeCloseTo(1, 5); - // All four scored cases: two must-block exit 1 (correct), one advisory - // exit 0 (correct), one clean exit 0 (correct) => 4/4 correct. - expect(metrics.gateVerdictCorrectness).toBeCloseTo(1, 5); - expect(metrics.meanCostUsdPerCase).toBeCloseTo(0.004 / 4, 6); - expect(metrics.latencyMs.p50).toBeGreaterThan(0); - expect(metrics.latencyMs.p95).toBe(2000); +describe("release report validation", () => { + test("accepts one complete full-corpus report", () => { + expect(() => assertValidReleaseReport(fakeReport())).not.toThrow(); + expect(extractObservedMetrics(fakeReport()).reportCount).toBe(1); }); - test("scores a wrongly-blocked advisory case as an incorrect gate verdict", () => { - const report = fakeReport({ - results: [ - { id: "must-block-1", type: "defect", scored: true, truthSeverity: "error", durationMs: 1000, exitCode: 1 }, - // An advisory (warn) case that the model over-blocked: exit 1 when it - // should have exited 0. - { id: "advisory-1", type: "defect", scored: true, truthSeverity: "warn", durationMs: 1500, exitCode: 1 }, - ], - }); - const metrics = extractObservedMetrics(report); - expect(metrics.gateVerdictCorrectness).toBeCloseTo(0.5, 5); + test("rejects 69 of 70 scored cases", () => { + const report = cloneReport(fakeReport()); + report.summary.scoredCases = 69; + report.results[69]!.scored = false; + report.results[69]!.durationMs = null; + expect(() => assertValidReleaseReport(report)).toThrow("scoredCases must equal totalCases"); }); - test("excludes unscored (errored) cases from gate-verdict and latency scoring", () => { - const report = fakeReport({ - results: [ - { id: "must-block-1", type: "defect", scored: true, truthSeverity: "error", durationMs: 1000, exitCode: 1 }, - { id: "errored-1", type: "defect", scored: false, truthSeverity: "error", durationMs: null, exitCode: undefined }, - ], - }); - const metrics = extractObservedMetrics(report); - expect(metrics.gateVerdictCorrectness).toBeCloseTo(1, 5); - expect(metrics.latencyMs.p50).toBe(1000); + test("rejects any operational error", () => { + const report = cloneReport(fakeReport()); + report.summary.errors = 1; + expect(() => assertValidReleaseReport(report)).toThrow("errors must be 0"); + }); + + test("rejects a missing scored-case exit code", () => { + const report = cloneReport(fakeReport()); + (report.results[12] as { exitCode?: number }).exitCode = undefined; + expect(() => assertValidReleaseReport(report)).toThrow( + "every scored result must have CLI exit code 0 or 1", + ); }); - test("names an all-unscored run an operational failure, not a regression", () => { - // A release blocked because the model got worse and one blocked because the - // run never reached the model need different responses from whoever reads - // the log, so the message has to say which happened. - const report = fakeReport({ - results: [ - { id: "errored-1", type: "defect", scored: false, truthSeverity: "error", durationMs: null, exitCode: undefined }, - { id: "errored-2", type: "defect", scored: false, truthSeverity: "error", durationMs: null, exitCode: undefined }, - ], - }); - expect(() => extractObservedMetrics(report)).toThrow( - "scored none of its 2 cases", + test("rejects incomplete summary or per-result cost accounting", () => { + const incompleteSummary = cloneReport(fakeReport()); + incompleteSummary.summary.costAccountingComplete = false; + expect(() => assertValidReleaseReport(incompleteSummary)).toThrow( + "summary cost accounting must be complete", ); - expect(() => extractObservedMetrics(report)).toThrow( - "operational failure rather than a quality regression", + + const incompleteResult = cloneReport(fakeReport()); + incompleteResult.results[12]!.costAccountingComplete = false; + expect(() => assertValidReleaseReport(incompleteResult)).toThrow( + "every result must have complete cost accounting", + ); + + const missingResultCost = cloneReport(fakeReport()); + missingResultCost.results[12]!.observedProviderCostUsdDecimal = null; + expect(() => assertValidReleaseReport(missingResultCost)).toThrow( + "every result must have canonical observed provider cost", ); }); -}); -describe("compareMetrics", () => { - test("passes when observed metrics match baseline exactly", () => { - const baseline = { ...populatedBaseline, detectionRate: 1, gateVerdictCorrectness: 1 }; - const observed = extractObservedMetrics(fakeReport({ - falsePositives: baseline.falsePositives, - observedProviderCostUsdDecimal: String(baseline.meanCostUsdPerCase * 4), - })); - const comparison = compareMetrics(baseline, observed); - expect(comparison.ok).toBe(true); - expect(comparison.rows.every((row) => row.verdict === "PASS")).toBe(true); + test("rejects noncanonical costs and a summary that does not equal the result total", () => { + const noncanonical = cloneReport(fakeReport()); + noncanonical.results[0]!.observedProviderCostUsdDecimal = "0.0010"; + expect(() => assertValidReleaseReport(noncanonical)).toThrow("canonical nonnegative decimal"); + + const inconsistent = cloneReport(fakeReport()); + inconsistent.summary.observedProviderCostUsdDecimal = "0.071"; + expect(() => assertValidReleaseReport(inconsistent)).toThrow("does not match result total"); }); - test("fails when detection rate drops more than the tolerance", () => { - const baseline = { ...populatedBaseline, detectionRate: 0.98 }; - const observed = extractObservedMetrics(fakeReport({ - detected: 1, - results: [ - { id: "must-block-1", type: "defect", scored: true, truthSeverity: "error", durationMs: 1000, exitCode: 1 }, - { id: "must-block-2", type: "defect", scored: true, truthSeverity: "error", durationMs: 1000, exitCode: 0 }, - ], - })); - expect(observed.detectionRate).toBeCloseTo(0.5, 5); - expect(0.98 - observed.detectionRate).toBeGreaterThan(DETECTION_RATE_MAX_DROP_PP / 100); - const comparison = compareMetrics(baseline, observed); - expect(comparison.ok).toBe(false); - expect(comparison.rows.find((row) => row.metric === "detection rate")?.verdict).toBe("FAIL"); + test("rejects non-release scope, provider contract, hash, and identity fields", () => { + const invalidReports: Array<[string, (report: LiveReportForComparison) => void]> = [ + ["reviewMode must be exhaustive", (report) => { report.summary.reviewMode = "bounded"; }], + ["evidenceScope must be full-corpus", (report) => { report.summary.evidenceScope = "selected-cases"; }], + ["selectedCaseIds must be empty", (report) => { report.summary.selectedCaseIds = ["defect-01"]; }], + ["providerContractEnforced must be true", (report) => { report.summary.providerContractEnforced = false; }], + ["binarySha256 must be exactly 64", (report) => { report.summary.binarySha256 = "A".repeat(64); }], + ["fixtureCorpusSha256 must be exactly 64", (report) => { report.summary.fixtureCorpusSha256 = "short"; }], + ["evaluatorSha256 must be exactly 64", (report) => { report.summary.evaluatorSha256 = "short"; }], + ["screeningProfileSha256 must be exactly 64", (report) => { report.summary.screeningProfileSha256 = "short"; }], + ["providerContractSha256 must be exactly 64", (report) => { report.summary.providerContractSha256 = "short"; }], + ["providerIdentity must be nonempty", (report) => { + (report.summary as { providerIdentity: string }).providerIdentity = ""; + }], + ["upstreamProviderIdentity must be nonempty", (report) => { report.summary.upstreamProviderIdentity = ""; }], + ["upstreamProviderRoute must be nonempty", (report) => { report.summary.upstreamProviderRoute = " "; }], + ["apiBase must be nonempty", (report) => { + (report.summary as { apiBase: string }).apiBase = ""; + }], + ["apiFormat must be nonempty", (report) => { + (report.summary as { apiFormat: string }).apiFormat = ""; + }], + ]; + + for (const [message, mutate] of invalidReports) { + const report = cloneReport(fakeReport()); + mutate(report); + expect(() => assertValidReleaseReport(report)).toThrow(message); + } }); - test("tolerates a detection-rate drop within the percentage-point budget", () => { - // Baseline 69/70, observed 68/70: a ~1.4pp drop, inside the 2pp budget. - const baseline = { ...populatedBaseline, detectionRate: 69 / 70 }; - const observed = extractObservedMetrics(fakeReport()); // detectionRate 1.0 here; force via override below - const nearlyEqual = { ...observed, detectionRate: 68 / 70 }; - const comparison = compareMetrics(baseline, nearlyEqual); - expect(comparison.rows.find((row) => row.metric === "detection rate")?.verdict).toBe("PASS"); + test("binds provider identity, route, models, and digest to the exact contract", () => { + const identityMismatch = cloneReport(fakeReport()); + (identityMismatch.summary as { providerIdentity: string }).providerIdentity = "custom"; + expect(() => assertValidReleaseReport(identityMismatch)).toThrow( + "providerIdentity must match the enforced provider contract", + ); + + const routeMismatch = cloneReport(fakeReport()); + routeMismatch.summary.upstreamProviderRoute = "azure/us"; + expect(() => assertValidReleaseReport(routeMismatch)).toThrow( + "upstreamProviderRoute must match the enforced provider contract", + ); + + const contractMismatch = cloneReport(fakeReport()); + contractMismatch.summary.providerContract.modelPriceBounds[0]!.outputMicrosPerMillionTokens += 1; + expect(() => assertValidReleaseReport(contractMismatch)).toThrow( + "providerContractSha256 must match the enforced provider contract", + ); + + const modelMismatch = cloneReport(fakeReport()); + modelMismatch.summary.model = "openai/other"; + expect(() => assertValidReleaseReport(modelMismatch)).toThrow( + "review model must have a generator price bound", + ); }); - test("reports false/unrelated findings past the budget without blocking", () => { - // Six runs of one unchanged binary spanned 4 to 7 false findings against a - // budget of baseline + 2, so this threshold marks a run worth reading, not - // a release worth stopping. - const observed = extractObservedMetrics(fakeReport({ - falsePositives: populatedBaseline.falsePositives + FALSE_FINDINGS_MAX_INCREASE + 1, - })); - const comparison = compareMetrics(populatedBaseline, observed); - const row = comparison.rows.find((r) => r.metric === "false/unrelated findings"); - expect(row?.verdict).toBe("FAIL"); - expect(row?.informational).toBe(true); - expect(comparison.ok).toBe(true); + test("binds reports to the supplied binary, corpus, evaluator, and screening profile", async () => { + const report = await inputBoundReport(); + const screeningProfilePath = resolve(import.meta.dir, "..", "..", "provisional-models.json"); + await expect( + assertReportsBoundToInputs([report], process.execPath, screeningProfilePath), + ).resolves.toBeUndefined(); + + report.summary.binarySha256 = "f".repeat(64); + await expect( + assertReportsBoundToInputs([report], process.execPath, screeningProfilePath), + ).rejects.toThrow("binarySha256 is not bound to the supplied release input"); }); - test("says so in the table when a reported metric leaves its usual range", () => { - const observed = extractObservedMetrics(fakeReport({ - falsePositives: populatedBaseline.falsePositives + FALSE_FINDINGS_MAX_INCREASE + 1, - })); - const table = formatComparisonTable(compareMetrics(populatedBaseline, observed).rows); - expect(table).toContain("Outside its usual range, but not blocking"); - expect(table).toContain("false/unrelated findings"); + test("rejects a non-release API even when its fields are nonempty", () => { + const report = cloneReport(fakeReport()); + (report.summary as { apiBase: string }).apiBase = "https://not-openrouter.invalid/v1"; + (report.summary as { apiFormat: string }).apiFormat = "arbitrary"; + expect(() => assertValidReleaseReport(report)).toThrow( + "provider API must be the managed OpenRouter release endpoint", + ); }); +}); - test("a detection-rate drop still blocks the release", () => { - // The one metric whose spread across those runs (3.5pp) stayed inside a - // threshold that can still catch a real regression. - const observed = { - ...extractObservedMetrics(fakeReport()), - detectionRate: populatedBaseline.detectionRate - (DETECTION_RATE_MAX_DROP_PP / 100) - 0.01, - }; +describe("three-report aggregation", () => { + test("52/54/53 passes the detection floor", () => { + const observed = aggregateObservedMetrics([ + fakeReport({ detected: 52 }), + fakeReport({ detected: 54 }), + fakeReport({ detected: 53 }), + ]); + expect(observed.detectionRate).toBe(53 / 57); + expect(populatedBaseline.detectionRate - observed.detectionRate).toBeLessThan( + DETECTION_RATE_MAX_DROP_PP / 100, + ); + const comparison = compareMetrics(populatedBaseline, observed); + expect(comparison.rows.find((row) => row.metric === "median detection rate")?.verdict).toBe("PASS"); + expect(comparison.ok).toBe(true); + }); + + test("52/54/52 fails the detection floor", () => { + const observed = aggregateObservedMetrics([ + fakeReport({ detected: 52 }), + fakeReport({ detected: 54 }), + fakeReport({ detected: 52 }), + ]); + expect(observed.detectionRate).toBe(52 / 57); const comparison = compareMetrics(populatedBaseline, observed); - expect(comparison.rows.find((r) => r.metric === "detection rate")?.verdict).toBe("FAIL"); + expect(comparison.rows.find((row) => row.metric === "median detection rate")?.verdict).toBe("FAIL"); expect(comparison.ok).toBe(false); }); - test("reports a gate-verdict drop past the tolerance without blocking", () => { - const baseline = { ...populatedBaseline, gateVerdictCorrectness: 1 }; - const observed = { ...extractObservedMetrics(fakeReport()), gateVerdictCorrectness: 1 - (GATE_VERDICT_MAX_DROP_PP / 100) - 0.01 }; - const comparison = compareMetrics(baseline, observed); - const row = comparison.rows.find((r) => r.metric === "gate verdict correctness"); - expect(row?.verdict).toBe("FAIL"); - expect(row?.informational).toBe(true); - expect(comparison.ok).toBe(true); + test("is independent of report order", () => { + const reports = [ + fakeReport({ detected: 52, falsePositives: 3, ranAt: "2026-08-25T00:00:01.000Z" }), + fakeReport({ detected: 54, falsePositives: 1, ranAt: "2026-08-25T00:00:03.000Z" }), + fakeReport({ detected: 53, falsePositives: 2, ranAt: "2026-08-25T00:00:02.000Z" }), + ] as const; + expect(aggregateObservedMetrics(reports)).toEqual( + aggregateObservedMetrics([reports[2], reports[0], reports[1]]), + ); + }); + + test("rejects a cohort mismatch", () => { + const mismatched = cloneReport(fakeReport()); + mismatched.summary.binarySha256 = "6".repeat(64); + expect(() => aggregateObservedMetrics([fakeReport(), mismatched, fakeReport()])).toThrow( + "cohort mismatch for summary.binarySha256", + ); + }); + + test("rejects repeated logical run identities even when raw files differ", () => { + const first = fakeReport(); + const reformatted = cloneReport(first); + const rewritten = cloneReport(first); + expect(() => assertDistinctRunIdentities([first, reformatted, rewritten])).toThrow( + "distinct benchmark run IDs", + ); + expect(() => assertExpectedRunIdentities([first], ["different-run"])).toThrow( + "does not match its expected run identity", + ); + expect(() => aggregateObservedMetrics([first, reformatted, rewritten])).toThrow( + "distinct benchmark run IDs", + ); }); - 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 }; + test("keeps false findings and gate correctness informational with the observed range", () => { + const observed = aggregateObservedMetrics([ + fakeReport({ falsePositives: 3 }), + fakeReport({ falsePositives: 5 }), + fakeReport({ falsePositives: 4 }), + ]); const comparison = compareMetrics(populatedBaseline, observed); - const row = comparison.rows.find((candidate) => candidate.metric === "mean cost per case"); - expect(row?.verdict).toBe("FAIL"); - expect(row?.informational).toBe(true); + const falseFindingRow = comparison.rows.find( + (row) => row.metric === "median false/unrelated findings", + ); + const gateRow = comparison.rows.find( + (row) => row.metric === "median gate verdict correctness", + ); + expect(falseFindingRow?.verdict).toBe("FAIL"); + expect(falseFindingRow?.informational).toBe(true); + expect(falseFindingRow?.detail).toContain("range 3-5"); + expect(gateRow?.informational).toBe(true); + expect(gateRow?.detail).toContain("range"); expect(comparison.ok).toBe(true); + expect(formatComparisonTable(comparison.rows)).toContain( + "Outside its usual range, but not blocking", + ); }); - 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); + test("cost remains blocking when the provider profile differs", () => { + const observed = aggregateObservedMetrics([ + fakeReport({ costPerCase: "1" }), + fakeReport({ costPerCase: "1" }), + fakeReport({ costPerCase: "1" }), + ]); + observed.screeningProfileSha256 = "f".repeat(64); + const comparison = compareMetrics(populatedBaseline, observed); + const cost = comparison.rows.find((row) => row.metric === "maximum mean cost per case"); + expect(cost?.verdict).toBe("FAIL"); + expect(cost?.informational).not.toBe(true); 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("compares the blocking cost ceiling without IEEE-754 rounding", () => { + expect(exactMeanCostWithinTolerance("8.75", 70, "7", 70)).toBe(true); + expect(exactMeanCostWithinTolerance("8.750000000000000007", 70, "7", 70)).toBe(false); }); +}); - test("fails when p95 latency rises past the ratio budget", () => { - const observed = { - ...extractObservedMetrics(fakeReport()), - latencyMs: { p50: 1000, p95: populatedBaseline.latencyMs.p95 * (1 + LATENCY_P95_MAX_INCREASE_RATIO) + 1 }, - }; - const comparison = compareMetrics(populatedBaseline, observed); - expect(comparison.rows.find((row) => row.metric === "p95 latency (ms)")?.verdict).toBe("FAIL"); +describe("CLI report selection", () => { + const releaseInputs = [ + "--binary", "target/release/postil", + "--screen-profile", "provisional-models.json", + ]; + + test("accepts exactly one or three distinct --result paths", () => { + expect(parseCliArguments([ + ...releaseInputs, + "--expected-run-id", "one", + "--result", "one.json", + ]).resultPaths).toEqual(["one.json"]); + expect(parseCliArguments([ + ...releaseInputs, + "--expected-run-id", "one", + "--expected-run-id", "two", + "--expected-run-id", "three", + "--result", "one.json", + "--result", "two.json", + "--result", "three.json", + ]).resultPaths).toEqual(["one.json", "two.json", "three.json"]); + }); + + test("rejects every other report count", () => { + expect(() => parseCliArguments([])).toThrow("exactly 1 or 3"); + expect(() => parseCliArguments(["--result", "one", "--result", "two"])).toThrow( + "exactly 1 or 3", + ); + expect(() => parseCliArguments([ + "--result", "one", + "--result", "two", + "--result", "three", + "--result", "four", + ])).toThrow("exactly 1 or 3"); + }); + + test("rejects duplicate paths and duplicate raw reports", () => { + expect(() => parseCliArguments([ + ...releaseInputs, + "--expected-run-id", "one", + "--expected-run-id", "two", + "--expected-run-id", "three", + "--result", "one.json", + "--result", "./one.json", + "--result", "three.json", + ])).toThrow("path must be distinct"); + expect(() => assertDistinctRawReportDigests([ + "a".repeat(64), + "b".repeat(64), + "a".repeat(64), + ])).toThrow("distinct raw SHA-256 digests"); + }); + + test("requires the same three-sample estimator for rebaselining", () => { + expect(() => parseCliArguments([ + ...releaseInputs, + "--expected-run-id", "one", + "--result", "one.json", + "--record", + ])).toThrow("--record requires exactly three --result reports"); + expect(parseCliArguments([ + ...releaseInputs, + "--expected-run-id", "one", + "--expected-run-id", "two", + "--expected-run-id", "three", + "--result", "one.json", + "--result", "two.json", + "--result", "three.json", + "--record", + ]).record).toBe(true); + }); + + test("requires explicit release binary and screening profile inputs", () => { + expect(() => parseCliArguments([ + "--expected-run-id", "one", + "--result", "one.json", + ])).toThrow( + "requires --binary", + ); + expect(() => parseCliArguments([ + "--binary", "postil", + "--expected-run-id", "one", + "--result", "one.json", + ])).toThrow("requires --screen-profile"); + expect(() => parseCliArguments([ + ...releaseInputs, + "--result", "one.json", + ])).toThrow("one --expected-run-id per --result report"); + }); + + test("prints an executable three-report rebaseline command only for a complete cohort", () => { + expect(formatRebaselineGuidance({ + binaryPath: "target/release/postil", + screeningProfilePath: "provisional models.json", + expectedRunIds: ["one", "two", "three"], + resultPaths: ["one.json", "two report.json", "three.json"], + })).toBe([ + " bun run bench:compare -- \\", + " --binary 'target/release/postil' \\", + " --screen-profile 'provisional models.json' \\", + " --expected-run-id 'one' \\", + " --expected-run-id 'two' \\", + " --expected-run-id 'three' \\", + " --result 'one.json' \\", + " --result 'two report.json' \\", + " --result 'three.json' \\", + " --record", + ].join("\n")); + + expect(formatRebaselineGuidance({ + binaryPath: "target/release/postil", + screeningProfilePath: "provisional-models.json", + expectedRunIds: ["one"], + resultPaths: ["one.json"], + })).toContain("collect three independent complete reports"); }); }); diff --git a/bench/src/compare-baseline.ts b/bench/src/compare-baseline.ts index af9c4d7..fc0181a 100644 --- a/bench/src/compare-baseline.ts +++ b/bench/src/compare-baseline.ts @@ -1,27 +1,49 @@ #!/usr/bin/env bun // Release-gate regression check for the diff-file live benchmark (see live.ts). // -// Consumes a LiveReport JSON artifact (the file `bun run bench:live --json-out -// ` writes, or the report the runner already writes under -// `.runs/live//report.json`) and compares its metrics against the -// committed `bench/baseline.json`. Exits non-zero on a material regression, so -// the release pipeline can refuse to ship a CLI that reviews worse than the -// last recorded baseline. +// Consumes one or three LiveReport JSON artifacts written by `bun run +// bench:live --json-out ` and compares their metrics against the committed +// `bench/baseline.json`. Every report must be complete full-corpus evidence. A +// three-report comparison additionally requires one identical benchmark cohort +// and distinct raw artifacts. Exits non-zero on invalid evidence or a material +// regression, so the release pipeline can refuse to ship a CLI that reviews +// worse than the last recorded baseline. // // Compare mode (default): // -// bun run bench:compare -- --result -// bun run bench:compare -- --run-id # resolves the runner's own path +// bun run bench:compare -- --binary --screen-profile +// --expected-run-id --result +// bun run bench:compare -- --binary --screen-profile +// --expected-run-id --expected-run-id --expected-run-id +// --result --result --result // -// Record mode writes the observed metrics into baseline.json as the new -// baseline for the report's model. This is the deliberate re-baseline path: +// Record mode writes the three-sample aggregate into baseline.json as the new +// baseline for the reports' model. This is the deliberate re-baseline path: // nothing updates baseline.json except an explicit --record invocation. // -// bun run bench:compare -- --result --record +// bun run bench:compare -- --binary --screen-profile +// --expected-run-id --expected-run-id --expected-run-id +// --result --result --result --record +import { createHash } from "node:crypto"; import { readFile, writeFile } from "node:fs/promises"; import { resolve } from "node:path"; +import { isDeepStrictEqual } from "node:util"; import { z } from "zod"; +import { cases } from "../fixtures/cases"; +import { benchmarkCase } from "./harness"; +import { + ADMISSION_API_BASE, + evaluatorSourceSha256, + screeningProfileMetadata, +} from "./live"; +import { + compareCanonicalDecimals, + formatCanonicalDecimal, + parseCanonicalDecimal, + providerContractSha256, + sumCanonicalDecimals, +} from "./livemodels-score"; // --------------------------------------------------------------------------- // Regression tolerances. Each constant is the largest change from baseline @@ -46,7 +68,7 @@ export const FALSE_FINDINGS_MAX_INCREASE = 2; export const GATE_VERDICT_MAX_DROP_PP = 2; /** - * What one run of this benchmark can and cannot decide. + * Measured run-to-run spread for the informational metrics. * * Six runs of a single unchanged binary against this corpus, four on * OpenRouter managed routing and two pinned to the qualified upstream @@ -60,18 +82,11 @@ export const GATE_VERDICT_MAX_DROP_PP = 2; * is the provider's own nondeterminism, and pinning the upstream provider did * not remove it: the widest false-finding count came from a pinned run. * - * A blocking threshold has to sit outside that spread or it fails releases at - * random, and a gate that fails at random gets bypassed, which is worse than - * no gate. Detection rate is the one metric whose spread is narrow enough to - * gate on from a single run, and it is also the metric that most directly - * answers "did the reviewer find the defects". The other two are real signals - * and are still reported and worth reading across releases, but a single - * sample of either cannot separate a regression from the noise floor, so they - * do not block. - * - * Narrowing them means reducing the noise, not tightening the number: - * comparing a median across repeated runs would be the direct fix, at - * proportionally more cost and wall-clock per release. + * The release gate compares three-run medians for quality and latency, and the + * maximum per-run mean cost. False findings and gate-verdict correctness remain + * informational because this measured sample does not establish sufficiently + * narrow blocking thresholds for those metrics. Their median and complete + * observed range stay visible in the comparison output. */ export const MEASURED_RUN_TO_RUN_SPREAD = { runs: 6, @@ -80,46 +95,119 @@ export const MEASURED_RUN_TO_RUN_SPREAD = { gateVerdictPp: 12.9, } as const; -/** 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; +/** Maximum per-run mean provider cost per case may rise at most this fraction + * above a baseline recorded under the same enforced provider profile. */ +const MEAN_COST_CEILING_NUMERATOR = 5n; +const MEAN_COST_CEILING_DENOMINATOR = 4n; +export const MEAN_COST_MAX_INCREASE_RATIO = + Number(MEAN_COST_CEILING_NUMERATOR) / Number(MEAN_COST_CEILING_DENOMINATOR) - 1; -/** p95 review latency may rise at most this fraction above baseline. Wide - * because provider latency is the least controllable metric here. */ +/** Median per-run p95 review latency may rise at most this fraction above + * baseline. Wide because provider latency is the least controllable metric. */ export const LATENCY_P95_MAX_INCREASE_RATIO = 0.5; // --------------------------------------------------------------------------- -// Live report shape (the subset of live.ts's LiveReport this gate needs). -// Loosely validated: this script reads an artifact the same run produced -// moments earlier, not untrusted input, so it checks shape, not full schema. +// Live report shape and release-evidence validation. + +const sha256Schema = z.string().regex(/^[0-9a-f]{64}$/u); +const nonemptyStringSchema = z.string().trim().min(1); +const canonicalCostSchema = z.string().superRefine((value, context) => { + try { + if (formatCanonicalDecimal(parseCanonicalDecimal(value)) !== value || !Number.isFinite(Number(value))) { + context.addIssue({ code: "custom", message: "must be a finite canonical nonnegative decimal" }); + } + } catch (error) { + context.addIssue({ + code: "custom", + message: error instanceof Error ? error.message : "must be a canonical nonnegative decimal", + }); + } +}); + +const providerContractSchema = z.object({ + version: z.literal(1), + benchmarkProviderIdentity: z.literal("openrouter:managed-routing"), + upstreamProviderIdentity: nonemptyStringSchema, + upstreamProviderRoute: nonemptyStringSchema, + dataCollection: z.literal("deny"), + zeroDataRetention: z.literal(true), + allowFallbacks: z.literal(false), + generatorRequireParameters: z.literal(false), + scorerRequireParameters: z.literal(true), + maxPricePinned: z.literal(true), + maxPriceUnits: z.literal("USD per million tokens"), + modelPriceBounds: z.array(z.object({ + model: nonemptyStringSchema, + roles: z.array(z.enum(["generator", "scorer"])).min(1), + inputMicrosPerMillionTokens: z.number().int().safe().positive(), + outputMicrosPerMillionTokens: z.number().int().safe().positive(), + })).min(1), +}).superRefine((contract, context) => { + const models = contract.modelPriceBounds.map((bound) => bound.model); + if (new Set(models).size !== models.length) { + context.addIssue({ code: "custom", message: "provider contract model price bounds must be unique" }); + } + for (const [index, bound] of contract.modelPriceBounds.entries()) { + if (new Set(bound.roles).size !== bound.roles.length) { + context.addIssue({ + code: "custom", + path: ["modelPriceBounds", index, "roles"], + message: "provider contract roles must be unique", + }); + } + } +}); const liveCaseResultSchema = z.object({ - id: z.string(), + id: nonemptyStringSchema, type: z.enum(["defect", "clean"]), scored: z.boolean(), - truthSeverity: z.string().nullable(), - durationMs: z.number().nullable(), - exitCode: z.number().optional(), + detected: z.boolean().nullable(), + truthSeverity: z.enum(["info", "warn", "error"]).nullable(), + falsePositives: z.number().int().nonnegative(), + durationMs: z.number().finite().nonnegative().nullable(), + observedProviderCostUsdDecimal: canonicalCostSchema.nullable(), + costAccountingComplete: z.boolean(), + exitCode: z.union([z.literal(0), z.literal(1)]), }); const liveReportSchema = z.object({ summary: z.object({ - model: z.string(), + runId: nonemptyStringSchema, + model: nonemptyStringSchema, + binarySha256: sha256Schema, + providerIdentity: z.literal("openrouter:managed-routing"), + apiBase: z.literal(ADMISSION_API_BASE), + apiFormat: z.literal("openai-compatible"), + scorerMode: z.enum(["disabled", "enabled"]), + scorerModel: nonemptyStringSchema.nullable(), reviewMode: z.enum(["exhaustive", "bounded"]), + evidenceScope: z.enum(["full-corpus", "selected-cases"]), + selectedCaseIds: z.array(nonemptyStringSchema), providerContractEnforced: z.boolean(), - screeningProfileSha256: z.string().nullable(), - upstreamProviderIdentity: z.string().nullable(), - fixtureCorpusSha256: z.string(), - evaluatorSha256: z.string(), - totalCases: z.number().int().nonnegative(), + screeningProfileSha256: sha256Schema, + upstreamProviderIdentity: nonemptyStringSchema, + upstreamProviderRoute: nonemptyStringSchema, + providerContractSha256: sha256Schema, + providerContract: providerContractSchema, + fixtureCorpusSha256: sha256Schema, + evaluatorSha256: sha256Schema, + timeoutOverrides: z.object({ + requestSeconds: z.string().nullable(), + totalSeconds: z.string().nullable(), + caseProcessMilliseconds: z.number().int().positive(), + }), + totalCases: z.number().int().positive(), scoredCases: z.number().int().nonnegative(), - defectCases: z.number().int().nonnegative(), + defectCases: z.number().int().positive(), + cleanCases: z.number().int().nonnegative(), detected: z.number().int().nonnegative(), falsePositives: z.number().int().nonnegative(), - observedProviderCostUsdDecimal: z.string(), - ranAt: z.string(), + detectionRate: nonemptyStringSchema, + observedProviderCostUsdDecimal: canonicalCostSchema, + costAccountingComplete: z.boolean(), + errors: z.number().int().nonnegative(), + ranAt: nonemptyStringSchema, }), results: z.array(liveCaseResultSchema), }); @@ -140,14 +228,16 @@ const baselineProfileSchema = z.discriminatedUnion("populated", [ reviewMode: z.enum(["exhaustive", "bounded"]), sourceRunAt: z.string(), providerContractEnforced: z.boolean(), - screeningProfileSha256: z.string().nullable(), - upstreamProviderIdentity: z.string().nullable(), + screeningProfileSha256: sha256Schema.nullable(), + upstreamProviderIdentity: nonemptyStringSchema.nullable(), totalCases: z.number().int().positive(), scoredCases: z.number().int().positive(), detectionRate: z.number().min(0).max(1), falsePositives: z.number().int().nonnegative(), gateVerdictCorrectness: z.number().min(0).max(1), meanCostUsdPerCase: z.number().nonnegative(), + maximumRunCostUsdDecimal: canonicalCostSchema.optional(), + costCaseCount: z.number().int().positive().optional(), latencyMs: z.object({ p50: z.number().nonnegative(), p95: z.number().nonnegative(), @@ -158,8 +248,8 @@ const baselineProfileSchema = z.discriminatedUnion("populated", [ const baselineFileSchema = z.object({ schemaVersion: z.literal(1), corpus: z.object({ - fixtureCorpusSha256: z.string(), - evaluatorSha256: z.string(), + fixtureCorpusSha256: sha256Schema, + evaluatorSha256: sha256Schema, }), profiles: z.record(z.string(), baselineProfileSchema), }); @@ -171,6 +261,7 @@ export type BaselineProfile = z.infer; // Metric extraction from a live report. export interface ObservedMetrics { + reportCount: 1 | 3; model: string; reviewMode: "exhaustive" | "bounded"; providerContractEnforced: boolean; @@ -184,8 +275,23 @@ export interface ObservedMetrics { detectionRate: number; falsePositives: number; gateVerdictCorrectness: number; + maximumRunCostUsdDecimal: string; + costCaseCount: number; meanCostUsdPerCase: number; latencyMs: { p50: number; p95: number }; + ranges: { + detectionRate: MetricRange; + falsePositives: MetricRange; + gateVerdictCorrectness: MetricRange; + meanCostUsdPerCase: MetricRange; + p50LatencyMs: MetricRange; + p95LatencyMs: MetricRange; + }; +} + +export interface MetricRange { + min: number; + max: number; } /** True when a scored case's postil exit code should be 1: the authored @@ -203,35 +309,287 @@ export function percentile(sorted: readonly number[], p: number): number { return sorted[index]!; } -export function extractObservedMetrics(report: LiveReportForComparison): ObservedMetrics { +export function median(values: readonly number[]): number { + if (values.length === 0) throw new Error("median of an empty sample is undefined"); + const sorted = [...values].sort((a, b) => a - b); + const middle = Math.floor(sorted.length / 2); + return sorted.length % 2 === 1 + ? sorted[middle]! + : (sorted[middle - 1]! + sorted[middle]!) / 2; +} + +function metricRange(values: readonly number[]): MetricRange { + return { min: Math.min(...values), max: Math.max(...values) }; +} + +function invalidReport(message: string): never { + throw new Error(`invalid release benchmark report: ${message}`); +} + +export function assertValidReleaseReport(report: LiveReportForComparison): void { const s = report.summary; - const scoredResults = report.results.filter((r) => r.scored); - if (scoredResults.length === 0) { - // Distinguishable on purpose. A release blocked because the model got worse - // and a release blocked because the run never reached the model call for - // different responses, and the second must not be mistaken for the first. - throw new Error( - `live report scored none of its ${report.results.length} cases, so no metric ` + - "could be computed. This is an operational failure rather than a quality " + - "regression: every case failed before producing a valid envelope. Check " + - "the provider credential, the account's remaining credit, and the model's " + - "availability, then rerun the benchmark.", + const hashes = [ + ["binarySha256", s.binarySha256], + ["fixtureCorpusSha256", s.fixtureCorpusSha256], + ["evaluatorSha256", s.evaluatorSha256], + ["screeningProfileSha256", s.screeningProfileSha256], + ["providerContractSha256", s.providerContractSha256], + ] as const; + for (const [field, value] of hashes) { + if (typeof value !== "string" || !/^[0-9a-f]{64}$/u.test(value)) { + invalidReport(`${field} must be exactly 64 lowercase hexadecimal characters`); + } + } + const providerFields = [ + ["providerIdentity", s.providerIdentity], + ["apiBase", s.apiBase], + ["apiFormat", s.apiFormat], + ["upstreamProviderIdentity", s.upstreamProviderIdentity], + ["upstreamProviderRoute", s.upstreamProviderRoute], + ] as const; + for (const [field, value] of providerFields) { + if (typeof value !== "string" || value.trim().length === 0) { + invalidReport(`${field} must be nonempty`); + } + } + if (s.apiBase !== ADMISSION_API_BASE || s.apiFormat !== "openai-compatible") { + invalidReport("provider API must be the managed OpenRouter release endpoint"); + } + if (s.reviewMode !== "exhaustive") invalidReport("reviewMode must be exhaustive"); + if (s.evidenceScope !== "full-corpus") invalidReport("evidenceScope must be full-corpus"); + if (s.selectedCaseIds.length !== 0) invalidReport("selectedCaseIds must be empty"); + if (!s.providerContractEnforced) invalidReport("providerContractEnforced must be true"); + if (s.providerIdentity !== s.providerContract.benchmarkProviderIdentity) { + invalidReport("providerIdentity must match the enforced provider contract"); + } + if (s.upstreamProviderIdentity !== s.providerContract.upstreamProviderIdentity) { + invalidReport("upstreamProviderIdentity must match the enforced provider contract"); + } + if (s.upstreamProviderRoute !== s.providerContract.upstreamProviderRoute) { + invalidReport("upstreamProviderRoute must match the enforced provider contract"); + } + if (providerContractSha256(s.providerContract) !== s.providerContractSha256) { + invalidReport("providerContractSha256 must match the enforced provider contract"); + } + const generatorModels = new Set( + s.providerContract.modelPriceBounds + .filter((bound) => bound.roles.includes("generator")) + .map((bound) => bound.model), + ); + const scorerModels = new Set( + s.providerContract.modelPriceBounds + .filter((bound) => bound.roles.includes("scorer")) + .map((bound) => bound.model), + ); + if (!generatorModels.has(s.model)) { + invalidReport("review model must have a generator price bound in the enforced provider contract"); + } + if (s.scorerMode === "disabled" && s.scorerModel !== null) { + invalidReport("scorerModel must be null when scorerMode is disabled"); + } + if (s.scorerMode === "enabled" && s.scorerModel === null) { + invalidReport("scorerModel must be nonempty when scorerMode is enabled"); + } + if (s.scorerModel !== null && !scorerModels.has(s.scorerModel)) { + invalidReport("scorer model must have a scorer price bound in the enforced provider contract"); + } + if (s.errors !== 0) invalidReport(`errors must be 0, received ${s.errors}`); + if (s.totalCases !== report.results.length) { + invalidReport(`totalCases ${s.totalCases} does not match ${report.results.length} results`); + } + if (s.scoredCases !== s.totalCases) { + invalidReport(`scoredCases must equal totalCases, received ${s.scoredCases}/${s.totalCases}`); + } + if (report.results.some((result) => !result.scored)) { + invalidReport("every result must be scored"); + } + if (report.results.some((result) => result.exitCode !== 0 && result.exitCode !== 1)) { + invalidReport("every scored result must have CLI exit code 0 or 1"); + } + if (report.results.some((result) => result.durationMs === null)) { + invalidReport("every result must have a duration"); + } + if (!s.costAccountingComplete) invalidReport("summary cost accounting must be complete"); + if (report.results.some((result) => !result.costAccountingComplete)) { + invalidReport("every result must have complete cost accounting"); + } + if (report.results.some((result) => result.observedProviderCostUsdDecimal === null)) { + invalidReport("every result must have canonical observed provider cost"); + } + + const ids = report.results.map((result) => result.id); + if (new Set(ids).size !== ids.length) invalidReport("result IDs must be unique"); + + const defectResults = report.results.filter((result) => result.type === "defect"); + const cleanResults = report.results.filter((result) => result.type === "clean"); + if (defectResults.length !== s.defectCases || cleanResults.length !== s.cleanCases) { + invalidReport( + `case counts do not match results: summary ${s.defectCases} defect/${s.cleanCases} clean, ` + + `results ${defectResults.length} defect/${cleanResults.length} clean`, ); } + if (s.defectCases + s.cleanCases !== s.totalCases) { + invalidReport("defectCases plus cleanCases must equal totalCases"); + } + if (defectResults.some((result) => typeof result.detected !== "boolean")) { + invalidReport("every defect result must have a boolean detected value"); + } + if (defectResults.some((result) => result.truthSeverity === null)) { + invalidReport("every defect result must have an authored truth severity"); + } + if (cleanResults.some((result) => result.detected !== null)) { + invalidReport("every clean result must have a null detected value"); + } + if (cleanResults.some((result) => result.truthSeverity !== null)) { + invalidReport("every clean result must have a null truth severity"); + } + const detected = defectResults.filter((result) => result.detected === true).length; + if (detected !== s.detected) { + invalidReport(`detected count ${s.detected} does not match ${detected} detected results`); + } + if (s.detectionRate !== `${s.detected}/${s.defectCases}`) { + invalidReport(`detectionRate must equal ${s.detected}/${s.defectCases}`); + } + const falsePositives = report.results.reduce((sum, result) => sum + result.falsePositives, 0); + if (falsePositives !== s.falsePositives) { + invalidReport(`falsePositives ${s.falsePositives} does not match result total ${falsePositives}`); + } + + const resultCosts = report.results.map((result) => + parseCanonicalDecimal(result.observedProviderCostUsdDecimal!), + ); + const resultCostTotal = formatCanonicalDecimal(sumCanonicalDecimals(resultCosts)); + if (resultCostTotal !== s.observedProviderCostUsdDecimal) { + invalidReport( + `summary observed provider cost ${s.observedProviderCostUsdDecimal} does not match ` + + `result total ${resultCostTotal}`, + ); + } + if (!Number.isFinite(Number(s.observedProviderCostUsdDecimal))) { + invalidReport("observed provider cost is outside the supported numeric range"); + } +} + +const COHORT_SUMMARY_FIELDS = [ + "model", + "binarySha256", + "fixtureCorpusSha256", + "evaluatorSha256", + "reviewMode", + "providerIdentity", + "apiBase", + "apiFormat", + "scorerMode", + "scorerModel", + "evidenceScope", + "selectedCaseIds", + "providerContractEnforced", + "screeningProfileSha256", + "upstreamProviderIdentity", + "upstreamProviderRoute", + "providerContractSha256", + "providerContract", + "timeoutOverrides", + "totalCases", + "defectCases", + "cleanCases", + "scoredCases", +] as const satisfies readonly (keyof LiveReportForComparison["summary"])[]; + +function resultCohort(report: LiveReportForComparison) { + return report.results + .map(({ id, type, truthSeverity }) => ({ id, type, truthSeverity })) + .sort((left, right) => left.id.localeCompare(right.id)); +} + +export function assertMatchingCohort(reports: readonly LiveReportForComparison[]): void { + if (reports.length < 2) return; + const reference = reports[0]!; + for (let index = 1; index < reports.length; index += 1) { + const candidate = reports[index]!; + for (const field of COHORT_SUMMARY_FIELDS) { + if (!isDeepStrictEqual(reference.summary[field], candidate.summary[field])) { + throw new Error(`live report cohort mismatch for summary.${field} between reports 1 and ${index + 1}`); + } + } + if (!isDeepStrictEqual(resultCohort(reference), resultCohort(candidate))) { + throw new Error(`live report cohort mismatch for result identities between reports 1 and ${index + 1}`); + } + } +} + +export function assertDistinctRunIdentities( + reports: readonly LiveReportForComparison[], +): void { + if (reports.length < 2) return; + const runIds = reports.map((report) => report.summary.runId); + if (new Set(runIds).size !== runIds.length) { + throw new Error("three-report comparison requires distinct benchmark run IDs"); + } + const runTimes = reports.map((report) => report.summary.ranAt); + if (new Set(runTimes).size !== runTimes.length) { + throw new Error("three-report comparison requires distinct benchmark run timestamps"); + } +} - const gateCorrect = scoredResults.filter( - (r) => (r.exitCode === 1) === expectedGateFailing(r), +interface PerRunMetrics { + detectionRate: number; + falsePositives: number; + gateVerdictCorrectness: number; + totalCostUsdDecimal: string; + meanCostUsdPerCase: number; + p50LatencyMs: number; + p95LatencyMs: number; +} + +function extractPerRunMetrics(report: LiveReportForComparison): PerRunMetrics { + const s = report.summary; + const gateCorrect = report.results.filter( + (result) => (result.exitCode === 1) === expectedGateFailing(result), ).length; - const durations = scoredResults - .map((r) => r.durationMs) - .filter((v): v is number => v !== null) + const durations = report.results + .map((result) => result.durationMs!) .sort((a, b) => a - b); - if (durations.length === 0) { - throw new Error("live report has no case with a recorded duration; nothing to compare"); + + return { + detectionRate: s.detected / s.defectCases, + falsePositives: s.falsePositives, + gateVerdictCorrectness: gateCorrect / s.totalCases, + totalCostUsdDecimal: s.observedProviderCostUsdDecimal, + meanCostUsdPerCase: Number(s.observedProviderCostUsdDecimal) / s.totalCases, + p50LatencyMs: percentile(durations, 50), + p95LatencyMs: percentile(durations, 95), + }; +} + +export function aggregateObservedMetrics( + reports: readonly LiveReportForComparison[], +): ObservedMetrics { + if (reports.length !== 1 && reports.length !== 3) { + throw new Error(`comparison requires exactly 1 or 3 reports, received ${reports.length}`); } + reports.forEach(assertValidReleaseReport); + assertMatchingCohort(reports); + assertDistinctRunIdentities(reports); + + const perRun = reports.map(extractPerRunMetrics); + const detectionRates = perRun.map((run) => run.detectionRate); + const falsePositives = perRun.map((run) => run.falsePositives); + const gateCorrectness = perRun.map((run) => run.gateVerdictCorrectness); + const meanCosts = perRun.map((run) => run.meanCostUsdPerCase); + const p50Latencies = perRun.map((run) => run.p50LatencyMs); + const p95Latencies = perRun.map((run) => run.p95LatencyMs); + const maximumCostRun = perRun.reduce((maximum, candidate) => + compareCanonicalDecimals( + parseCanonicalDecimal(candidate.totalCostUsdDecimal), + parseCanonicalDecimal(maximum.totalCostUsdDecimal), + ) > 0 ? candidate : maximum); + const s = reports[0]!.summary; + const ranAt = reports.map((report) => report.summary.ranAt).sort()[Math.floor(reports.length / 2)]!; return { + reportCount: reports.length as 1 | 3, model: s.model, reviewMode: s.reviewMode, providerContractEnforced: s.providerContractEnforced, @@ -239,24 +597,34 @@ export function extractObservedMetrics(report: LiveReportForComparison): Observe upstreamProviderIdentity: s.upstreamProviderIdentity, fixtureCorpusSha256: s.fixtureCorpusSha256, evaluatorSha256: s.evaluatorSha256, - ranAt: s.ranAt, + ranAt, totalCases: s.totalCases, scoredCases: s.scoredCases, - detectionRate: s.detected / s.defectCases, - falsePositives: s.falsePositives, - gateVerdictCorrectness: gateCorrect / scoredResults.length, - // Mean cost across every case the run attempted, not only scored ones: - // a case that burned a provider call and then failed to parse still cost - // money, and a regression that starts producing unparseable output - // should not get a cost pass for it. - meanCostUsdPerCase: Number(s.observedProviderCostUsdDecimal) / s.totalCases, + detectionRate: median(detectionRates), + falsePositives: median(falsePositives), + gateVerdictCorrectness: median(gateCorrectness), + maximumRunCostUsdDecimal: maximumCostRun.totalCostUsdDecimal, + costCaseCount: s.totalCases, + meanCostUsdPerCase: maximumCostRun.meanCostUsdPerCase, latencyMs: { - p50: percentile(durations, 50), - p95: percentile(durations, 95), + p50: median(p50Latencies), + p95: median(p95Latencies), + }, + ranges: { + detectionRate: metricRange(detectionRates), + falsePositives: metricRange(falsePositives), + gateVerdictCorrectness: metricRange(gateCorrectness), + meanCostUsdPerCase: metricRange(meanCosts), + p50LatencyMs: metricRange(p50Latencies), + p95LatencyMs: metricRange(p95Latencies), }, }; } +export function extractObservedMetrics(report: LiveReportForComparison): ObservedMetrics { + return aggregateObservedMetrics([report]); +} + // --------------------------------------------------------------------------- // Comparison @@ -283,35 +651,69 @@ function usd(v: number): string { return `$${v.toFixed(6)}`; } +export function exactMeanCostWithinTolerance( + observedTotalCostUsdDecimal: string, + observedCaseCount: number, + baselineTotalCostUsdDecimal: string, + baselineCaseCount: number, +): boolean { + if (!Number.isSafeInteger(observedCaseCount) || observedCaseCount < 1) { + throw new Error("observed cost case count must be a positive safe integer"); + } + if (!Number.isSafeInteger(baselineCaseCount) || baselineCaseCount < 1) { + throw new Error("baseline cost case count must be a positive safe integer"); + } + const observed = parseCanonicalDecimal(observedTotalCostUsdDecimal); + const baseline = parseCanonicalDecimal(baselineTotalCostUsdDecimal); + const left = { + coefficient: + observed.coefficient * BigInt(baselineCaseCount) * MEAN_COST_CEILING_DENOMINATOR, + scale: observed.scale, + }; + const right = { + coefficient: + baseline.coefficient * BigInt(observedCaseCount) * MEAN_COST_CEILING_NUMERATOR, + scale: baseline.scale, + }; + return compareCanonicalDecimals(left, right) <= 0; +} + export function compareMetrics(baseline: Extract, observed: ObservedMetrics): ComparisonResult { const rows: MetricVerdict[] = []; + const sampleLabel = observed.reportCount === 1 ? "1 run" : `${observed.reportCount} runs`; const detectionFloor = baseline.detectionRate - DETECTION_RATE_MAX_DROP_PP / 100; rows.push({ - metric: "detection rate", + metric: "median detection rate", baseline: pct(baseline.detectionRate), observed: pct(observed.detectionRate), verdict: observed.detectionRate >= detectionFloor ? "PASS" : "FAIL", - detail: `floor ${pct(detectionFloor)} (baseline - ${DETECTION_RATE_MAX_DROP_PP}pp)`, + detail: + `floor ${pct(detectionFloor)} (baseline - ${DETECTION_RATE_MAX_DROP_PP}pp); ` + + `${sampleLabel} range ${pct(observed.ranges.detectionRate.min)}-${pct(observed.ranges.detectionRate.max)}`, }); const falsePositiveCeiling = baseline.falsePositives + FALSE_FINDINGS_MAX_INCREASE; rows.push({ - metric: "false/unrelated findings", + metric: "median false/unrelated findings", baseline: String(baseline.falsePositives), observed: String(observed.falsePositives), verdict: observed.falsePositives <= falsePositiveCeiling ? "PASS" : "FAIL", - detail: `watch above ${falsePositiveCeiling}; observed spread ${MEASURED_RUN_TO_RUN_SPREAD.falseFindingsCount} unchanged`, + detail: + `watch above ${falsePositiveCeiling}; ${sampleLabel} range ` + + `${observed.ranges.falsePositives.min}-${observed.ranges.falsePositives.max}`, informational: true, }); const gateFloor = baseline.gateVerdictCorrectness - GATE_VERDICT_MAX_DROP_PP / 100; rows.push({ - metric: "gate verdict correctness", + metric: "median gate verdict correctness", baseline: pct(baseline.gateVerdictCorrectness), observed: pct(observed.gateVerdictCorrectness), verdict: observed.gateVerdictCorrectness >= gateFloor ? "PASS" : "FAIL", - detail: `watch below ${pct(gateFloor)}; observed spread ${MEASURED_RUN_TO_RUN_SPREAD.gateVerdictPp}pp unchanged`, + detail: + `watch below ${pct(gateFloor)}; ${sampleLabel} range ` + + `${pct(observed.ranges.gateVerdictCorrectness.min)}-${pct(observed.ranges.gateVerdictCorrectness.max)}`, informational: true, }); @@ -321,35 +723,48 @@ export function compareMetrics(baseline: Extract row.metric).join(", ")}. ` + - "One run cannot separate these signals from provider nondeterminism. Worth reading across " + - "several releases, not acting on alone.", + "The observed sample range is informational and does not change the release verdict.", ); } return lines.join("\n"); @@ -393,37 +807,273 @@ export function formatComparisonTable(rows: readonly MetricVerdict[]): string { // --------------------------------------------------------------------------- // CLI -function flagValue(args: string[], flag: string): string | undefined { - const index = args.indexOf(flag); - const value = index === -1 ? undefined : args[index + 1]; - return value?.startsWith("--") === true ? undefined : value; +export interface CompareCliOptions { + baselinePath: string; + binaryPath: string; + screeningProfilePath: string; + expectedRunIds: string[]; + resultPaths: string[]; + record: boolean; } -async function readJson(path: string): Promise { - return JSON.parse(await readFile(path, "utf8")); +function requiredFlagValue(args: readonly string[], index: number, flag: string): string { + const value = args[index + 1]; + if (value === undefined || value.startsWith("--")) { + throw new Error(`${flag} requires a path`); + } + return value; } function defaultBaselinePath(): string { return resolve(import.meta.dir, "..", "baseline.json"); } -function liveRunnerReportPath(runId: string): string { - return resolve(import.meta.dir, "..", ".runs", "live", runId, "report.json"); +export function assertDistinctResultPaths(paths: readonly string[]): void { + const normalized = paths.map((path) => resolve(path)); + if (new Set(normalized).size !== normalized.length) { + throw new Error("every --result path must be distinct"); + } } -async function main() { - const args = process.argv.slice(2); - const record = args.includes("--record"); - const baselinePath = flagValue(args, "--baseline") ?? defaultBaselinePath(); - const resultFlag = flagValue(args, "--result"); - const runIdFlag = flagValue(args, "--run-id"); - if (resultFlag === undefined && runIdFlag === undefined) { - throw new Error("bench:compare needs either --result or --run-id "); +export function parseCliArguments(args: readonly string[]): CompareCliOptions { + const resultPaths: string[] = []; + const expectedRunIds: string[] = []; + let baselinePath = defaultBaselinePath(); + let binaryPath: string | undefined; + let screeningProfilePath: string | undefined; + let baselineSeen = false; + let record = false; + + for (let index = 0; index < args.length; index += 1) { + const argument = args[index]!; + if (argument === "--result") { + resultPaths.push(requiredFlagValue(args, index, argument)); + index += 1; + continue; + } + if (argument === "--baseline") { + if (baselineSeen) throw new Error("--baseline may be specified only once"); + baselinePath = requiredFlagValue(args, index, argument); + baselineSeen = true; + index += 1; + continue; + } + if (argument === "--binary") { + if (binaryPath !== undefined) throw new Error("--binary may be specified only once"); + binaryPath = requiredFlagValue(args, index, argument); + index += 1; + continue; + } + if (argument === "--screen-profile") { + if (screeningProfilePath !== undefined) { + throw new Error("--screen-profile may be specified only once"); + } + screeningProfilePath = requiredFlagValue(args, index, argument); + index += 1; + continue; + } + if (argument === "--expected-run-id") { + expectedRunIds.push(requiredFlagValue(args, index, argument)); + index += 1; + continue; + } + if (argument === "--record") { + if (record) throw new Error("--record may be specified only once"); + record = true; + continue; + } + throw new Error(`unknown bench:compare argument ${argument}`); } - const resultPath = resultFlag ?? liveRunnerReportPath(runIdFlag!); - const report = liveReportSchema.parse(await readJson(resultPath)); - const observed = extractObservedMetrics(report); + if (record && resultPaths.length !== 3) { + throw new Error(`--record requires exactly three --result reports, received ${resultPaths.length}`); + } + if (!record && resultPaths.length !== 1 && resultPaths.length !== 3) { + throw new Error(`bench:compare requires exactly 1 or 3 --result reports, received ${resultPaths.length}`); + } + if (binaryPath === undefined) throw new Error("bench:compare requires --binary"); + if (screeningProfilePath === undefined) { + throw new Error("bench:compare requires --screen-profile"); + } + if (expectedRunIds.length !== resultPaths.length) { + throw new Error( + `bench:compare requires one --expected-run-id per --result report, received ${expectedRunIds.length}/${resultPaths.length}`, + ); + } + if (new Set(expectedRunIds).size !== expectedRunIds.length) { + throw new Error("every --expected-run-id must be distinct"); + } + assertDistinctResultPaths(resultPaths); + return { + baselinePath, + binaryPath, + screeningProfilePath, + expectedRunIds, + resultPaths, + record, + }; +} + +export function assertDistinctRawReportDigests(digests: readonly string[]): void { + if (new Set(digests).size !== digests.length) { + throw new Error("three-report comparison requires distinct raw SHA-256 digests"); + } +} + +export function assertExpectedRunIdentities( + reports: readonly LiveReportForComparison[], + expectedRunIds: readonly string[], +): void { + if (reports.length !== expectedRunIds.length) { + throw new Error("release report and expected run identity counts differ"); + } + for (const [index, report] of reports.entries()) { + if (report.summary.runId !== expectedRunIds[index]) { + throw new Error( + `release benchmark report ${index + 1} runId does not match its expected run identity`, + ); + } + } +} + +async function loadReports(paths: readonly string[]): Promise { + const rawReports = await Promise.all(paths.map(async (path) => { + const raw = await readFile(path).catch((error) => { + throw new Error(`could not read live report at ${path}: ${error instanceof Error ? error.message : String(error)}`); + }); + return { + path, + raw, + sha256: createHash("sha256").update(raw).digest("hex"), + }; + })); + if (rawReports.length === 3) { + assertDistinctRawReportDigests(rawReports.map((report) => report.sha256)); + } + + return rawReports.map(({ path, raw }) => { + let parsed: unknown; + try { + parsed = JSON.parse(raw.toString("utf8")); + } catch (error) { + throw new Error(`could not parse live report at ${path}: ${error instanceof Error ? error.message : String(error)}`); + } + const report = liveReportSchema.safeParse(parsed); + if (!report.success) { + throw new Error(`invalid release benchmark report at ${path}: ${report.error.message}`); + } + try { + assertValidReleaseReport(report.data); + } catch (error) { + throw new Error(`${path}: ${error instanceof Error ? error.message : String(error)}`); + } + return report.data; + }); +} + +export async function assertReportsBoundToInputs( + reports: readonly LiveReportForComparison[], + binaryPath: string, + screeningProfilePath: string, +): Promise { + const parsedCases = cases.map((input) => benchmarkCase.parse(input)); + const expectedResultCohort = parsedCases + .map((fixture) => { + const finding = fixture.groundTruth.findings[0]; + return { + id: fixture.id, + type: finding === undefined ? "clean" as const : "defect" as const, + truthSeverity: finding?.severity ?? null, + }; + }) + .sort((left, right) => left.id.localeCompare(right.id)); + const [binary, evaluatorSha256, profile] = await Promise.all([ + readFile(binaryPath).catch((error) => { + throw new Error( + `could not read release binary at ${binaryPath}: ${error instanceof Error ? error.message : String(error)}`, + ); + }), + evaluatorSourceSha256(), + screeningProfileMetadata(screeningProfilePath), + ]); + const binarySha256 = createHash("sha256").update(binary).digest("hex"); + const fixtureCorpusSha256 = createHash("sha256") + .update(JSON.stringify(parsedCases)) + .digest("hex"); + + for (const [index, report] of reports.entries()) { + const summary = report.summary; + const reportNumber = index + 1; + const bindings: Array<[string, unknown, unknown]> = [ + ["binarySha256", summary.binarySha256, binarySha256], + ["fixtureCorpusSha256", summary.fixtureCorpusSha256, fixtureCorpusSha256], + ["evaluatorSha256", summary.evaluatorSha256, evaluatorSha256], + ["screeningProfileSha256", summary.screeningProfileSha256, profile.sha256], + ["upstreamProviderIdentity", summary.upstreamProviderIdentity, profile.upstreamProviderIdentity], + ["upstreamProviderRoute", summary.upstreamProviderRoute, profile.upstreamProviderRoute], + ["providerContractSha256", summary.providerContractSha256, profile.providerContractSha256], + ["providerContract", summary.providerContract, profile.providerContract], + ]; + for (const [field, actual, expected] of bindings) { + if (!isDeepStrictEqual(actual, expected)) { + throw new Error( + `release benchmark report ${reportNumber} ${field} is not bound to the supplied release input`, + ); + } + } + if (!isDeepStrictEqual(resultCohort(report), expectedResultCohort)) { + throw new Error( + `release benchmark report ${reportNumber} result identities do not match the current full corpus`, + ); + } + } +} + +async function readJson(path: string): Promise { + return JSON.parse(await readFile(path, "utf8")); +} + +function shellQuote(value: string): string { + return `'${value.replaceAll("'", `'"'"'`)}'`; +} + +export function formatRebaselineGuidance(options: Pick< + CompareCliOptions, + "binaryPath" | "screeningProfilePath" | "expectedRunIds" | "resultPaths" +>): string { + if (options.resultPaths.length !== 3 || options.expectedRunIds.length !== 3) { + return " collect three independent complete reports, then use the three-report --record command documented in bench/README.md"; + } + return [ + " bun run bench:compare -- \\", + ` --binary ${shellQuote(options.binaryPath)} \\`, + ` --screen-profile ${shellQuote(options.screeningProfilePath)} \\`, + ...options.expectedRunIds.map((runId) => + ` --expected-run-id ${shellQuote(runId)} \\`), + ...options.resultPaths.map((path) => ` --result ${shellQuote(path)} \\`), + " --record", + ].join("\n"); +} + +async function main() { + const { + baselinePath, + binaryPath, + screeningProfilePath, + expectedRunIds, + resultPaths, + record, + } = parseCliArguments(process.argv.slice(2)); + const reports = await loadReports(resultPaths); + assertExpectedRunIdentities(reports, expectedRunIds); + await assertReportsBoundToInputs(reports, binaryPath, screeningProfilePath); + const observed = aggregateObservedMetrics(reports); + const rebaselineGuidance = formatRebaselineGuidance({ + binaryPath, + screeningProfilePath, + expectedRunIds, + resultPaths, + }); const baselineRaw = await readJson(baselinePath).catch((error) => { throw new Error(`could not read baseline at ${baselinePath}: ${error instanceof Error ? error.message : String(error)}`); @@ -445,6 +1095,8 @@ async function main() { falsePositives: observed.falsePositives, gateVerdictCorrectness: observed.gateVerdictCorrectness, meanCostUsdPerCase: observed.meanCostUsdPerCase, + maximumRunCostUsdDecimal: observed.maximumRunCostUsdDecimal, + costCaseCount: observed.costCaseCount, latencyMs: observed.latencyMs, }; baselineFile.corpus = { @@ -469,7 +1121,7 @@ async function main() { "This is not a regression the gate can evaluate; it means the fixtures or scoring\n" + "code changed since the baseline was recorded. Re-baseline deliberately once you\n" + "have confirmed the new corpus is intentional:\n" + - ` bun run bench:compare -- --result ${resultPath} --record`, + rebaselineGuidance, ); process.exitCode = 1; return; @@ -479,7 +1131,7 @@ async function main() { if (profile === undefined) { console.error( `No baseline is recorded for model ${observed.model}. Populate one with:\n` + - ` bun run bench:compare -- --result ${resultPath} --record`, + rebaselineGuidance, ); process.exitCode = 1; return; @@ -492,15 +1144,53 @@ async function main() { return; } + if ( + !profile.providerContractEnforced || + profile.screeningProfileSha256 !== observed.screeningProfileSha256 || + profile.upstreamProviderIdentity !== observed.upstreamProviderIdentity || + profile.maximumRunCostUsdDecimal === undefined || + profile.costCaseCount === undefined || + profile.costCaseCount !== profile.totalCases + ) { + console.error( + "BASELINE PROVIDER MISMATCH: release quality and cost must use the exact recorded provider profile and exact cost evidence.\n" + + ` baseline screeningProfileSha256 ${profile.screeningProfileSha256 ?? "missing"}\n` + + ` observed screeningProfileSha256 ${observed.screeningProfileSha256 ?? "missing"}\n` + + ` baseline upstreamProviderIdentity ${profile.upstreamProviderIdentity ?? "missing"}\n` + + ` observed upstreamProviderIdentity ${observed.upstreamProviderIdentity ?? "missing"}`, + ); + process.exitCode = 1; + return; + } + + if ( + profile.reviewMode !== observed.reviewMode || + profile.totalCases !== observed.totalCases || + profile.scoredCases !== profile.totalCases || + profile.scoredCases !== observed.scoredCases + ) { + console.error( + "BASELINE COHORT MISMATCH: the validated report does not match the baseline execution mode or complete case count.\n" + + ` baseline reviewMode ${profile.reviewMode}; observed ${observed.reviewMode}\n` + + ` baseline totalCases ${profile.totalCases}; observed ${observed.totalCases}\n` + + ` baseline scoredCases ${profile.scoredCases}; observed ${observed.scoredCases}`, + ); + process.exitCode = 1; + return; + } + const comparison = compareMetrics(profile, observed); - console.log(`postil bench-live release gate: ${observed.model} (${observed.reviewMode})`); + console.log( + `postil bench-live release gate: ${observed.model} (${observed.reviewMode}, ` + + `${observed.reportCount} validated ${observed.reportCount === 1 ? "report" : "reports"})`, + ); console.log(formatComparisonTable(comparison.rows)); if (!comparison.ok) { console.error( "\nRELEASE BLOCKED: the live benchmark regressed past tolerance against bench/baseline.json.\n" + "Fix the regression, or if the new numbers are an accepted tradeoff, re-baseline\n" + "deliberately with:\n" + - ` bun run bench:compare -- --result ${resultPath} --record`, + rebaselineGuidance, ); process.exitCode = 1; return; diff --git a/bench/src/live.ts b/bench/src/live.ts index b3b340a..87526c2 100644 --- a/bench/src/live.ts +++ b/bench/src/live.ts @@ -43,7 +43,7 @@ import { } from "./livemodels-score"; const execFile = promisify(execFileCb); -const ADMISSION_API_BASE = "https://openrouter.ai:443/api/v1"; +export const ADMISSION_API_BASE = "https://openrouter.ai:443/api/v1"; const REQUEST_TIMEOUT_ENV = "POSTIL_LLM_REQUEST_TIMEOUT_SECS"; const TOTAL_TIMEOUT_ENV = "POSTIL_LLM_TOTAL_TIMEOUT_SECS"; const DEFAULT_CASE_TIMEOUT_MS = 180_000; @@ -163,6 +163,7 @@ export interface LiveCaseResult { } export interface LiveSummary { + runId: string; model: string; binary: string; binarySha256: string; @@ -367,7 +368,7 @@ async function writeLiveRunContract( }); } -async function screeningProfileMetadata(path: string): Promise<{ +export async function screeningProfileMetadata(path: string): Promise<{ sha256: string; upstreamProviderIdentity: string; upstreamProviderRoute: string; @@ -982,6 +983,7 @@ function summarize( : [parseCanonicalDecimal(result.observedProviderCostUsdDecimal)])); return { + runId: options.runId, model: options.model, binary: options.binary, binarySha256, diff --git a/bench/src/livemodels.test.ts b/bench/src/livemodels.test.ts index 3500202..473c3b5 100644 --- a/bench/src/livemodels.test.ts +++ b/bench/src/livemodels.test.ts @@ -1382,11 +1382,37 @@ describe("managed admission workflow", () => { expect(release.indexOf("bun run scorer-eval --json-out")).toBeLessThan( release.indexOf("bun run bench:live --"), ); - expect(release).toContain("bun run bench:live --"); + expect([...release.matchAll(/bun run bench:live --/gu)]).toHaveLength(3); + for (const sample of [1, 2, 3]) { + expect(release).toContain(`name: Run diff-file live benchmark sample ${sample}`); + expect(release).toContain(`id: live-sample-${sample}`); + expect(release).toContain(`--run-id "release-\${{ github.ref_name }}-sample-${sample}"`); + expect(release).toContain( + `--expected-run-id "release-\${{ github.ref_name }}-sample-${sample}"`, + ); + expect(release).toContain(`--json-out "\${{ runner.temp }}/bench-live-report-${sample}.json"`); + } + expect(release.indexOf("Run diff-file live benchmark sample 1")).toBeLessThan( + release.indexOf("Run diff-file live benchmark sample 2"), + ); + expect(release.indexOf("Run diff-file live benchmark sample 2")).toBeLessThan( + release.indexOf("Run diff-file live benchmark sample 3"), + ); + expect(release).toMatch( + /name: Upload the diff-file live reports\n\s+if: always\(\)[\s\S]*bench-live-report-1\.json[\s\S]*bench-live-report-2\.json[\s\S]*bench-live-report-3\.json[\s\S]*retention-days: 30/u, + ); expect(release).toMatch( - /name: Upload the diff-file live report\n\s+if: always\(\)[\s\S]*bench-live-report\.json[\s\S]*retention-days: 30/u, + /bun run bench:compare --[\s\S]*--binary "\$\{\{ github\.workspace \}\}\/target\/release\/postil"[\s\S]*--screen-profile \.\.\/provisional-models\.json[\s\S]*--result "\$\{\{ runner\.temp \}\}\/bench-live-report-1\.json"[\s\S]*--result "\$\{\{ runner\.temp \}\}\/bench-live-report-2\.json"[\s\S]*--result "\$\{\{ runner\.temp \}\}\/bench-live-report-3\.json"/u, ); - expect(release).toContain("bun run bench:compare --"); + expect([...release.matchAll(/continue-on-error: true/gu)]).toHaveLength(3); + expect(release).toContain("SAMPLE_1_OUTCOME: ${{ steps.live-sample-1.outcome }}"); + expect(release).toContain("SAMPLE_2_OUTCOME: ${{ steps.live-sample-2.outcome }}"); + expect(release).toContain("SAMPLE_3_OUTCOME: ${{ steps.live-sample-3.outcome }}"); + expect(release).not.toContain("bench-live-report-1.json.partial"); + expect(release).not.toContain("bench-live-report-2.json.partial"); + expect(release).not.toContain("bench-live-report-3.json.partial"); + expect(release).not.toContain("bench_live_override_reason"); + expect(release).not.toContain("OVERRIDE_REASON"); expect(release).toMatch(/build:\n\s+needs: \[validate-tag, bench-live\]/u); let checkedReferences = 0; const workflowGlob = new Bun.Glob("*.yml"); From f4d9d84774ce0b45b0beb33b2078cf58603ba64a Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Tue, 25 Aug 2026 19:20:48 +0000 Subject: [PATCH 3/7] Review oversized changes within deterministic bounds --- ARCHITECTURE.md | 29 +++--- ROADMAP.md | 3 +- src/diff.rs | 124 ++++++++++++++++------- src/envelope.rs | 45 +++++++++ src/review.rs | 111 ++++++++++++++++----- tests/e2e.rs | 257 +++++++++++++++++++++++++++++++++++++++++++----- 6 files changed, 476 insertions(+), 93 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 07ece20..1de095d 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -79,9 +79,12 @@ acquire diff --> parse supported lockfiles --> parse + index --> bounded evidenc executable vendor hunks require direct source evidence. Low-risk hunks receive semantic credit only from selected proof batches that retain the exact repository path, hunk identity, every changed line, and a substantive added line in the final - model request. Missing - direct capacity, proof evidence, or complete receipt coverage fails before plan - registration or provider contact. When + model request. The route fills its fixed capacity in stable metadata, risk, and path + order. A hunk receives direct credit only when every direct batch is selected; + otherwise it receives eligible exact semantic credit or remains unreviewed. The + selected schedule still runs when receipt coverage is incomplete, preserves valid + findings from selected evidence, and adds an error-severity operational finding that + keeps review trust and the merge gate failed. When `POSTIL_LARGE_REVIEW_PLAN_ENDPOINT` and `POSTIL_LARGE_REVIEW_PLAN_TOKEN` are set, the CLI registers a versioned deterministic request plan with the authenticated loopback endpoint before any provider call. A missing, rejected, or unreachable registration @@ -252,10 +255,11 @@ additions (violations are `kind: contentPolicy`). Content policy is on by defaul irreducible owner decision, not uncertainty about a concrete defect. Admin overrides apply to that kind-only decision rather than ordinary risk findings. 9. Review resource or request-budget exhaustion cannot produce a clean verdict. An - incomplete deterministic receipt fails before durable plan registration or model - contact. Other preflight exhaustion emits the generic internal `Review incomplete` - operational finding without model contact. Both preserve the hosted global deadline - and keep full-review reconciliation untrustworthy. + incomplete deterministic receipt registers and runs only its immutable bounded + schedule, then emits an operational coverage finding and keeps full-review + reconciliation untrustworthy. Other preflight exhaustion emits the generic internal + `Review incomplete` operational finding without model contact. Both preserve the + hosted global deadline and cannot produce a clean gate verdict. 10. Bounded JSON metadata pages use a 32 MiB per-page cap and a 64 MiB aggregate metadata cap. Source files and reconstructed diffs stream beyond that page limit into one 512 MiB operation workspace shared by acquired source snapshots, @@ -271,11 +275,12 @@ additions (violations are `kind: contentPolicy`). Content policy is on by defaul 12. Every completed review envelope records source-batch coverage when batching runs. Deterministic large reviews also record a plan hash and direct, semantic, and unreviewed hunk counts. Every normalized hunk has exactly one disposition; evidence - identifiers bind the exact hunk digest, and any unreviewed hunk rejects the plan - before registration or provider contact. Semantic coverage cannot resolve baseline - findings. Bounded reviews expose selected and total source-batch counts in compact - output. Planner fallback remains audit metadata and does not expose provider failure - details to a PR. + identifiers bind the exact hunk digest, and the plan hash binds the selected batch + order and payloads. Any unreviewed hunk forces failed review trust and a blocking + synthetic finding after the selected provider calls. Incomplete coverage cannot + resolve baseline findings. Bounded reviews expose selected and total source-batch + counts in compact output. Planner fallback remains audit metadata and does not expose + provider failure details to a PR. 13. Operational and provider virtual anchors expire after each run. Reviewable PR-description and change-metadata anchors carry across unrelated incremental reviews, and a same-head rerun with either anchor falls back to a full review. diff --git a/ROADMAP.md b/ROADMAP.md index ade0d13..21cf532 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -23,7 +23,8 @@ semantic batches for low-risk hunks, a 24-request quality ceiling lowered by hosted model and scorer fan-out, four-way concurrency, format-specific lockfile summaries, oversized-line segmentation, a hosted ceiling that reserves every enabled - post-processing call, and fail-closed incomplete coverage. + post-processing call, and fail-closed partial review that preserves findings from the + selected schedule when normalized hunks remain unreviewed. - `.coderabbit.yaml` translation for zero-cost migration. - Model cascade + concurrent multi-model consensus over any OpenAI-compatible endpoint; bounded retry with jittered backoff on transient provider errors. diff --git a/src/diff.rs b/src/diff.rs index 2f28dfe..c755cda 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -1433,7 +1433,8 @@ impl ModelBatchSpool { /// normalized hunk receives exactly one receipt disposition. Mandatory /// security, control-plane, dependency, and executable-vendor evidence is /// admitted first; remaining capacity is filled by stable risk and path - /// order. A mandatory hunk that cannot fit fails before provider contact. + /// order. Evidence beyond the hard request limit remains explicitly + /// unreviewed instead of preventing the selected schedule from running. pub fn deterministic_bounded_receipt( &mut self, selected_limit: usize, @@ -1498,12 +1499,12 @@ impl ModelBatchSpool { .then_with(|| left.hunk.digest.cmp(&right.hunk.digest)) }); - let mut selected = self.metadata_batch_ids.clone(); - anyhow::ensure!( - selected.len() <= selected_limit, - "mandatory dependency and artifact evidence needs {} batches, exceeding the {selected_limit} batch large-review limit", - selected.len() - ); + let mut selected = self + .metadata_batch_ids + .iter() + .take(selected_limit) + .copied() + .collect::>(); for candidate in ranked.iter().filter(|candidate| candidate.mandatory) { anyhow::ensure!( !candidate.direct_batch_ids.is_empty(), @@ -1511,14 +1512,12 @@ impl ModelBatchSpool { candidate.hunk.path, candidate.hunk.new_start ); - let additional = candidate.direct_batch_ids.difference(&selected).count(); - anyhow::ensure!( - selected.len().saturating_add(additional) <= selected_limit, - "mandatory hunk {}:{} cannot fit the {selected_limit} batch large-review limit; no provider request was made", - candidate.hunk.path, - candidate.hunk.new_start - ); - selected.extend(candidate.direct_batch_ids.iter().copied()); + for id in &candidate.direct_batch_ids { + if selected.len() >= selected_limit { + break; + } + selected.insert(*id); + } } for candidate in ranked.iter().filter(|candidate| !candidate.mandatory) { let direct_additional = candidate.direct_batch_ids.difference(&selected).count(); @@ -1560,6 +1559,13 @@ impl ModelBatchSpool { && selected.len().saturating_add(semantic_additional) <= selected_limit { selected.insert(id); + } else { + for id in &candidate.direct_batch_ids { + if selected.len() >= selected_limit { + break; + } + selected.insert(*id); + } } } @@ -1604,10 +1610,17 @@ impl ModelBatchSpool { "bounded coverage receipt contains a duplicate hunk disposition" ); + let selected_batch_text = self.batch_text_by_id(&selected)?; let mut hasher = Sha256::new(); + hasher.update(b"postil-deterministic-bounded-plan-v2\0"); hasher.update(format!("limit={selected_limit}\n").as_bytes()); for id in &selected { - hasher.update(format!("batch={id}\n").as_bytes()); + let batch = selected_batch_text + .get(id) + .expect("selected batch text was validated against the spool"); + hasher.update((*id as u64).to_le_bytes()); + hasher.update((batch.len() as u64).to_le_bytes()); + hasher.update(batch.as_bytes()); } for entry in &entries { hasher.update(entry.hunk.canonical().as_bytes()); @@ -7984,17 +7997,16 @@ diff --git a/two.rs b/two.rs false, ) .unwrap(); - let error = batches.deterministic_bounded_receipt(1).unwrap_err(); - assert!( - error - .to_string() - .contains("mandatory hunk vendor/tools/runner:1") - ); - assert!( - error - .to_string() - .contains("cannot fit the 1 batch large-review limit") - ); + let receipt = batches.deterministic_bounded_receipt(1).unwrap(); + assert_eq!(receipt.selected_batch_ids.len(), 1); + assert_eq!(receipt.entries.len(), 1); + assert_eq!(receipt.direct_hunks(), 0); + assert_eq!(receipt.semantic_hunks(), 0); + assert_eq!(receipt.unreviewed_hunks(), 1); + assert!(matches!( + receipt.entries[0].disposition, + HunkDisposition::Unreviewed { .. } + )); } #[test] @@ -8246,16 +8258,62 @@ diff --git a/two.rs b/two.rs } #[test] - fn deterministic_large_plan_rejects_an_unselected_security_hunk() { + fn deterministic_large_plan_is_stable_when_mandatory_hunks_exceed_capacity() { let source = deterministic_large_fixture(25); + let first = deterministic_receipt_for(&source); + let second = deterministic_receipt_for(&source); + + assert_eq!(first, second); + assert_eq!( + first.selected_batch_ids.len(), + crate::review::MAX_LARGE_DIFF_SELECTED_BATCHES + ); + assert!(first.direct_hunks() > 0); + assert!(first.unreviewed_hunks() > 0); + assert_eq!( + first.direct_hunks() + first.semantic_hunks() + first.unreviewed_hunks(), + first.entries.len() + ); + } + + #[test] + fn deterministic_large_plan_bounds_excess_metadata_and_mandatory_source() { + use std::fmt::Write as _; + + let mut source = String::new(); + for package in 0..81 { + let path = format!("packages/package-{package}/package-lock.json"); + writeln!( + source, + "diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n@@ -1,2 +1,2 @@\n \"dependency-{package}\": {{\n- \"version\": \"1.0.0\"\n+ \"version\": \"2.0.0\"" + ) + .unwrap(); + } + for file in 0..8 { + let path = format!("vendor/lib/Runner-{file}.java"); + writeln!( + source, + "diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n@@ -1 +1 @@\n-int value = {file};\n+int value = {}; // {}", + file + 1, + "x".repeat(20_000), + ) + .unwrap(); + } + let snapshot = DiffSnapshot::from_bytes(source.as_bytes()).unwrap(); let mut prepared = prepare_review(&snapshot).unwrap(); let mut batches = spool_model_batches(&mut prepared, 16_000, 4_096, false).unwrap(); - let error = batches - .deterministic_bounded_receipt(crate::review::MAX_LARGE_DIFF_SELECTED_BATCHES) - .unwrap_err(); - assert!(error.to_string().contains("mandatory hunk")); - assert!(error.to_string().contains("cannot fit")); + assert!(batches.metadata_batch_ids.len() > 5); + let receipt = batches.deterministic_bounded_receipt(5).unwrap(); + + assert_eq!(receipt.selected_batch_ids.len(), 5); + assert!( + receipt + .selected_batch_ids + .is_subset(&batches.metadata_batch_ids) + ); + assert_eq!(receipt.entries.len(), 8); + assert_eq!(receipt.unreviewed_hunks(), 8); } #[test] diff --git a/src/envelope.rs b/src/envelope.rs index 9e545a2..98f90c7 100644 --- a/src/envelope.rs +++ b/src/envelope.rs @@ -1870,6 +1870,41 @@ pub(crate) fn incomplete_review_finding(reason: IncompleteReviewReason) -> Findi } } +/// The synthetic finding emitted when a deterministic large review runs its +/// complete selected schedule but the hard request limit leaves normalized +/// hunks without direct or exact semantic coverage. +pub(crate) fn incomplete_large_review_finding(unreviewed_hunks: u32) -> Finding { + let noun = if unreviewed_hunks == 1 { + "hunk" + } else { + "hunks" + }; + Finding { + path: OPERATIONAL_PATH.to_string(), + line: 1, + end_line: None, + severity: Severity::Error, + kind: Kind::Uncertainty, + confidence: 1.0, + title: "Large review coverage is incomplete".to_string(), + body: format!( + "Deterministic large-review coverage left {unreviewed_hunks} normalized {noun} \ + unreviewed within the hard request limit. Findings from completed requests \ + remain available, but this result cannot be trusted as a pass." + ), + evidence: None, + id: None, + generator_confidence: None, + scorer_confidence: None, + generator_kind: None, + scorer_kind: None, + scorer_reason: None, + repository_claim: None, + machine_claim: None, + machine_claim_deferred: false, + } +} + /// The synthetic finding emitted when a review stopped before producing a /// verdict for a reason that is not model output: an admission limit, a /// configuration problem, or an unexpected internal error. Postil still fails @@ -2046,6 +2081,16 @@ mod tests { assert_eq!(validate_finding_publication(&finding), Ok(())); } + #[test] + fn incomplete_large_review_finding_is_publishable_and_fail_closed() { + let finding = incomplete_large_review_finding(3); + assert_eq!(finding.severity, Severity::Error); + assert_eq!(finding.path, OPERATIONAL_PATH); + assert!(finding.body.contains("left 3 normalized hunks unreviewed")); + assert_eq!(validate_finding_public_language(&finding), Ok(())); + assert_eq!(validate_finding_publication(&finding), Ok(())); + } + #[test] fn fail_closed_finding_neutralizes_evidence_boundary_details() { let finding = fail_closed_finding("none grounded in the diff"); diff --git a/src/review.rs b/src/review.rs index c3cd5ab..86cf1c3 100644 --- a/src/review.rs +++ b/src/review.rs @@ -1711,6 +1711,7 @@ async fn review_diff_at( let mut findings: Vec = Vec::new(); let mut suppressed_findings = Vec::new(); let mut review_trust = filter::ReviewTrust::Failed; + let mut incomplete_large_review_hunks = 0u32; let mut scorer_model: Option = None; let mut scorer_error: Option = None; let mut scorer_disagreements: Option = None; @@ -1837,11 +1838,6 @@ async fn review_diff_at( .then(|| batches.deterministic_bounded_receipt(large_diff_selected_limit)) .transpose()?; if let Some(receipt) = &large_diff_receipt { - anyhow::ensure!( - receipt.unreviewed_hunks() == 0, - "deterministic large-review plan leaves {} normalized hunks unreviewed within its {large_diff_selected_limit}-request limit; no provider request was made", - receipt.unreviewed_hunks() - ); eprintln!( "postil: deterministic large-review plan={} direct_hunks={} semantic_hunks={} unreviewed_hunks={} selected_batches={}/{} concurrency={} request_timeout={}s review_budget={}s", receipt.plan_sha256, @@ -1871,6 +1867,9 @@ async fn review_diff_at( }) }) .transpose()?; + incomplete_large_review_hunks = large_receipt_summary + .as_ref() + .map_or(0, |receipt| receipt.unreviewed_hunks); let deterministic_large_review = large_diff_receipt.is_some(); let mut durable_plan_registration = if let Some(registrar) = DurablePlanRegistrar::from_env()? { @@ -2392,7 +2391,7 @@ async fn review_diff_at( // carries baseline evidence outside the selected input. // A changed citation expires only when a completed model // request covered its coordinate and did not reproduce it. - review_trust = if batch_failed { + review_trust = if batch_failed || incomplete_large_review_hunks > 0 { filter::ReviewTrust::Failed } else if risk_selected_review { filter::ReviewTrust::Bounded @@ -2439,7 +2438,7 @@ async fn review_diff_at( let mut all_adjudication_candidates = kept.clone(); let fresh_candidate_count = all_adjudication_candidates.len(); let mut baseline_candidate_indices = Vec::new(); - if full_rereview { + if full_rereview && incomplete_large_review_hunks == 0 { for (baseline_index, previous) in baseline.iter().enumerate() { // The complete direct diff may prove an exact citation was // deleted even when bounded source selection omitted its batch. @@ -2792,6 +2791,12 @@ async fn review_diff_at( if let Some(failure) = batch_failure { findings.push(failure); } + if incomplete_large_review_hunks > 0 { + review_trust = filter::ReviewTrust::Failed; + findings.push(crate::envelope::incomplete_large_review_finding( + incomplete_large_review_hunks, + )); + } } } } @@ -2843,13 +2848,18 @@ async fn review_diff_at( suppressed = suppressed.saturating_add(repository_suppressed.len() as u32); suppressed_findings.extend(repository_suppressed); - let machine_application = apply_machine_claim_policy( + let mut machine_application = apply_machine_claim_policy( &mut findings, &mut baseline, &adjudication_preserved_baseline, claim_verification.as_ref(), repository_revision.as_deref(), ); + if incomplete_large_review_hunks > 0 { + // Exact-head verification remains useful for fresh findings, but an + // incomplete review cannot close any carried baseline entry. + baseline.append(&mut machine_application.resolved); + } suppressed = suppressed.saturating_add(machine_application.suppressed.len() as u32); suppressed_findings.extend(machine_application.suppressed); @@ -2922,17 +2932,18 @@ async fn review_diff_at( .iter() .map(|kind| kind.as_str().to_string()) .collect(); - let gate_failing = findings.iter().any(|f| { - if gate_disabled { - false - } else if f.path == crate::envelope::OPERATIONAL_PATH { - true - } else if f.path == crate::envelope::PROVIDER_PATH { - !advisory_on_error - } else { - crate::envelope::finding_blocks_gate(f, gate_fail_on, &gate_block_on_kinds, false) - } - }); + let gate_failing = incomplete_large_review_hunks > 0 + || findings.iter().any(|f| { + if gate_disabled { + false + } else if f.path == crate::envelope::OPERATIONAL_PATH { + true + } else if f.path == crate::envelope::PROVIDER_PATH { + !advisory_on_error + } else { + crate::envelope::finding_blocks_gate(f, gate_fail_on, &gate_block_on_kinds, false) + } + }); let silent = findings.is_empty(); // Raw summaries are validated for agreement with raw structured output, // then discarded. Presentation is derived from the final reconciled set so @@ -3735,6 +3746,10 @@ fn error_envelope( incomplete_input.then_some(crate::envelope::IncompleteReviewReason::IncompleteInput) }); let review_failure = err.downcast_ref::(); + let incomplete_large_review_hunks = review_failure + .and_then(|failure| failure.review_coverage.as_ref()) + .and_then(|coverage| coverage.receipt.as_ref()) + .map_or(0, |receipt| receipt.unreviewed_hunks); let invalid_output = review_failure.is_some_and(|failure| failure.kind == ReviewFailureKind::InvalidOutput); let advisory_operational_error = review_failure @@ -3750,7 +3765,7 @@ fn error_envelope( .downcast_ref::() .is_some_and(|error| error.is_connect() || error.is_timeout()) }); - let findings = vec![if let Some(reason) = incomplete_reason { + let mut findings = vec![if let Some(reason) = incomplete_reason { crate::envelope::incomplete_review_finding(reason) } else if invalid_output || !advisory_operational_error { // Only a recorded scorer error is genuinely about model output. Every @@ -3764,11 +3779,16 @@ fn error_envelope( } else { crate::envelope::provider_error_finding(&format!("{err:#}")) }]; + if incomplete_large_review_hunks > 0 { + findings.push(crate::envelope::incomplete_large_review_finding( + incomplete_large_review_hunks, + )); + } let counts = Envelope::counts_of(&findings, 0); let buckets = Envelope::buckets_of(&findings); let gate_disabled = cfg.gate_fail_on.as_str().eq_ignore_ascii_case("never"); - let blocking = - !gate_disabled && (!advisory_operational_error || cfg.gate_on_error == OnError::Block); + let blocking = incomplete_large_review_hunks > 0 + || (!gate_disabled && (!advisory_operational_error || cfg.gate_on_error == OnError::Block)); let mut model_usage = review_failure .map(|failure| failure.model_usage.clone()) .unwrap_or_default(); @@ -4012,6 +4032,13 @@ mod tests { } fn rich_scorer_failure(kind: ReviewFailureKind) -> anyhow::Error { + rich_scorer_failure_with_receipt(kind, None) + } + + fn rich_scorer_failure_with_receipt( + kind: ReviewFailureKind, + receipt: Option, + ) -> anyhow::Error { ReviewFailure { kind, detail: "hosted scorer could not complete the admitted profile".to_string(), @@ -4069,7 +4096,7 @@ mod tests { selected_batches: 5, total_batches: 9, planner_fallback: false, - receipt: None, + receipt, }), review_admission: Some(ReviewAdmission { provider_attempts: 12, @@ -4134,6 +4161,44 @@ mod tests { assert_eq!(envelope.review_admission.unwrap().provider_attempts, 12); } + #[test] + fn incomplete_coverage_stays_blocking_when_the_provider_also_fails() { + let cfg = Config { + gate_fail_on: crate::config::GateLevel::Never, + gate_on_error: OnError::Advisory, + ..Config::default() + }; + let error = rich_scorer_failure_with_receipt( + ReviewFailureKind::Provider, + Some(ReviewCoverageReceipt { + plan_sha256: "a".repeat(64), + total_hunks: 1, + direct_hunks: 0, + semantic_hunks: 0, + unreviewed_hunks: 1, + }), + ); + let envelope = error_envelope(&cfg, &error, "head", Some(&pr_meta()), 99); + + assert!(envelope.gate.failing); + assert!(envelope.findings.iter().any(|finding| { + finding.path == crate::envelope::PROVIDER_PATH + && finding.title == "Model provider unavailable" + })); + assert!(envelope.findings.iter().any(|finding| { + finding.path == crate::envelope::OPERATIONAL_PATH + && finding.title == "Large review coverage is incomplete" + })); + assert_eq!( + envelope + .review_coverage + .as_ref() + .and_then(|coverage| coverage.receipt.as_ref()) + .map(|receipt| receipt.unreviewed_hunks), + Some(1) + ); + } + #[test] fn planning_failure_remains_blocking_under_advisory_provider_policy() { let cfg = Config { diff --git a/tests/e2e.rs b/tests/e2e.rs index f5055fa..26a3734 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -2672,6 +2672,71 @@ fn qualification_candidate_admits_complete_large_review_inside_watchdog_capacity assert!(envelope.get("modelUsage").is_none()); } +#[cfg(feature = "qualification-candidate")] +#[test] +fn qualification_candidate_reports_incomplete_bounded_coverage_without_provider_contact() { + use std::fmt::Write as _; + + let dir = tempfile::tempdir().unwrap(); + let diff_path = dir.path().join("large-incomplete.diff"); + let mut diff = String::new(); + for file in 0..30 { + let path = format!("src/auth/permission-{file}.ts"); + writeln!( + diff, + "diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n@@ -1,2 +1,2 @@\n-if (!actor.can('admin')) throw new Error('Forbidden');\n+await privilegedWrite(input_{file});\n {}", + "x".repeat(20_000), + ) + .unwrap(); + } + std::fs::write(&diff_path, diff).unwrap(); + + let metadata = postil_cli::config::qualification_metadata(); + let model = "openai/gpt-5-mini"; + let profile_path = dir.path().join("candidate.json"); + std::fs::write( + &profile_path, + serde_json::to_vec(&json!({ + "benchmarkProviderIdentity": postil_cli::config::MANAGED_OPENROUTER_PROVIDER_IDENTITY, + "upstreamProviderIdentity": "test-provider", + "upstreamProviderRoute": "test-provider", + "apiBase": metadata.default_api_base, + "apiFormat": metadata.default_api_format, + "generatorChain": [model], + "consensus": 1, + "scorerChain": [model], + "modelPriceBounds": [{ + "model": model, + "inputMicrosPerMillionTokens": 1, + "outputMicrosPerMillionTokens": 1 + }] + })) + .unwrap(), + ) + .unwrap(); + + let out = postil() + .current_dir(dir.path()) + .env("CI", "true") + .env("GITHUB_API_URL", "http://127.0.0.1:9") + .env("POSTIL_BENCH_REQUIRE_HOSTED_PROVIDER_PRIVACY", "1") + .env("POSTIL_QUALIFICATION_CANDIDATE_PROFILE", &profile_path) + .env("POSTIL_QUALIFICATION_PLAN_ONLY", "1") + .args(["review", "--diff-file"]) + .arg(&diff_path) + .args(["--output", "json"]) + .assert() + .success(); + let envelope: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); + let coverage = &envelope["reviewCoverage"]; + assert_eq!(coverage["mode"], "bounded"); + assert!(coverage["receipt"]["unreviewedHunks"].as_u64().unwrap() > 0); + assert!(coverage["selectedBatches"].as_u64().unwrap() <= 24); + assert!(envelope.get("reviewAdmission").is_some()); + assert_eq!(envelope["modelUsed"], "none (qualification plan)"); + assert!(envelope.get("modelUsage").is_none()); +} + #[cfg(feature = "qualification-candidate")] #[test] fn qualification_candidate_splits_json_escaped_batches_within_model_context() { @@ -4003,25 +4068,35 @@ async fn unverifiable_git_binary_patch_fails_before_provider_contact() { } #[tokio::test] -async fn oversized_security_hunk_fails_before_provider_contact() { +async fn oversized_security_hunk_runs_bounded_review_and_fails_closed() { use std::fmt::Write as _; let server = MockServer::start().await; + let registration_token = "oversized-security-plan-token"; + Mock::given(method("POST")) + .and(path("/durable-plan")) + .and(header( + "authorization", + format!("Bearer {registration_token}"), + )) + .respond_with(ResponseTemplate::new(204)) + .mount(&server) + .await; Mock::given(method("POST")) .and(path("/chat/completions")) .respond_with(|request: &wiremock::Request| { let body = String::from_utf8_lossy(&request.body); - let findings = if body.contains("dangerous_final_call") { + let findings = if body.contains("dangerous_selected_call") { let evidence = - prompt_evidence(request, "src/auth.rs", 10_000, "dangerous_final_call"); + prompt_evidence(request, "src/auth.rs", 1, "dangerous_selected_call"); json!([{ "path": "src/auth.rs", - "line": 10_000, + "line": 1, "severity": "warn", "kind": "risk", "confidence": 0.95, - "title": "Validate the final call", - "body": "The final call receives untrusted input. Validate it before use.", + "title": "Validate the selected call", + "body": "The selected call receives untrusted input. Validate it before use.", "evidence": evidence }]) } else { @@ -4038,7 +4113,13 @@ async fn oversized_security_hunk_fails_before_provider_contact() { "diff --git a/src/auth.rs b/src/auth.rs\n--- a/src/auth.rs\n+++ b/src/auth.rs\n@@ -0,0 +1,10000 @@" ) .unwrap(); - for line in 1..10_000 { + writeln!( + source, + "+dangerous_selected_call(user_input); // {}", + "x".repeat(3_500), + ) + .unwrap(); + for line in 2..=10_000 { writeln!( source, "+let reviewed_{line:05} = validate(input_{line:05}); // {}", @@ -4046,29 +4127,72 @@ async fn oversized_security_hunk_fails_before_provider_contact() { ) .unwrap(); } - source.push_str("+dangerous_final_call(user_input);\n"); assert!(source.len() > 32 * 1024 * 1024); let dir = tempfile::tempdir().unwrap(); let diff = dir.path().join("large-source.diff"); std::fs::write(&diff, source).unwrap(); + std::fs::write( + dir.path().join(".postil.yaml"), + "gate:\n failOn: never\n onError: advisory\n", + ) + .unwrap(); let out = postil() .current_dir(dir.path()) .env("POSTIL_API_BASE", server.uri()) + .env( + "POSTIL_LARGE_REVIEW_PLAN_ENDPOINT", + format!("{}/durable-plan", server.uri()), + ) + .env("POSTIL_LARGE_REVIEW_PLAN_TOKEN", registration_token) .env("POSTIL_DISABLE_SCORER", "1") .args(["review", "--diff-file"]) .arg(&diff) .args(["--output", "json"]) .assert() - .code(2); + .code(1); - let stderr = String::from_utf8_lossy(&out.get_output().stderr); + let envelope: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); + let coverage = &envelope["reviewCoverage"]; + let receipt = &coverage["receipt"]; + assert_eq!(coverage["mode"], "bounded"); + assert_eq!(receipt["semanticHunks"], 0); + assert!(receipt["unreviewedHunks"].as_u64().unwrap() > 0); + assert_eq!( + receipt["directHunks"].as_u64().unwrap() + + receipt["semanticHunks"].as_u64().unwrap() + + receipt["unreviewedHunks"].as_u64().unwrap(), + receipt["totalHunks"].as_u64().unwrap() + ); + assert!(coverage["selectedBatches"].as_u64().unwrap() <= 24); assert!( - stderr.contains("mandatory hunk src/auth.rs:1 cannot fit the 24 batch large-review limit"), - "{stderr}" + coverage["selectedBatches"].as_u64().unwrap() < coverage["totalBatches"].as_u64().unwrap() ); + assert_eq!(envelope["gate"]["failing"], true); + let findings = envelope["findings"].as_array().unwrap(); + assert!( + findings + .iter() + .any(|finding| finding["title"] == "Validate the selected call") + ); + assert!(findings.iter().any(|finding| { + finding["title"] == "Large review coverage is incomplete" && finding["severity"] == "error" + })); + let requests = server.received_requests().await.unwrap(); - assert!(requests.is_empty()); + assert_eq!(requests[0].url.path(), "/durable-plan"); + let registration: Value = serde_json::from_slice(&requests[0].body).unwrap(); + assert_eq!(registration["selectedBatches"], 24); + assert_eq!(registration["unreviewedHunks"], receipt["unreviewedHunks"]); + assert_eq!( + registration["planSha256"], + coverage["receipt"]["planSha256"] + ); + let source_requests = requests + .iter() + .filter(|request| is_source_review_request(request)) + .count(); + assert_eq!(source_requests, 24); } #[tokio::test] @@ -4244,7 +4368,7 @@ async fn automatic_large_diff_route_reviews_losslessly_compacted_low_signal_hunk } #[tokio::test] -async fn mandatory_raw_dependency_and_vendor_overflow_fails_before_provider_contact() { +async fn mandatory_raw_dependency_and_vendor_overflow_runs_within_capacity() { use std::fmt::Write as _; let server = MockServer::start().await; @@ -4279,16 +4403,30 @@ async fn mandatory_raw_dependency_and_vendor_overflow_fails_before_provider_cont .arg(&diff) .args(["--output", "json"]) .assert() - .code(2); + .code(1); - let stderr = String::from_utf8_lossy(&out.get_output().stderr); - assert!(stderr.contains("mandatory hunk")); - assert!(stderr.contains("cannot fit the 24 batch large-review limit")); - assert!(server.received_requests().await.unwrap().is_empty()); + let envelope: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); + let coverage = &envelope["reviewCoverage"]; + assert_eq!(coverage["mode"], "bounded"); + assert!(coverage["receipt"]["unreviewedHunks"].as_u64().unwrap() > 0); + assert_eq!(envelope["gate"]["failing"], true); + assert!( + envelope["findings"] + .as_array() + .unwrap() + .iter() + .any(|finding| { + finding["title"] == "Large review coverage is incomplete" + && finding["severity"] == "error" + }) + ); + let requests = server.received_requests().await.unwrap(); + assert!(!requests.is_empty()); + assert!(requests.len() <= 24); } #[tokio::test] -async fn automatic_large_diff_route_rejects_unreviewed_hunks_before_provider_contact() { +async fn automatic_large_diff_route_registers_and_reviews_incomplete_coverage() { use std::fmt::Write as _; let server = MockServer::start().await; @@ -4345,6 +4483,36 @@ async fn automatic_large_diff_route_rejects_unreviewed_hunks_before_provider_con let dir = tempfile::tempdir().unwrap(); let diff = dir.path().join("automatic-large.diff"); std::fs::write(&diff, source).unwrap(); + let baseline_path = dir.path().join("baseline.json"); + std::fs::write( + &baseline_path, + json!({ + "version": 1, + "summary": "", + "silent": false, + "findings": [{ + "path": "src/churn/file-0.ts", + "line": 1, + "severity": "error", + "kind": "risk", + "confidence": 0.9, + "title": "Keep the prior finding open", + "body": "The prior finding remains open until a complete review resolves it.", + "evidence": "const value = 0;" + }], + "resolved": [], + "counts": {"info": 0, "warn": 0, "error": 1, "suppressed": 0}, + "confidenceBuckets": [0, 0, 0, 0, 1], + "gate": {"failOn": "error", "failing": true}, + "modelUsed": "model", + "usage": {"promptTokens": 0, "completionTokens": 0}, + "baseSha": null, + "headSha": null, + "sinceSha": null + }) + .to_string(), + ) + .unwrap(); let out = postil() .current_dir(dir.path()) .env("POSTIL_API_BASE", server.uri()) @@ -4357,15 +4525,56 @@ async fn automatic_large_diff_route_rejects_unreviewed_hunks_before_provider_con .env("REVIEW_MODEL", "mistralai/mistral-small-3.2-24b-instruct") .args(["review", "--diff-file"]) .arg(&diff) + .arg("--baseline") + .arg(&baseline_path) .args(["--output", "json"]) .assert() - .code(2); + .code(1); let stderr = String::from_utf8_lossy(&out.get_output().stderr); - assert!(stderr.contains("normalized hunks unreviewed"), "{stderr}"); - assert!(stderr.contains("no provider request was made"), "{stderr}"); + assert!(stderr.contains("unreviewed_hunks="), "{stderr}"); assert!(!stderr.contains(registration_token)); - assert!(server.received_requests().await.unwrap().is_empty()); + let envelope: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); + let coverage = &envelope["reviewCoverage"]; + assert_eq!(coverage["mode"], "bounded"); + assert!(coverage["receipt"]["unreviewedHunks"].as_u64().unwrap() > 0); + assert_eq!(envelope["gate"]["failing"], true); + assert!( + envelope["findings"] + .as_array() + .unwrap() + .iter() + .any(|finding| { + finding["title"] == "Large review coverage is incomplete" + && finding["severity"] == "error" + }) + ); + assert!( + envelope["findings"] + .as_array() + .unwrap() + .iter() + .any(|finding| { + finding["title"] == "Keep the prior finding open" + && finding["body"] + .as_str() + .is_some_and(|body| body.starts_with("[carried from previous review]")) + }) + ); + assert_eq!(envelope["resolved"], json!([])); + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests[0].url.path(), "/durable-plan"); + let registration: Value = serde_json::from_slice(&requests[0].body).unwrap(); + assert_eq!( + registration["unreviewedHunks"], + coverage["receipt"]["unreviewedHunks"] + ); + let source_requests = requests + .iter() + .filter(|request| is_source_review_request(request)) + .count(); + assert!(source_requests > 0); + assert!(source_requests <= 24); } #[tokio::test] From 20574e1867c0af2eaf4ff6156374cac32b854512 Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Tue, 25 Aug 2026 19:53:50 +0000 Subject: [PATCH 4/7] Record three-sample Luna baseline --- bench/baseline.json | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/bench/baseline.json b/bench/baseline.json index 1ccc426..7a9200f 100644 --- a/bench/baseline.json +++ b/bench/baseline.json @@ -26,23 +26,23 @@ }, "openai/gpt-5.6-luna": { "populated": true, - "generatedAt": "2026-08-25T17:11:56.688Z", + "generatedAt": "2026-08-25T19:53:38.680Z", "reviewMode": "exhaustive", - "sourceRunAt": "2026-08-25T17:09:36.499Z", + "sourceRunAt": "2026-08-25T19:41:11.694Z", "providerContractEnforced": true, "screeningProfileSha256": "aea05c3f5622cebec480d2a8daf5bb53055bc0160206e6f22e889391ea71fa49", "upstreamProviderIdentity": "Azure", "totalCases": 70, "scoredCases": 70, - "detectionRate": 0.9473684210526315, + "detectionRate": 0.9298245614035088, "falsePositives": 0, - "gateVerdictCorrectness": 0.8428571428571429, - "meanCostUsdPerCase": 0.0008315965428571429, - "maximumRunCostUsdDecimal": "0.058211758", + "gateVerdictCorrectness": 0.8142857142857143, + "meanCostUsdPerCase": 0.0007914606857142857, + "maximumRunCostUsdDecimal": "0.055402248", "costCaseCount": 70, "latencyMs": { - "p50": 5659, - "p95": 9530 + "p50": 5449, + "p95": 9874 } } } From db0cbcbb655f81e7ca7338ec13ba6a5d975db54e Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Tue, 25 Aug 2026 20:13:09 +0000 Subject: [PATCH 5/7] Prioritize mandatory evidence in bounded reviews --- src/diff.rs | 28 ++++++++++++++-------------- tests/e2e.rs | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/diff.rs b/src/diff.rs index c755cda..6e50c67 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -1499,12 +1499,7 @@ impl ModelBatchSpool { .then_with(|| left.hunk.digest.cmp(&right.hunk.digest)) }); - let mut selected = self - .metadata_batch_ids - .iter() - .take(selected_limit) - .copied() - .collect::>(); + let mut selected = BTreeSet::new(); for candidate in ranked.iter().filter(|candidate| candidate.mandatory) { anyhow::ensure!( !candidate.direct_batch_ids.is_empty(), @@ -1512,12 +1507,16 @@ impl ModelBatchSpool { candidate.hunk.path, candidate.hunk.new_start ); - for id in &candidate.direct_batch_ids { - if selected.len() >= selected_limit { - break; - } - selected.insert(*id); + let additional = candidate.direct_batch_ids.difference(&selected).count(); + if selected.len().saturating_add(additional) <= selected_limit { + selected.extend(candidate.direct_batch_ids.iter().copied()); + } + } + for id in &self.metadata_batch_ids { + if selected.len() >= selected_limit { + break; } + selected.insert(*id); } for candidate in ranked.iter().filter(|candidate| !candidate.mandatory) { let direct_additional = candidate.direct_batch_ids.difference(&selected).count(); @@ -8277,7 +8276,7 @@ diff --git a/two.rs b/two.rs } #[test] - fn deterministic_large_plan_bounds_excess_metadata_and_mandatory_source() { + fn deterministic_large_plan_prioritizes_mandatory_source_over_excess_metadata() { use std::fmt::Write as _; let mut source = String::new(); @@ -8308,12 +8307,13 @@ diff --git a/two.rs b/two.rs assert_eq!(receipt.selected_batch_ids.len(), 5); assert!( - receipt + !receipt .selected_batch_ids .is_subset(&batches.metadata_batch_ids) ); assert_eq!(receipt.entries.len(), 8); - assert_eq!(receipt.unreviewed_hunks(), 8); + assert!(receipt.direct_hunks() > 0); + assert!(receipt.unreviewed_hunks() < 8); } #[test] diff --git a/tests/e2e.rs b/tests/e2e.rs index 26a3734..4003721 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -4072,7 +4072,7 @@ async fn oversized_security_hunk_runs_bounded_review_and_fails_closed() { use std::fmt::Write as _; let server = MockServer::start().await; - let registration_token = "oversized-security-plan-token"; + let registration_token = "TEST_FIXTURE_NOT_A_SECRET"; Mock::given(method("POST")) .and(path("/durable-plan")) .and(header( From 3b50dd85a3bff42bc7132f8769457848940615b2 Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Tue, 25 Aug 2026 20:17:18 +0000 Subject: [PATCH 6/7] Retain partial evidence for oversized mandatory hunks --- src/diff.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/diff.rs b/src/diff.rs index 6e50c67..59e388c 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -1507,9 +1507,11 @@ impl ModelBatchSpool { candidate.hunk.path, candidate.hunk.new_start ); - let additional = candidate.direct_batch_ids.difference(&selected).count(); - if selected.len().saturating_add(additional) <= selected_limit { - selected.extend(candidate.direct_batch_ids.iter().copied()); + for id in &candidate.direct_batch_ids { + if selected.len() >= selected_limit { + break; + } + selected.insert(*id); } } for id in &self.metadata_batch_ids { From 6910f35f1004029823d051c9d920697e7b0d0a16 Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Tue, 25 Aug 2026 20:48:52 +0000 Subject: [PATCH 7/7] Record v0.9.3 Luna release baseline --- bench/baseline.json | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/bench/baseline.json b/bench/baseline.json index 7a9200f..48b5a7b 100644 --- a/bench/baseline.json +++ b/bench/baseline.json @@ -26,23 +26,23 @@ }, "openai/gpt-5.6-luna": { "populated": true, - "generatedAt": "2026-08-25T19:53:38.680Z", + "generatedAt": "2026-08-25T20:48:41.276Z", "reviewMode": "exhaustive", - "sourceRunAt": "2026-08-25T19:41:11.694Z", + "sourceRunAt": "2026-08-25T20:37:29.452Z", "providerContractEnforced": true, "screeningProfileSha256": "aea05c3f5622cebec480d2a8daf5bb53055bc0160206e6f22e889391ea71fa49", "upstreamProviderIdentity": "Azure", "totalCases": 70, "scoredCases": 70, - "detectionRate": 0.9298245614035088, + "detectionRate": 0.9473684210526315, "falsePositives": 0, - "gateVerdictCorrectness": 0.8142857142857143, - "meanCostUsdPerCase": 0.0007914606857142857, - "maximumRunCostUsdDecimal": "0.055402248", + "gateVerdictCorrectness": 0.8285714285714286, + "meanCostUsdPerCase": 0.0007761659714285714, + "maximumRunCostUsdDecimal": "0.054331618", "costCaseCount": 70, "latencyMs": { - "p50": 5449, - "p95": 9874 + "p50": 5240, + "p95": 9363 } } }