Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

Dynamic Review wrap-up: time reserve, history growth, error codes (follow-up to #293)

Open
  • Is something wrong?
#301 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
15/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
typescript
Domain
ai, backend

Research direction

Start with the clock reserve and final-call logic in the proxy budget code and the deadline watch in the adapter, as the test table names them, along with FINAL_CALL_GROWTH_BYTES and docs/dynamic-review.md. Read T01 to T11 to see the expected behavior for each finding, and settle the F1 fix design (timeout clamping or a watch grace) before editing. Done means the listed tests pass and the documented window matches the code.

Written by the indexing model from the issue text.

Description

area: runner type: bug

Follow-up to #293 (merged in #294). Review findings F1 to F4 and the nit F5; nothing here blocks the release, and nothing in this issue is implemented yet.

Findings

F1 (major): the time-limit wrap-up is not guaranteed a usable final call. The clock trigger fires only when a request arrives with 150 s or less left. A normal call that starts at 151 s may run for up to 120 s (the upstream timeout is fixed, not clamped to the remaining time) and end with about 31 s left. The final call then gets about 31 s with an upstream timeout that is still 120 s, and the deadline watch ends the run at 0 s even if the final call is almost done. A slow reasoning model with a full output allowance hits this exactly in the case the wrap-up is meant for. The outcome is no worse than before (dynamic_budget_exceeded, no review), but the time wrap-up produces a review less often than the reserve suggests. Fix, either of: (1) clamp the upstream timeout of a normal call to remaining - reserve, so a normal call never eats the reserve, and of the final call to remaining + a small grace; (2) let the watch allow a short grace past the deadline while a final call is in flight (capped, for example at 60 s). Either way, document the real window.

F2 (minor): the cost trigger assumes a fixed 128 KiB of history growth. FINAL_CALL_GROWTH_BYTES assumes two tool results between this call and the final one. A single answer may hold more parallel tool calls (the total is capped at 4 MiB), and the assistant's own message is not counted. Near the limit the final call can then be larger than assumed, fail the "final call must fit" check and end with dynamic_budget_exceeded without a review. Fail-closed and rare (the slack between reserved and settled cost usually absorbs it). Fix: derive the growth from the request (the largest result seen so far times the number of tool calls in the last answer, plus the output allowance), or cap the tool calls per answer.

F3 (minor): a final call that returns prose is classified as a budget error. Every request after the final call is refused with dynamic_budget_exceeded and recorded as the run's failure. If the provider ignores the forced tool choice and answers in prose, the engine retries, the retry is refused, and the run ends with dynamic_budget_exceeded: the run asked for a call after its final call. llm_output_invalid is the right code; an operator will otherwise look at the cost limit instead of the model. Fix: a distinct detail, or map the refusal to llm_output_invalid when the final call itself returned 200 without a structured-output tool call.

F4 (minor): the history of the final call names tools that are no longer offered. Earlier assistant tool_calls for read, grep and glob stay in the messages while tools holds only the return tool. The fake and the real engine accept this; a real provider reached through OpenRouter may validate tool names in the history and answer 400 (llm_bad_request, no review, and the final call has cost money). No code change is required up front: watch the first production wrap-ups, make the final call visible in the result log, and if it happens keep the original tool definitions and rely on tool_choice instead.

F5 (nit): a request that cannot be rewritten still follows the plain rule near the limit. A request without the return tool (a title call, for instance) can spend headroom that the final call needs. Harmless in practice because the engine's tool set cannot produce such calls late. Either reserve the headroom for such requests too, or document it.

Test cases

ID kind level case expected
T01 behavior unit (proxy, budget) F1: a normal call starts shortly before the clock reserve its upstream timeout is clamped to the time left minus the reserve, so it cannot run into the reserve; with no time beyond the reserve the call is the final call
T02 behavior unit (proxy) F1: the final call its upstream timeout is at most the time left plus the grace
T03 behavior unit (adapter) F1: a final call is in flight when the deadline passes the watch waits for it for at most the grace, then ends the run with dynamic_budget_exceeded; a final call that finishes inside the grace is published
T04 regression engine (real OpenCode, fake model, fake clock) F1: a slow normal call near the reserve, then the final call the review completes and is marked time_limit
T05 behavior unit (budget) F2: the last answer holds many parallel tool calls near the cost limit the growth reserved for the final call follows the request, or the number of tool calls per answer is capped; a final call that is larger than a fixed 128 KiB still gets its review
T06 behavior unit (budget) F2: the growth assumption documented in docs/dynamic-review.md, with the real bound
T07 behavior unit (proxy) F3: the final call returns 200 without the return tool call, and the engine retries the run ends with llm_output_invalid, not dynamic_budget_exceeded
T08 regression unit (proxy) F3: the engine continues after a successful final call that did return the answer still refused with dynamic_budget_exceeded
T09 behavior unit (proxy, runner) F4: the provider answers 400 to the final call the run ends with llm_bad_request and the result log says it was the final call
T10 docs review F4: the observation docs/dynamic-review.md names the risk and the fallback (keep the original tool definitions)
T11 behavior unit (budget) F5: a request that cannot be rewritten near the limit it cannot spend the headroom the final call needs, or the behavior is documented and pinned by a test
Dominant language
TypeScript
Stars
0
Forks
1
Avg merge
1h 15m
Merged PRs (30d)
217

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ExpidetGames/code-reviewer

All issues in ExpidetGames/code-reviewer

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.