Skip to content

fix(harness): treat HTTP 408 timeouts as retryable and score exhausted timeouts as Skipped - #83

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1789150312-408-retryable-skipped
Open

fix(harness): treat HTTP 408 timeouts as retryable and score exhausted timeouts as Skipped#83
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1789150312-408-retryable-skipped

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

TL;DR

Community fix from @warun7 (authored commit preserved, brought in from warun7:benchmark-harness:fix/score-408-timeouts-as-skipped, follow-up to #77): ModelError with status 408 is now retryable, so timeouts go through rateLimitRetrySchedule and, if retries are exhausted, the sample scores Skipped instead of Incorrect, matching 429 and 5xx.

What changed?

  • isRetryableModelError in src/harness/core.ts adds error.status === 408.
  • Because rateLimitRetrySchedule and runBenchmark's ModelError handler both key off isRetryableModelError, 408s are retried (default 6 attempts, exponential jittered backoff from 1s) and exhausted 408s become ScoreValue.Skipped.
  • Tests: new src/harness/core.test.ts for the retryable/systemic classifiers, a runBenchmark case for exhausted 408 scoring Skipped, and a rateLimitRetrySchedule case for 408 retry.

Why?

Reported on Discord by warun7. Today a 408 is scored Incorrect, which counts a transport or timeout failure as a wrong answer in the accuracy denominator, unlike 429 and 5xx which are excluded as Skipped.

Searched existing PRs for "408", "timeout", "Skipped"; none matched.

How to test

bun test src/harness/core.test.ts src/harness/run.test.ts src/runtime/retry.test.ts

Expected: all pass, including "scores a sample as Skipped (excluded from accuracy) when the model exhausts 408 retries" and "retries transient 408 timeout errors".

Locally on this branch: bun run format:check, bun run check, bun run typecheck, bun test (1436 pass), bun run build all succeed.

Benchmark impact

This changes reported scores. Any run where a sample previously hit a 408 will now either recover on retry (adding to correct or incorrect) or be excluded from the accuracy denominator. Runs with no 408s are unaffected. Pre and post fix results for the same model are not directly comparable when timeouts occurred.

Two consequences reviewers should weigh, see below.

Reviewer focus

  • 408 is not only a server status here. src/providers/responses-model.ts synthesizes a 408 ModelError when the harness's own timeoutMs elapses. So this PR also changes the meaning of --solver-config '{"timeoutMs": N}' from "hard per-sample budget, failure counts against the model" to "per-attempt budget, retried up to 6 times, then excluded from accuracy". A model that consistently cannot answer within the budget is no longer penalized. Whether a timeout is an infrastructure failure or a model failure is a methodology decision that should be made explicitly.
  • Wall-clock. With timeoutMs set, worst case per sample grows from 1x to 7x timeoutMs plus backoff. On long agentic evals this is material.
  • Alternative if the team wants to keep client-side timeouts as model failures: give the synthesized client timeout a distinct marker (or status) and only make server-returned 408 retryable.

Checklist

  • Tests cover changed behavior
  • Public API or configuration changes are backward compatible, or the break is documented
  • Benchmark changes document dataset provenance and licensing (n/a)
  • No credentials, private results, or restricted dataset contents are included
  • Documentation is updated where needed (n/a)

Link to Devin session: https://openrouter.devinenterprise.com/sessions/e348d039d09045f3a3cc9fc485bf2cda
Open in Devin Desktop: https://openrouter.devinenterprise.com/desktop/session/e348d039d09045f3a3cc9fc485bf2cda?variant=devin
Requested by: @jasoncrabtree-or


Note

Devin errored when opening this Pull Request as jasoncrabtree-or.
As a fallback, Devin opened this PR as itself.

@devin-ai-integration
devin-ai-integration Bot requested a review from a team as a code owner September 11, 2026 18:12
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Original prompt from Jason

help me with this report from a discord user:

Hey team! I was going through ``@openrouter/bench-harness and found a few bugs in the CLI runner. I put together a fix with full test coverage:

1. **`--solver-config` ignored**: In `src/cli/index.ts`, `gpqa_diamond`, `mmlu_pro`, `mmmu_pro_vision`, and `ifstruct` dropped `panelConfig`. Flags like `providerOnly`, `maxTokens`,

timeoutMs, and sort were parsed but discarded. I routed them through buildSchemaValidatedConfig
2. Terminal cursor hidden on crash: Wrapped runBenchmarkById in try ... finally so bar.stop() always runs if an error occurs
3. NaN numeric flag handling: Added finite number checks for --concurrency, --limit, etc., to avoid NaN values

The repo has interaction limits on, so I could not open a PR directly. The branch and tests (1,409 passing) are ready on my fork:
👉 https://github.com/OpenRouterTeam/benchmark-harness/compare/main...warun7:benchmark-harness:fix/cli-solver-config-and-resilience

I also have a fix ready for HTTP 408 timeouts being scored as `Incorrect` instead of `Skipped` if helpful!

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