Skip to content

Activate default LLM timeout budget - #50

Merged
morgaesis merged 5 commits into
mainfrom
fix/default-llm-total-budget
Jul 11, 2026
Merged

morgaesis merged 5 commits into
mainfrom
fix/default-llm-total-budget

Conversation

@morgaesis

@morgaesis morgaesis commented Jul 11, 2026 •

Copy link
Copy Markdown
Contributor

Adds a default total LLM request budget so hosted reviews stay inside the worker's outer watchdog even when no environment variable overrides it: 540 seconds total, reserving 120 seconds for the independent scorer and leaving a 60-second margin before the 600-second watchdog fires. Local and interactive runs are unaffected by default and keep no total budget unless POSTIL_LLM_TOTAL_TIMEOUT_SECS is explicitly set. Crate version bumped to 0.4.2, with new regression tests for the default budget and for a cascade retry expiring mid-request.

@postil-dev postil-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default LLM total timeout is now always active, including for local runs, which may unexpectedly break long-running local reviews that previously had no total timeout.

  • warn warn src/llm.rs:195 — Default total timeout now applies to local runs · confidence 0.90 · kind: humanEscalation

Model: deepseek/deepseek-v4-pro

Comment thread src/llm.rs Outdated
@morgaesis
morgaesis force-pushed the fix/default-llm-total-budget branch 4 times, most recently from 37d9585 to 7eab70f Compare July 11, 2026 23:28

@postil-dev postil-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README incorrectly states that every review (local or hosted) is subject to a default 540-second total timeout, but the code applies that default only to hosted runs; local reviews have no total timeout unless the environment variable is set. This false claim could mislead users.

  • error error README.md:201 — README falsely claims default total timeout applies to local reviews · confidence 0.90 · kind: contentPolicy
  • warn warn src/llm.rs:195 — Default total timeout now applies to local runs · confidence 0.90 · kind: humanEscalation

Model: deepseek/deepseek-v4-pro

Comment thread README.md Outdated

@postil-dev postil-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default per-request LLM timeout was reduced from 480s to 420s, which may cause timeouts for large diffs in local runs where no total budget is active. This change is not mentioned in the PR description and could be an unintended regression.

  • warn warn src/llm.rs:144 — Default per-request LLM timeout reduced from 480s to 420s · confidence 0.70 · kind: humanEscalation
  • error error README.md:201 — README falsely claims default total timeout applies to local reviews · confidence 0.90 · kind: contentPolicy
  • warn warn src/llm.rs:195 — Default total timeout now applies to local runs · confidence 0.90 · kind: humanEscalation

Model: deepseek/deepseek-v4-pro

Comment thread src/llm.rs Outdated

@postil-dev postil-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change reduces DEFAULT_REQUEST_TIMEOUT_SECS from 480 to 420 seconds for all execution modes. This may cause previously passing long-running local/CI reviews to time out. Confirm this is intentional.

  • warn warn src/llm.rs:144 — Default request timeout quietly reduced from 480 to 420 · confidence 0.80 · kind: risk
  • error error README.md:201 — README falsely claims default total timeout applies to local reviews · confidence 0.90 · kind: contentPolicy
  • warn warn src/llm.rs:195 — Default total timeout now applies to local runs · confidence 0.90 · kind: humanEscalation

Model: deepseek/deepseek-v4-pro

Comment thread src/llm.rs Outdated
@morgaesis
morgaesis force-pushed the fix/default-llm-total-budget branch from c050ed8 to d6e3dd6 Compare July 11, 2026 23:32
@morgaesis
morgaesis force-pushed the fix/default-llm-total-budget branch from d6e3dd6 to 7854d36 Compare July 11, 2026 23:35

@postil-dev postil-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new default LLM timeout budget is introduced for hosted remote reviews, with regression coverage added. The PR title and description contain a content-policy violation.

  • error error .postil/pr-description:2 — remove fabricated claim · confidence 0.25 · kind: uncertainty

pass 3 finding(s) from the previous review resolved.

Model: mistralai/mistral-small-3.2-24b-instruct

The per-request timeout default had dropped from 480s to 420s
universally, including for local/interactive runs that have no total
budget to fit within -- 420s only makes sense for the hosted path
(540s total - 120s scorer). Threads a request-timeout default through
from_env_for_remote_review alongside the existing total-timeout
default, matching the split already used for the total budget: local
keeps 480s, hosted uses 420s explicitly via HOSTED_LLM_REQUEST_TIMEOUT_SECS.

@postil-dev postil-dev Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A doc comment on HOSTED_LLM_REQUEST_TIMEOUT_SECS contains an arithmetic contradiction (claims 4*120=480s fits within 420s budget).

  • error error src/review.rs:32 — Doc comment contradicts constant value · confidence 0.95 · kind: contentPolicy

Model: deepseek/deepseek-v4-pro

Comment thread src/review.rs Outdated
The comment claimed the 420s value came from '4 models * 120s', which
doesn't match the actual constant or its real derivation (540s total
minus the 120s scorer reserve = 420s).
@morgaesis
morgaesis merged commit c6c87b6 into main Jul 11, 2026
8 checks passed
@morgaesis
morgaesis deleted the fix/default-llm-total-budget branch July 11, 2026 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant