From d29de8fb2c30888f62c7703601d18864e5fefc71 Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Tue, 18 Aug 2026 21:50:20 +0000 Subject: [PATCH 1/2] Give a one-wave hosted review its whole generator phase The per-model slot exists so a large review's batch waves all fit inside the generator phase, but it was pinned to the large-diff constant for every hosted review rather than to the timeout the review itself selected. A single-wave review therefore ran under a many-wave bound: each model call was cut at sixty seconds, and because the slot then equalled the phase remainder the timeout retry was unreachable, so one slow answer ended the review with a provider-unavailable finding. The review model's own latency sits well above that cut, so a tenth of hosted reviews were failing on answers that were still arriving. --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/llm.rs | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bde1bc4..00e9541 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1104,7 +1104,7 @@ checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" [[package]] name = "postil-cli" -version = "0.8.23" +version = "0.8.24" dependencies = [ "aho-corasick", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index a5b4324..27dc18f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "postil-cli" -version = "0.8.23" +version = "0.8.24" 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/src/llm.rs b/src/llm.rs index 7d97673..54f1742 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -2632,9 +2632,14 @@ impl LlmClient { review_deadline, total_deadline, )?; - client.review_model_timeout = Some(timeouts.request.min(Duration::from_secs( - crate::review::LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS, - ))); + // The slot bounds one model operation so a review's batch waves all fit + // inside the generator phase. `default_request_timeout` already carries + // the per-review answer: the short large-diff slot when the review is + // spread across many waves, the full hosted request timeout when it is + // not. Pinning the slot to the large-diff constant instead applies a + // many-wave bound to a single-wave review, which caps every hosted model + // call at that constant and makes any slower answer terminal. + client.review_model_timeout = Some(timeouts.request.min(default_request_timeout)); Ok(client) } @@ -6710,7 +6715,7 @@ mod tests { assert_eq!( client.review_model_timeout, Some(Duration::from_secs( - crate::review::LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS + crate::review::HOSTED_LLM_REQUEST_TIMEOUT_SECS )) ); let remaining = client.remaining_budget(LlmPhase::Total).unwrap().unwrap(); @@ -6719,6 +6724,65 @@ mod tests { assert!(remaining > Duration::from_secs(crate::review::HOSTED_LLM_TOTAL_TIMEOUT_SECS - 5)); } + /// A single-wave hosted review must get its whole generator phase for the + /// one model call it makes. Bounding it to the many-wave slot caps every + /// hosted review at that slot, and because the slot then equals the phase + /// remainder the timeout retry is unreachable, so one slow answer is + /// terminal. + #[test] + fn single_wave_hosted_reviews_keep_the_whole_generator_phase() { + let _lock = env_lock().lock().unwrap(); + let _env = EnvRestore::capture(&[REQUEST_TIMEOUT_ENV, TOTAL_TIMEOUT_ENV, "POSTIL_API_KEY"]); + EnvRestore::remove(TOTAL_TIMEOUT_ENV); + EnvRestore::set("POSTIL_API_KEY", "test-key"); + // The hosted worker raises the request timeout above both per-review + // defaults, so the slot has to come from the review's own choice. + EnvRestore::set(REQUEST_TIMEOUT_ENV, "420"); + + let config = Config::default(); + let review_budget = Duration::from_secs(crate::review::hosted_review_timeout_secs(&config)); + + let ordinary = LlmClient::from_env_for_remote_review( + &config, + Instant::now(), + Duration::from_secs(crate::review::HOSTED_LLM_REQUEST_TIMEOUT_SECS), + review_budget, + Duration::from_secs(crate::review::HOSTED_LLM_TOTAL_TIMEOUT_SECS), + ) + .unwrap(); + assert_eq!( + ordinary.review_model_timeout, + Some(Duration::from_secs( + crate::review::HOSTED_LLM_REQUEST_TIMEOUT_SECS + )) + ); + assert!( + ordinary.review_model_timeout.unwrap() + > Duration::from_secs(crate::review::LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS), + "a one-wave review must not inherit the many-wave slot" + ); + assert!( + ordinary.review_model_timeout.unwrap() <= review_budget, + "the slot must stay inside the generator phase it is spent from" + ); + + let large = LlmClient::from_env_for_remote_review( + &config, + Instant::now(), + Duration::from_secs(crate::review::LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS), + review_budget, + Duration::from_secs(crate::review::HOSTED_LLM_TOTAL_TIMEOUT_SECS), + ) + .unwrap(); + assert_eq!( + large.review_model_timeout, + Some(Duration::from_secs( + crate::review::LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS + )), + "many-wave reviews still need the short slot so every wave fits" + ); + } + #[test] fn local_from_env_has_no_total_deadline_without_env_override() { let _lock = env_lock().lock().unwrap(); From ade3a6f767e05908f73c6c089b8dfe3889fc24cf Mon Sep 17 00:00:00 2001 From: Postil Maintainer Date: Tue, 18 Aug 2026 22:02:23 +0000 Subject: [PATCH 2/2] Size the single-wave slot to fit its scheduling reserve The generator phase admits a model operation only when the slot still fits the deadline that remains once the diff has been fetched, less the scheduling reserve. A slot set to the full hosted request timeout equals the phase itself and is refused before any provider request, so the single-wave slot is its own bound, above the review model's tail latency and clear of the reserve. --- src/llm.rs | 27 ++++++++++++++++----------- src/review.rs | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/llm.rs b/src/llm.rs index 54f1742..6c48f4f 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -2633,13 +2633,13 @@ impl LlmClient { total_deadline, )?; // The slot bounds one model operation so a review's batch waves all fit - // inside the generator phase. `default_request_timeout` already carries - // the per-review answer: the short large-diff slot when the review is - // spread across many waves, the full hosted request timeout when it is - // not. Pinning the slot to the large-diff constant instead applies a - // many-wave bound to a single-wave review, which caps every hosted model - // call at that constant and makes any slower answer terminal. - client.review_model_timeout = Some(timeouts.request.min(default_request_timeout)); + // inside the generator phase. Pinning it to the large-diff slot applies + // a many-wave bound to every hosted review: a single-wave review is cut + // at that slot, and because the slot then equals the phase remainder + // the timeout retry is unreachable, so one slow answer is terminal. + client.review_model_timeout = Some(timeouts.request.min(Duration::from_secs( + crate::review::single_wave_request_timeout_secs(default_request_timeout.as_secs()), + ))); Ok(client) } @@ -6715,7 +6715,7 @@ mod tests { assert_eq!( client.review_model_timeout, Some(Duration::from_secs( - crate::review::HOSTED_LLM_REQUEST_TIMEOUT_SECS + crate::review::SINGLE_WAVE_LLM_REQUEST_TIMEOUT_SECS )) ); let remaining = client.remaining_budget(LlmPhase::Total).unwrap().unwrap(); @@ -6753,7 +6753,7 @@ mod tests { assert_eq!( ordinary.review_model_timeout, Some(Duration::from_secs( - crate::review::HOSTED_LLM_REQUEST_TIMEOUT_SECS + crate::review::SINGLE_WAVE_LLM_REQUEST_TIMEOUT_SECS )) ); assert!( @@ -6761,9 +6761,14 @@ mod tests { > Duration::from_secs(crate::review::LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS), "a one-wave review must not inherit the many-wave slot" ); + // The schedule check runs once the diff is fetched, against whatever is + // left of the phase then, so the slot needs headroom under the reserve + // rather than to merely equal it. assert!( - ordinary.review_model_timeout.unwrap() <= review_budget, - "the slot must stay inside the generator phase it is spent from" + ordinary.review_model_timeout.unwrap() + < review_budget + - Duration::from_secs(crate::review::HOSTED_REVIEW_SCHEDULING_RESERVE_SECS), + "the slot must leave the generator phase room for its scheduling reserve" ); let large = LlmClient::from_env_for_remote_review( diff --git a/src/review.rs b/src/review.rs index 6598d40..a4839fc 100644 --- a/src/review.rs +++ b/src/review.rs @@ -64,6 +64,14 @@ pub(crate) const HOSTED_LLM_REQUEST_TIMEOUT_SECS: u64 = 240; /// bounded by this slot. Admission prices complete batch waves and sequential /// cascades against the review-phase deadline. pub(crate) const LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS: u64 = 60; +/// The slot a review that fits in a single batch wave gets for its one model +/// operation. The large-diff slot above is sized so many waves fit; spending it +/// on a single wave ends the review the first time the model answers slower +/// than a fraction of the phase it was given. This sits above the review +/// model's observed tail latency and below the generator phase less its +/// scheduling reserve, which `ensure_hosted_review_schedule` enforces against +/// the deadline that actually remains once the diff has been fetched. +pub(crate) const SINGLE_WAVE_LLM_REQUEST_TIMEOUT_SECS: u64 = 180; pub(crate) const HOSTED_LLM_REVIEW_TIMEOUT_SECS: u64 = 420; pub(crate) const HOSTED_REVIEW_SCHEDULING_RESERVE_SECS: u64 = 30; const FORGE_READ_TIMEOUT_SECS: u64 = 60; @@ -157,6 +165,16 @@ pub(crate) fn large_diff_batch_concurrency(cfg: &Config) -> usize { (MAX_LARGE_DIFF_CONCURRENCY / consensus_width).max(1) } +/// A review spread across many waves keeps the slot each wave was priced +/// against; anything else is a single wave and gets the single-wave slot. +pub(crate) fn single_wave_request_timeout_secs(review_request_timeout_secs: u64) -> u64 { + if review_request_timeout_secs <= LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS { + review_request_timeout_secs + } else { + SINGLE_WAVE_LLM_REQUEST_TIMEOUT_SECS.min(review_request_timeout_secs) + } +} + fn hosted_request_timeout_secs(deterministic_large_review: bool) -> u64 { if deterministic_large_review { LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS