fix(benchmark): bound the LLM-judge call with --timeout - #155
Open
addyCooks wants to merge 3 commits into
Open
Conversation
`--timeout` is documented as the timeout per test and is enforced for inference, but not for the judge call that decides pass/fail. The AbortController the loop builds is cleared in a `finally` that runs before `callJudge` is ever reached, and `callJudge` took no signal to forward into `generateText` anyway. So a judge provider that accepts the connection and never answers — a hung local server, a slow backoff after a rate limit — hangs the whole benchmark on one test, with no progress indicator to tell "slow" from "dead" and no way out but killing the process. The AI SDK's default retry policy sits on top of that, re-issuing a request nobody is timing. Give `callJudge` an `abortSignal` and forward it into `generateText`. One signal covers the retries too: the SDK cancels the in-flight fetch, rethrows an abort without retrying it, and interrupts the backoff delay between attempts. Judging gets its own budget of `timeout` rather than sharing the inference one. It is a second model call, and the existing loop already spends one budget per call — with `--samples n` a test may take up to n timeouts. Extending the inference timer instead would leave the judge whatever inference did not use, so a slow-but-fine generation could starve the judge and flip a passing test to failed. A judge that never answered is not a verdict, so the sample fails and the run moves on, exactly as an inference timeout already does a few lines up. `judgeScore` is left unset and the reason recorded in `judgeReasoning` — "Judge timed out after Nms" — so the report does not read as the model having scored 0. Closes Nano-Collective#132
Both tests run against a local HTTP server, so they need no network and no live provider. The first stands up the endpoint the issue describes and cannot easily be reproduced by hand: one that accepts the connection and never replies. Strip the `abortSignal` back out of `generateText` and this test does not fail, it hangs until AVA's own timeout — which is the bug, stated as a test. The elapsed-time assertion is the point of it: "it threw" is not enough, because the SDK retries a failed call by default and an unaborted attempt would still be waiting rather than having given up. The second is the other half — a provider that answers inside its budget still gets scored normally, so the new optional parameter stays out of the way of the path everyone actually uses. It covers `callJudge` and `createJudgeProvider` end to end for the first time. Sockets held open by an unanswered request are destroyed in the teardown; without that the server never finishes closing and the worker never exits.
"Timeout per test" was the promise the flag did not keep for LLM-judge tests, and it is imprecise now that it does: the budget is spent per model call, so a judged test can spend it twice — once generating, once being scored — and an n-sample run spends it n times over.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
--timeoutis documented as the timeout per test and is enforced for inference,but not for the LLM-judge call that decides pass/fail. The
AbortControllerthebenchmark loop builds is cleared in a
finallythat runs beforecallJudgeisreached, and
callJudgetook no signal to forward intogenerateTextanyway. Ajudge provider that accepts the connection and never answers therefore hangs the
whole benchmark on one test, with no way out but killing the process — and the AI
SDK's default retry policy sits on top of that, re-issuing a request nobody is
timing.
callJudgenow takes an optionalabortSignaland forwards it intogenerateText. One signal covers the retries too: the SDK cancels the in-flightfetch, rethrows an abort without retrying it, and interrupts the backoff delay
between attempts, so no
maxRetriesoverride is needed.Judging gets its own budget of
timeoutrather than sharing the inference one.It is a second model call, and the loop already spends one budget per call (with
--samples na test may take up to n timeouts). Extending the inference timerinstead would leave the judge whatever generation didn't use, so a slow-but-fine
generation could starve the judge and flip a passing test to failed.
A judge that never answered is not a verdict: the sample fails and the run moves
on, exactly as an inference timeout already does a few lines up.
judgeScoreisleft unset and the reason recorded in
judgeReasoning("Judge timed out afterNms"), so the report doesn't read as the model having scored 0.
Closes #132
Type of Change
Testing
Automated Tests
pnpm test:allcompletes successfully)Manual Testing
nanotune initnanotune datacommands (add/import/list/validate)nanotune trainnanotune exportnanotune benchmarkChecklist
pnpm format)