emrg: vibe check no timeout — wait for the completion judgment - #900
Conversation
Rant 2026-08-20T20:19:31: the 20s deadline in _request_vibe_check drops the work/reason data when concurrent tasks hold the LLM (a single vibe check call can exceed 20s). The vibe check is the completion judgment right after a finished cycle — waiting longer for an accurate result beats dropping it. Remove the deadline; the daemon's LLM call keeps its own retry/timeout and a dead daemon raises ConnectionClosed, so the wait is bounded in practice.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 1 (1 file +10/−9: 20s deadline removed from _request_vibe_check — now waits for vibe_check_result without a cap; bounded in practice by the daemon's own LLM retry/timeout + ConnectionClosed on daemon death; caller still conservatively leaves the slowdown state unchanged. Scheduler suite 82/82, pytest 983 + 1 skip; test + test-windows CI PASS)
|
Independent test report (cycle 2026-08-20, Contributor): |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 2 (fresh re-review: checkout + full diff verified; 20s deadline removed, while True waits for vibe_check_result, only ConnectionClosed breaks — bounded by daemon LLM retry/timeout + daemon-death close. Scheduler 82/82 + full pytest 983 passed + 1 skipped locally; CI test + test-windows PASS)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 3 (head unchanged since cycle-2 review: 94353f9, scheduler.py no-timeout vibe check; scheduler 82/82 + pytest 983 passed locally; CI test + test-windows PASS). 3 consecutive LGTMs from cycles 204400/205713/210909 — merging.
Remove the 20s deadline in
_request_vibe_check(scheduler.py), per host rant 2026-08-20T20:19:31.Root cause: the 20s timeout drops work/reason data when concurrent tasks hold the LLM — a single vibe check LLM call can exceed 20s (observed in opencode-opensource-task: empty
workwhile the task actually completed with 20 tools). The vibe check is the completion judgment right after a finished cycle; waiting longer for an accurate result beats dropping it.Change: the scheduler now waits for the
vibe_check_resultframe without a deadline. The wait stays bounded in practice — the daemon's LLM call (llm.chat) has its own retry/timeout and returnsok: falseon failure, and a dead daemon raisesConnectionClosedwhich breaks the loop (the caller conservatively leaves the slowdown state unchanged, as before).Tests: scheduler suite 82/82 (no-frame case now breaks via ConnectionClosed; structured-result case unchanged); pytest 983 passed + 1 skipped; import + CLI OK.