From 84a41eba637f12d09e0b3b1f1973991f4c94fcbc Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Fri, 11 Sep 2026 00:39:06 +0000 Subject: [PATCH] Complete hosted source coverage within the existing review budget --- ARCHITECTURE.md | 12 +++-- src/llm.rs | 113 +++++++++++++++++++++++++++++++++--------------- src/review.rs | 16 ++++--- tests/e2e.rs | 102 ++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 193 insertions(+), 50 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index aba863b..1289795 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -68,11 +68,15 @@ acquire diff --> parse supported lockfiles --> parse + index --> bounded evidenc provider-attempt, output-token, and worst-case token-exposure budgets before calls; one UTF-8 byte counts as one projected token rather than using an optimistic ratio. A diff that exceeds its selected-request capacity enters a deterministic large-review - route on every surface, including `--diff-file`. Non-hosted execution selects at most - 24 requests. Hosted execution lowers that ceiling when the configured generator, + route on every surface, including `--diff-file`. Hosted reviews also use that route + above five source batches unless explicit bounded selection is requested. + Non-hosted execution selects at most 24 requests. Hosted execution lowers that ceiling when the configured generator, consensus, scorer, uncertainty-resolution, and finding-compression fan-out needs - fewer requests to stay inside the 64-call watchdog plan. The route uses at most four - concurrent provider calls; consensus reduces batch concurrency so combined model + fewer requests to stay inside the 64-call watchdog plan. Admission divides the + remaining generation budget across the selected batch waves, reserving time for + at least one bounded attempt per operation. Retries and corrections share each + operation's slot; completed waves do not reset the deadline or spend counters. + The route uses at most four concurrent provider calls; consensus reduces batch concurrency so combined model fan-out stays within that limit. It commits an exact hunk receipt by SHA-256 before provider contact. Security, authorization, configuration, policy, billing, migration, release-control, and diff --git a/src/llm.rs b/src/llm.rs index 33631d4..3d5473d 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -804,7 +804,11 @@ pub(crate) fn max_hosted_review_batches(cfg: &Config, planner: bool) -> Result Option { let base = self.review_model_timeout?; match self.review_model_slot_secs.load(Ordering::Relaxed) { 0 => Some(base), - seconds => Some(Duration::from_secs(seconds).max(base)), + seconds => Some(Duration::from_secs(seconds)), } } @@ -5142,7 +5142,12 @@ impl LlmClient { .context("hosted review schedule operation count overflowed")?; let operations = u32::try_from(review_model_operations) .context("hosted review schedule operation count exceeds duration range")?; - let required = slot + ensure!( + operations > 0, + "hosted review schedule has no model operations" + ); + let minimum_slot = slot.min(RETRY_ATTEMPT_RESERVE); + let required = minimum_slot .checked_mul(operations) .context("hosted review schedule duration overflowed")?; let remaining = self @@ -5160,15 +5165,10 @@ impl LlmClient { elapsed_text(required), elapsed_text(available), ); - // The plan is priced per wave, but it is admitted as a whole, so what - // the phase leaves each of these operations is what each may run for. - // Holding them to the wave slot when the schedule is one operation wide - // spends a fraction of the phase and calls the rest a provider failure. - let widened = Duration::from_secs(available.as_secs() / u64::from(operations)) - .min(self.request_timeout) - .max(slot); + let scheduled = Duration::from_secs(available.as_secs() / u64::from(operations)) + .min(self.request_timeout); self.review_model_slot_secs - .store(widened.as_secs(), Ordering::Relaxed); + .store(scheduled.as_secs(), Ordering::Relaxed); Ok(()) } @@ -7223,8 +7223,8 @@ mod tests { .expect("a four-batch review must schedule"); let many_slot = many.scheduled_review_model_slot().unwrap(); assert!( - many_slot >= wave_slot, - "a scheduled slot may never fall below the slot the plan was priced at" + many_slot >= RETRY_ATTEMPT_RESERVE, + "a scheduled slot must leave time for one bounded attempt" ); assert!( many_slot @@ -7263,6 +7263,49 @@ mod tests { ); } + #[test] + fn complete_hosted_source_schedule_shares_the_existing_phase_budget() { + let _lock = env_lock().lock().unwrap(); + let config = Config { + scorer_enabled: true, + scorer: "openai/gpt-5.6-luna".into(), + ..Config::default() + }; + let budget = crate::review::hosted_review_timeout_secs(&config); + assert_eq!(budget, 240); + let mut client = LlmClient::build( + &config, + "test-key".into(), + Duration::from_secs(420), + Some(Instant::now() + Duration::from_secs(budget)), + Some(Instant::now() + Duration::from_secs(540)), + ) + .unwrap(); + client.review_model_timeout = Some(Duration::from_secs(60)); + assert!(max_hosted_review_batches(&config, false).unwrap() >= 14); + client + .ensure_hosted_review_schedule(&config, 14, false, 4) + .expect("four source waves must share the existing generation budget"); + let slot = client.scheduled_review_model_slot().unwrap(); + assert!(slot < Duration::from_secs(60)); + assert!(slot >= RETRY_ATTEMPT_RESERVE); + assert!(slot * 4 <= Duration::from_secs(budget - 30)); + let operation = client.for_review_model_operation(); + assert!( + operation + .remaining_budget(LlmPhase::Review) + .unwrap() + .unwrap() + <= slot + ); + client.review_deadline = Some(Instant::now() + Duration::from_secs(45)); + assert!( + client + .ensure_hosted_review_schedule(&config, 14, false, 4) + .is_err() + ); + } + #[test] fn local_from_env_keeps_the_original_480s_request_timeout() { let _lock = env_lock().lock().unwrap(); @@ -8619,7 +8662,7 @@ mod tests { ..Config::default() }; let capacity = max_hosted_review_batches(&config, true).unwrap(); - assert_eq!(capacity, 4); + assert_eq!(capacity, 13); let review_model_count = planned_review_models(&config).len(); let review_calls = capacity * review_model_count * MAX_LOGICAL_CALLS_PER_REVIEW_MODEL; let planner_calls = review_model_count * MAX_LOGICAL_CALLS_PER_REVIEW_MODEL; @@ -8656,15 +8699,15 @@ mod tests { ..Config::default() }; for (generator_count, scorer_count, without_planner, with_planner) in [ - (1, 0, Some(20), Some(16)), - (1, 1, Some(12), Some(8)), - (1, 2, Some(12), Some(8)), - (2, 0, Some(8), Some(6)), - (2, 1, Some(6), Some(2)), - (2, 2, Some(6), Some(2)), + (1, 0, Some(24), Some(23)), + (1, 1, Some(23), Some(22)), + (1, 2, Some(22), Some(21)), + (2, 0, Some(8), Some(7)), + (2, 1, Some(7), Some(6)), + (2, 2, Some(7), Some(6)), (3, 0, Some(3), Some(2)), - (3, 1, Some(2), None), - (3, 2, Some(2), None), + (3, 1, Some(2), Some(1)), + (3, 2, Some(2), Some(1)), ] { let config = config(generator_count, scorer_count); assert_eq!( @@ -8692,8 +8735,8 @@ mod tests { ..Config::default() }; - assert_eq!(max_hosted_review_batches(&config, false).unwrap(), 8); - assert_eq!(max_hosted_review_batches(&config, true).unwrap(), 4); + assert_eq!(max_hosted_review_batches(&config, false).unwrap(), 10); + assert_eq!(max_hosted_review_batches(&config, true).unwrap(), 9); } #[tokio::test] @@ -8755,7 +8798,7 @@ mod tests { } #[tokio::test] - async fn hosted_schedule_boundary_rejects_a_seventh_batch_wave_before_provider_contact() { + async fn hosted_schedule_rejects_a_wave_without_one_attempt_budget_before_provider_contact() { let server = wiremock::MockServer::start().await; let config = Config { api_base: server.uri(), @@ -8766,7 +8809,7 @@ mod tests { ..Config::default() }; let deadline = Instant::now() - + Duration::from_secs(crate::review::HOSTED_REVIEW_SCHEDULING_RESERVE_SECS + 361); + + Duration::from_secs(crate::review::HOSTED_REVIEW_SCHEDULING_RESERVE_SECS + 91); let mut client = LlmClient::build( &config, "test-key".into(), diff --git a/src/review.rs b/src/review.rs index 42779cc..87499ba 100644 --- a/src/review.rs +++ b/src/review.rs @@ -68,9 +68,9 @@ pub(crate) const HOSTED_LLM_TOTAL_TIMEOUT_SECS: u64 = 540; /// operation. Hosted review generation applies the shorter operation slot /// below across the primary attempt, retries, and correction call together. pub(crate) const HOSTED_LLM_REQUEST_TIMEOUT_SECS: u64 = 240; -/// Every hosted review-model operation, including retries and correction, is -/// bounded by this slot. Admission prices complete batch waves and sequential -/// cascades against the review-phase deadline. +/// Default hosted review-model slot before preflight. Admission divides the +/// remaining phase budget across complete batch waves and sequential cascades; +/// each operation shares its assigned slot with retries and correction. pub(crate) const LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS: u64 = 60; pub(crate) const HOSTED_LLM_REVIEW_TIMEOUT_SECS: u64 = 420; pub(crate) const HOSTED_REVIEW_SCHEDULING_RESERVE_SECS: u64 = 30; @@ -1823,7 +1823,11 @@ async fn review_diff_at( model_used = "none (empty diff)".to_string(); review_trust = filter::ReviewTrust::Exhaustive; } else { - let large_diff_receipt = (batches.count > large_diff_selected_limit) + let hosted_source_schedule = crate::config::hosted_runtime_mode() + && !args.bounded + && batches.source_count > MAX_HOSTED_SELECTED_BATCHES; + let large_diff_receipt = (batches.count > large_diff_selected_limit + || hosted_source_schedule) .then(|| batches.deterministic_bounded_receipt(large_diff_selected_limit)) .transpose()?; if let Some(receipt) = &large_diff_receipt { @@ -4462,7 +4466,7 @@ mod tests { assert_eq!(hosted_review_timeout_secs(&scorer_disabled), 360); assert_eq!( crate::llm::max_hosted_review_batches(&scorer_disabled, false).unwrap(), - 20 + 24 ); let scorer_enabled_budgets = hosted_review_phase_budgets(&scorer_enabled); @@ -4483,7 +4487,7 @@ mod tests { assert_eq!(hosted_review_timeout_secs(&scorer_enabled), 240); assert_eq!( crate::llm::max_hosted_review_batches(&scorer_enabled, false).unwrap(), - 12 + 23 ); } diff --git a/tests/e2e.rs b/tests/e2e.rs index 5741e09..98ea776 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -2737,6 +2737,98 @@ fn qualification_candidate_reports_incomplete_bounded_coverage_without_provider_ assert!(envelope.get("modelUsage").is_none()); } +#[cfg(feature = "qualification-candidate")] +#[tokio::test] +async fn hosted_source_coverage_extends_beyond_three_waves_without_raising_hard_bounds() { + use std::fmt::Write as _; + + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with(|_: &Request| { + let mut response = llm_content(json!([])); + response["model"] = json!("openai/gpt-5.6-luna"); + response["provider"] = json!("Azure"); + ResponseTemplate::new(200).set_body_json(response) + }) + .mount(&server) + .await; + + let directory = tempfile::tempdir().unwrap(); + let profile = directory.path().join("profile.json"); + std::fs::write(&profile, include_str!("../provisional-models.json")).unwrap(); + let mut source = String::new(); + for file in 0..14 { + let path = format!("src/auth/permission-{file}.ts"); + writeln!( + source, + "diff --git a/{path} b/{path}\n--- a/{path}\n+++ b/{path}\n@@ -1 +1,80 @@\n-if (!actor.can('admin')) throw new Error('Forbidden');", + ) + .unwrap(); + for line in 0..80 { + writeln!( + source, + "+const permission_{file}_{line} = '{}';", + "x".repeat(250) + ) + .unwrap(); + } + } + let diff = directory.path().join("source.diff"); + std::fs::write(&diff, source).unwrap(); + let output = postil() + .current_dir(directory.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) + .env("POSTIL_QUALIFICATION_CAPTURE_API_BASE", server.uri()) + .env("POSTIL_ALLOW_PRIVATE_API_BASE", "1") + .args(["review", "--diff-file"]) + .arg(&diff) + .args(["--output", "json"]) + .assert() + .success(); + let envelope: Value = serde_json::from_slice(&output.get_output().stdout).unwrap(); + let coverage = &envelope["reviewCoverage"]; + assert_eq!(coverage["mode"], "bounded"); + assert_eq!( + coverage["selectedBatches"], + 14, + "{coverage}; stderr={}", + String::from_utf8_lossy(&output.get_output().stderr) + ); + assert_eq!(coverage["totalBatches"], 14); + assert_eq!(coverage["receipt"]["directHunks"], 14); + assert_eq!(coverage["receipt"]["unreviewedHunks"], 0); + assert_eq!(envelope["gate"]["failing"], false); + assert!(envelope["findings"].as_array().unwrap().is_empty()); + assert!( + envelope["reviewAdmission"]["projectedCostMicros"] + .as_u64() + .unwrap() + <= 25_000_000 + ); + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 14); + for file in 0..14 { + let marker = format!("permission_{file}_79"); + assert_eq!( + requests + .iter() + .filter(|request| String::from_utf8_lossy(&request.body).contains(&marker)) + .count(), + 1, + "each source file must reach exactly one model request: {marker}" + ); + } + assert!( + requests + .iter() + .all(|request| request.body.len() <= 256 * 1024) + ); +} + #[cfg(feature = "qualification-candidate")] #[test] fn qualification_candidate_splits_json_escaped_batches_within_model_context() { @@ -2784,12 +2876,12 @@ fn qualification_candidate_splits_json_escaped_batches_within_model_context() { .success(); let envelope: Value = serde_json::from_slice(&out.get_output().stdout).unwrap(); assert_eq!(envelope["reviewCoverage"]["mode"], "bounded"); - assert!( - envelope["reviewCoverage"]["selectedBatches"] - .as_u64() - .unwrap() - < envelope["reviewCoverage"]["totalBatches"].as_u64().unwrap() + assert!(envelope["reviewCoverage"]["totalBatches"].as_u64().unwrap() > 1); + assert_eq!( + envelope["reviewCoverage"]["selectedBatches"], + envelope["reviewCoverage"]["totalBatches"] ); + assert_eq!(envelope["reviewCoverage"]["receipt"]["unreviewedHunks"], 0); assert!( envelope["reviewAdmission"]["serializedInputBytes"] .as_u64()