Removing the implicit 90 s model timeout leaves review runner occupancy unbounded — needs an admission/continuation bound first

[seonghobae]

The situation

contextual-orchestrator#1118 removed ModelClient.__init__'s implicit per-request wall-clock timeout: timeout: int = 90 at the currently vendored pin 414f2297, timeout: float | None = None at 012beaac. That removal is correct — an implicit 90-second cap on the inference path is exactly what ADR-0003's 2026-08-31 amendment forbids and what #1889/#1890/#1892 were reverted for reintroducing. It is not the problem. It is, however, load-bearing today in a way nobody planned for, and this issue is about what has to exist before it goes away.

What the 90 s cap is currently doing

Production evidence, #1884 head e85fc437, run 34732993973, sidecar stderr artifact 10310273053. The serving phase ran 02:41:17Z → 02:56:27Z (907.3 s) and ended HTTP 503. Inside that window the gateway made 15 provider attempts across two agents on two credential accounts:

10x  nvidia_nim     / deepseek-ai/deepseek-v4-flash-0731
 5x  nvidia_nim_sub / deepseek-ai/deepseek-v4-pro-0813

Measured attempt→failure durations:

  90.0s   90.0s   22.0s   90.0s   90.0s
  90.0s   90.0s   90.0s   90.0s    0.0s

Eight of ten land on exactly 90.0 s. That is not the providers answering — it is ModelClient's default timeout firing, over and over. Roughly 720 of the 907 seconds is the cap expiring against endpoints that never respond.

Circuit breaker in the same window: circuit_failure ×10, circuit_opened ×2, circuit_reset ×2, circuit_cleared ×1. The breaker opens at failures=3.0 threshold=3, resets 30 s later, and the ranking hands the same route straight back.

Why removing the cap makes this worse before it makes it better

At the target pin, nothing bounds a single attempt:

• contextual_orchestrator_review_launcher.py:1240 builds the serving client as ModelClient(max_output_tokens=..., temperature=...) — no timeout argument, so it takes the library default.
• noema-review.yml:261 states "No job-level timeout-minutes here, deliberately."

So a socket that accepts a connection and then delivers nothing has no bound below GitHub's 6-hour job ceiling. Today's failure mode — 15 minutes, bounded, with a clear 503 and a full attempt trace — becomes an indefinite runner hold with one open attempt and no verdict. That is a worse operational outcome even though the change causing it is the correct one.

What must NOT be done

Re-adding a wall-clock timeout. docs/product-goal-directive.md §8 accepts that a model path may take more than two hours and states speed is not a core consideration; #1889, #1890, and #1892 each added a 900-second cap on genuine multi-hour-hang evidence and were all reverted (#1891, #1895). The directive names the alternative explicitly: fix runner occupancy at the admission/continuation boundary instead; never convert elapsed inference time into a model-failure verdict. This issue is not a request to reopen that.

What to build instead

The distinction the reverted PRs collapsed, and which this evidence makes concrete: a model streaming tokens for two hours is healthy; a socket that has delivered zero bytes for fifteen minutes is not inference at all. The bound belongs on progress, not on total elapsed time:

1. Socket-idle bound, not a deadline. Reset on every byte received. A response actively streaming is never interrupted regardless of total duration; a connection that has produced nothing since the request was written is a black hole and can be abandoned without any claim about how long the model "should" take.
2. Continuation admission. Once the breaker has opened on an agent, a reset should not immediately re-offer the same route while equally-ranked alternatives have not been tried. In this run, two ready routes (nvidia_nim/meta/llama-3.2-11b-vision-instruct and nvidia_nim_sub/meta/llama-3.2-11b-vision-instruct, both ready at preflight) were never attempted during serving while the two deepseek routes were cycled fifteen times.
3. Occupancy ceiling at the job boundary, not the inference boundary. If a runner must be released, release it as an explicit occupancy decision with its own event — never as a model-failure verdict attributed to the provider.

Item 1 needs an ADR, since "is an idle-socket bound a wall-clock timeout?" is exactly the question the reverts turned on and it should be settled in writing before code.

Sequencing

#2137 advances the pin to 012beaac and therefore carries this exposure. I have flagged it there rather than merging past it. Options, for the owner to decide:

• land the occupancy bound first, then #2137;
• land #2137 and accept a window where a hung provider can hold a review runner for up to 6 hours;
• land #2137 with an explicit timeout= at the launcher's serving call site — which would just reinstate the reverted cap under a different name, and I am not proposing it.

Found while root-causing #1884's noema-review failure. Refs #1915, #2000, #2137, ContextualWisdomLab/contextual-orchestrator#1118.

[seonghobae]

ADR drafted as #2140 (docs/adr/0030-review-runner-occupancy-progress-bound.md, Proposed, docs-only, 3039 tests passing).

