Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
73 changes: 71 additions & 2 deletions src/llm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2632,8 +2632,13 @@ impl LlmClient {
review_deadline,
total_deadline,
)?;
// The slot bounds one model operation so a review's batch waves all fit
// 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::LARGE_DIFF_LLM_REQUEST_TIMEOUT_SECS,
crate::review::single_wave_request_timeout_secs(default_request_timeout.as_secs()),
)));
Ok(client)
}
Expand Down Expand Up @@ -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::SINGLE_WAVE_LLM_REQUEST_TIMEOUT_SECS
))
);
let remaining = client.remaining_budget(LlmPhase::Total).unwrap().unwrap();
Expand All @@ -6719,6 +6724,70 @@ 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::SINGLE_WAVE_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"
);
// 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
- 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(
&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();
Expand Down
18 changes: 18 additions & 0 deletions src/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down