[linear3735]

Required human contributor answers

The contributor explicitly confirmed these five statements on 2026-10-29 after reviewing the existing #101 implementation and its deadline finding.

1. What outcome are you trying to achieve?

   Make the local served OneJev integration reliable enough to land.

2. What is the highest-leverage variable for that outcome?

   Whether one HTTP decision, including its permitted retry, respects the configured total time budget.

3. How will this work move that variable rather than create busywork?

   Fix the reproduced deadline reset in [Feat] Served OneJev multimodal decision backend (--model onejev) #101 with one shared deadline and regression tests, while reusing its existing implementation.

4. Who is the customer or user, and what do they actually need?

   People running browser agents against a local OneJev server need predictable timeouts and failure behavior.

After answering the four questions: is a PR needed at all?

Yes. The contributor confirmed submitting this small patch to the existing implementation; the earlier independent adapter remains a local reference.

Observed bug evidence: #101's deadline finding. The added regressions also fail on its head b0a341b6f8ca9addb6ef3744983dd29e251c614d: a retry receives a fresh 2-second budget after the first attempt consumes 1.25 seconds; exhausting the first budget still sends a second request.

Why

#101 documents ONEJEV_TIMEOUT_S as including the retry, but gives every HTTP attempt the full timeout. This can double the decision's HTTP time budget.

This PR targets issue-88-onejev-served, the branch behind #101, so the review diff contains only the deadline fix, its tests and the changelog entry. It contributes to #88 without introducing another OneJev backend.

How / What

OneJevClient.decide computes one monotonic deadline before the retry loop. Each attempt supplies the remaining budget to both HTTPX and asyncio.wait_for; an exhausted budget fails before constructing another request.

2.00 s decision budget
  first attempt consumes 1.25 s
  retry gets 0.75 s
  no remaining budget → MODEL_CALL_FAILED, no new request

The permitted retry count and conditions, request ID, explicit health check, wire mapping and last-attempt latency measurement are unchanged. This patch does not make the separate /health warmup part of the HTTP decision budget.

Three new test methods cover ConnectError / HTTP 502 / HTTP 504 retries, exhausted budgets, and cancellation of a stalled retry. Total diff: 3 files, 90 additions / 3 deletions; authored Python changes: 88 lines including tests.

Verification

Executed locally against this patch using the existing Python 3.11 contributor environment. Submitted commit: 5990c6888363fe3180634d8a420f9981fa9cb61b; the committed diff is byte-identical to the reviewed patch.

• ruff format --check ., ruff check ., ty check, git diff --check.
• python -m pytest -q: 1342 passed, 132 skipped, 258 subtests passed; 3 existing dependency deprecation warnings.
• scripts/smoke.sh: passed with this checkout on PYTHONPATH.
• CHANGELOG.md records the fix; the existing retry-inclusive timeout documentation now matches the HTTP decision loop.

The targeted adapter/factory/browser selection passed 69 tests, 2 skips, 18 subtests. Before the implementation change, the added deadline cases reported 7 failures (six subtest failures plus the stalled-retry test). They use a controlled monotonic clock and fake HTTP transport; the stalled retry also exercises actual asyncio cancellation.

These are CPU/client checks. No model, browser task, GPU performance or general task-quality result is claimed. CI results must be checked on the pushed head separately.

Independent Codex review found no blocker in this patch. Kimi independently checked the diff hash, reran the OneJev and related tests, and reproduced the seven failures against unmodified #101 code with the new tests. Its review also passed; it did not rerun the full suite or smoke check.

Demo / evidence

The deterministic regression tests are the reproduction for this timeout-only client fix. No new backend or application-success demonstration is introduced by this patch; #101 owns the broader integration and its demo disposition. This patch does not supply or claim the required application → Agents → Omni video.

Review order: s1a/decision_models/onejev.py, then tests/test_decision_models_onejev.py. The change is reversible and affects only the served OneJev HTTP retry budget. No merge is requested.

[Birfy]

Reviewed the diff against issue-88-onejev-served (#101's head b0a341b6). LGTM — approving.

Correctness

• decide now computes one monotonic deadline (self._clock() + self._timeout_s, using the injectable clock that already exists on OneJevClient) and passes the remaining budget to both the HTTPX timeout= and asyncio.wait_for, so the retry-inclusive ONEJEV_TIMEOUT_S documented in #101 is now actually enforced.
• The remaining <= 0 guard runs before a request is built, so an exhausted budget can't send a second request. Retry count/conditions, X-Request-Id reuse, error mapping and latency measurement are untouched.
• _no_answer(cause: Exception | None) typing change is correct; build_error already accepts cause=None.

Tests

• The three new cases are well targeted: remaining budget seen by both HTTPX (request.extensions["timeout"]) and wait_for for ConnectError/502/504, no retry once the budget is spent, and real asyncio cancellation of a stalled retry.
• Locally: ruff check / ruff format --check clean; tests/test_decision_models_onejev.py 34 passed, 2 skipped, 6 subtests; full suite 1226 passed, 132 skipped (two collection errors in the jev_vl_served modules are from optional deps missing in my env, unrelated to this diff). CI core 3.11/3.13 and benchmarks green, full still running.

Non-blocking nit

• When the first attempt gets a 502/504 (or ConnectError) and that attempt alone exhausts the budget, the caller now sees the generic "no answer within N s" error with cause=None, so the original HTTP status/transport error is lost. Consider carrying the last failure into _no_answer(...) so the logs still say why the first attempt failed. Fine as a follow-up.

Note: this targets #101's branch, so it lands in #101 rather than main; merging stays with the maintainer.