It settles the question this issue said had to be settled in writing first: an idle-socket bound is not a wall-clock deadline, because it cannot expire on a request that is making progress. That is the distinction #1889/#1890/#1892 collapsed, and leaving it to code review is how all three happened.

Carries the three items from this issue as the decision — idle-socket bound reset on every byte, continuation admission so a breaker reset does not re-offer the failing agent while preflight-ready alternatives sit untried, and runner reclamation as its own job-boundary event — plus the #1884 trace as evidence and the explicit non-goals (no total-duration limit; no threshold value until time-to-first-byte is measured).

One thing the ADR makes sharper than this issue did: the current 90 s cap is not merely load-bearing, it is also doing harm. The same trace has google/gemma-4-31b-it rejected at preflight after exactly 90.0 s, which is indistinguishable from a model that needed 91 seconds. So the org holds both defects simultaneously — a cap that discards slow-but-working routes, and no bound at all on a dead socket once it is removed. That is an argument for landing the replacement rather than for keeping the cap.

Item 2 (continuation admission) is independent of the timeout question entirely and is the cheaper half; it would have shortened the 907-second run on its own, since two ready routes on the same accounts were never tried once. If item 1 stays contentious, item 2 can still go.

---

Generated by Claude Code

[seonghobae]

Fresh production evidence strengthens this issue beyond the earlier single-run trace. After protected #2163 advanced the central review sidecar to contextual-orchestrator@767e67fbc6b881a452761f32abb69b9971b9b03b (null default model timeout), #712 measured 32 in-progress jobs and 113 queued with no self-hosted runners; 31 in-progress review jobs were still current-head work and therefore not valid cancellation targets. The longest holders were spending roughly 3.5–5+ hours in Provision contextual-orchestrator review sidecar.

This separates two causal repairs that should not be collapsed into a timeout rollback:

1. preflight/provision startup latency: existing .github#1629 already has a provider-account concurrency implementation and synchronization-barrier regression so independent credential accounts can probe concurrently while same-account routes remain serial; its current branch is stale/non-mergeable and has its own no-heuristic RED, so it must be repaired/reconciled rather than copied into a competing branch;
2. transport/continuation occupancy: this issue/docs(adr): bound review runner occupancy by progress, not elapsed inference time #2140 owns progress-based idle/continuation/runner-reclamation semantics. A total elapsed model deadline remains prohibited.

The observed queue starvation is therefore no longer hypothetical exposure from removing 90 seconds: long-running current-head review provisioning is consuming the finite hosted concurrency pool and preventing unrelated exact-head jobs from acquiring runners. Acceptance for this lane should include unchanged-head evidence that review provisioning makes bounded forward progress/reclaims occupancy without converting elapsed inference time into provider failure, plus #712 evidence that unrelated required jobs can again acquire a runner. Final provider admission/routing/TTC ownership still belongs to contextual-orchestrator#1106 and the immutable release migration tracked by #1759.

[seonghobae]

four-pillars consumer-side saturation evidence, 2026-09-14T08:12Z

Measured from a consumer repository, showing the occupancy problem has now reached scheduled sentinels and fresh PRs, not just review jobs.

Organization .github at 08:12:54Z:

in_progress = 23
queued      = 100   (API page cap; actual backlog is at least this)

ContextualWisdomLab/four-pillars at the same moment — 11 runs queued, none started:

created (UTC)	run	state
04:56:51	Hourly Product Quality Loop (schedule)	queued 3h16m, never started
05:29:22	Hourly NVIDIA NIM Product Development (schedule)	queued 2h43m, never started
06:34:00–06:34:02	9 runs for PR #46 head 4187b0f (CI, CodeQL, and the required-workflow set)	queued 1h39m, every check still pending at 0s

Two consequences worth separating from ordinary latency:

1. The read-only quality sentinel has not executed since 00:28Z. The minute-17 loop is the repository's standing regression detector; while it is starved, a real regression on main would go unreported and the loop's failure-issue lifecycle cannot open or close anything.
2. A new PR cannot obtain any verdict. PR Ignore transient governance helper cancellations #46 opened at 06:34Z and none of its checks have started, so its required checks are neither passing nor failing — they are absent. That is distinct from the 502/429 transport failures tracked in noema-review fails closed on provider transport errors (502/429) after gateway failover; blocks unchanged consumer PRs #2165 and from the CodeQL wake-credential defect in CodeQL dispatch terminal status publication remains unproven across repositories #1929; here no job is dispatched at all.

This is consistent with the mechanism described in this issue: long-lived serving phases hold runner slots, and with no admission or continuation bound the queue drains slower than it fills, so the backlog crosses from review workloads into unrelated scheduled and PR workloads in consumer repositories.

No action requested from four-pillars' side; recording the consumer-visible blast radius as input to the admission-bound design.