nisse-review: a timed-out model call shouldn't throw away the whole review #174

New Issue

2026-10-20T01:15:34Z

dev-agent commented

2026-10-20 01:15:34 +00:00

Seen twice:

#166 at 17:02 on 2026-10-19;
#171 at 01:06 on 2026-10-20, after 48 minutes of reviewing.

Both times, one model call passed llm.request_timeout_s (600 s) on qwen3.8-27b. A late turn on a large PR, with a long context, can take over 10 minutes. The run was marked failed, and the next one started from scratch.

Done now (bring-up config only): the reviewer's request_timeout_s is 1200. LiteLLM sets no timeout for that model. llama-server has its own read/write timeout (600 s by default), which may be the next limit. If a failure still comes at about 10 minutes, but with a different error, look there. That's on the inference host.

Proposed

Retry a timed-out call once (or twice), instead of failing the run on the first timeout. Today models/llm.py deliberately doesn't retry timeouts ("a hung model has had its request_timeout_s already"). For a slow model that is still working, one retry is cheap compared with losing the run.
Resume a failed run from its transcript. The canonical events are already stored. A run that failed on a model error could continue from its last completed turn instead of starting over, keeping its drafted comments and points. This needs care: the PR's head may have moved, which forces a fresh start.
Revisit the default DEFAULT_REQUEST_TIMEOUT_S (600) for the reviewer's local model, or make it per role.
Longer term: why do late turns get so slow? Prompt processing on a long context without a cache hit, perhaps. That links to --cache-ram after the RAM upgrade.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk

**Seen twice:** - #166 at 17:02 on 2026-10-19; - #171 at 01:06 on 2026-10-20, after 48 minutes of reviewing. Both times, one model call passed `llm.request_timeout_s` (600 s) on qwen3.8-27b. A late turn on a large PR, with a long context, can take over 10 minutes. The run was marked failed, and the next one started from scratch. **Done now (bring-up config only):** the reviewer's `request_timeout_s` is 1200. LiteLLM sets no timeout for that model. llama-server has its own read/write timeout (600 s by default), which may be the next limit. If a failure still comes at about 10 minutes, but with a different error, look there. That's on the inference host. **Proposed** 1. **Retry a timed-out call** once (or twice), instead of failing the run on the first timeout. Today `models/llm.py` deliberately doesn't retry timeouts ("a hung model has had its request_timeout_s already"). For a slow model that is still working, one retry is cheap compared with losing the run. 2. **Resume a failed run from its transcript.** The canonical events are already stored. A run that failed on a model error could continue from its last completed turn instead of starting over, keeping its drafted comments and points. This needs care: the PR's head may have moved, which forces a fresh start. 3. **Revisit the default** `DEFAULT_REQUEST_TIMEOUT_S` (600) for the reviewer's local model, or make it per role. 4. Longer term: why do late turns get so slow? Prompt processing on a long context without a cache hit, perhaps. That links to `--cache-ram` after the RAM upgrade. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk

dev-agent referenced this issue

2026-10-20 23:07:19 +00:00

Log every model call's time and tokens #175

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dev/nisse#174