fix(orchestrator): bound route_once's combined retry/failover wall-clock

[seonghobae]

Draft PR body (contextual-orchestrator)

Root cause (verified against contextual-orchestrator#946's four consecutive

noema-review TimeoutError: timed out failures)

scripts/ci/noema_review_gate.py's call_llm() (in ContextualWisdomLab/.github)
issues exactly one HTTP request per review, opener.open(request, timeout=120),
against the vendored contextual-orchestrator review sidecar's
/v1/chat/completions. That sidecar's launcher
(scripts/ci/contextual_orchestrator_review_launcher.py) constructs its
serving ModelClient/TaskOrchestrator with zero explicit overrides:

client = ModelClient(
    max_output_tokens=REVIEW_MAX_OUTPUT_TOKENS,
    temperature=REVIEW_TEMPERATURE,
)
orchestrator = TaskOrchestrator(agents, client=client)

unlike the same file's deliberately-bounded preflight client a few lines
above it (timeout=REVIEW_PREFLIGHT_TIMEOUT_SECONDS=10, max_retries=0). That
means the serving path runs on ModelClient's general-purpose defaults
(timeout=90, max_retries=2) and TaskOrchestrator's default
tool_retry_attempts=1 -- none tuned to fit inside the caller's fixed 120s
budget.

Enumerated worst case (current main, exact line numbers)

1. ModelClient._send_with_retry (orchestrator.py:1603): up to
   max_retries + 1 = 3 HTTP attempts per chat() call for any exception
   is_transient_error() (orchestrator.py:1102) classifies transient --
   which explicitly includes TimeoutError/socket.timeout
   (orchestrator.py:1118). Backoff between attempts is _backoff_delay
   (orchestrator.py:1648): uniform(0, min(8.0, 0.5 * 2**attempt)), ≤1.5s
   total across 2 backoffs. Worst case per chat() call: 3×90 + 1.5 =
   271.5s -- already 2.26x the external 120s budget from this layer alone.
2. _invoke's same-agent tool retry (orchestrator.py:6502-..., the
   layer route_once actually calls): a ProviderUpstreamError(retryable=True)
   -- which is exactly what classify_provider_failure (provider_errors.py:265)
   returns for TimeoutError/URLError/ConnectionError -- is classified by
   classify_provider_transport_failure (tool_fallback.py:124-159) as
   RETRY_SAME_AGENT. Bounded by
   retry_limit = min(self.tool_retry_attempts, MAX_TOOL_RETRY_ATTEMPTS)
   (orchestrator.py:6606); with the launcher's unchanged default
   tool_retry_attempts=1, retry_limit=1, i.e. 2 full chat() calls per
   agent before failover. Worst case per agent: 2×271.5 + ~0.25s backoff
   ≈ 543.25s.
3. _invoke's cross-candidate failover (orchestrator.py:6614,
   for agent in candidates:): tries every eligible agent in the pool with
   no combined ceiling. The review sidecar's own catalog is bounded to
   ORCHESTRATOR_CATALOG_LIMIT/REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES = 12
   (contextual_orchestrator_review_launcher.py:47,
   contextual_orchestrator_review_sidecar.sh:38). Worst case for one
   _invoke() call, every candidate failing: 12×543.25 ≈ 6519s.

Even without layers 2-3, layer 1 alone (a single agent, one retry) is
2×90 = 180s > 120s. This matches the observed symptom exactly: the
sidecar's own preflight step uses a genuinely bounded client
(timeout=10s, max_retries=0) and a tiny prompt, so it correctly reports a
route "ready"; the same route, hit with the real review's much larger
payload (diff up to MAX_DIFF_CHARS=60000 chars + context up to
MAX_REVIEW_CONTEXT_CHARS=24000 chars, vs. the preflight's ~60-character
probe) under the serving client's un-tuned defaults, can legitimately need
more than one 90s attempt -- and every such retry silently consumes budget
the external 120s caller has no visibility into and no way to recover from.

Both hypotheses in the investigation prompt are real and compounding:
this is a genuine layered-retry-vs-external-timeout mismatch (confirmed), and
it is worsened by a real preflight-vs-serving payload-size gap (also
confirmed) that makes a single attempt more likely to approach or exceed the
90s per-attempt ceiling in the first place, triggering the retry cascade
above.

Live evidence (job logs, not speculation)

Pulled directly from the most recent noema-review failure on #946 (run
33382752122, job 99458567351, vendored pin
8cd99f139915131ba0239bce12a5d6a5fd85394e):

• The sidecar's own preflight probed all 12 catalog routes and found 2
  ready, 10 rejected (7 TimeoutError, 3 HTTPError 404) -- matching the
  investigation prompt's "2-3 ready routes out of 12" exactly.
• python3 -m scripts.ci.noema_review_gate step started at
  2026-08-31T10:57:24.216Z; the TimeoutError: timed out traceback (from
  noema_review_gate.py:656's opener.open(request, timeout=120)) landed
  at 2026-08-31T10:59:31.681Z -- 127.46s total step wall-clock. That
  time also covers this step's own several gh api/gh graphql calls
  (fetching the PR, diff, changed files, review-thread context) before
  call_llm() ever opens the sidecar connection, so the actual serving
  request's own in-flight time was somewhat under 127.46s but consistent
  with a single attempt (or one attempt plus the start of a second) running
  into the 90-120s range -- the minimal end of the enumerated worst case
  above, not the full 12-candidate cascade. This is exactly what "layer 1
  alone already exceeds 120s" predicts, and does not require layers 2-3 to
  explain this specific occurrence (though they make the tail far worse).
• main()'s top-level handler only catches RuntimeError
  (noema_review_gate.py:826); TimeoutError is an OSError subtype, not
  a RuntimeError, so this failure surfaces as a raw, uncaught Python
  traceback rather than the gate's own clean, redacted error path -- a
  secondary, unrelated diagnosability gap in the same function (not fixed
  by this PR; noted for whoever picks up the .github-side change below).

Related prior art

ContextualWisdomLab/.github#1415 ("fix(noema): batch sidecar route
preflight", open, blocked/unmerged, conflicts with #1422) already gives the
serving ModelClient timeout=120, max_retries=0 -- removing layer 1's
compounding. That alone is not sufficient: with tool_retry_attempts
left at its default 1, layer 2 (_invoke's own same-agent retry) still
produces 2×120 = 240s worst case for a single agent, still 2x the external
120s budget, before layer 3 (cross-candidate failover) is even considered.

The fix

This repository cannot itself force the review sidecar's actual behavior
inside the 120s window: the values that need to change
(ModelClient(timeout=..., max_retries=...) and
TaskOrchestrator(..., tool_retry_attempts=...)) are constructed entirely
inside ContextualWisdomLab/.github's
scripts/ci/contextual_orchestrator_review_launcher.py, which this PR does
not touch. I attempted to open a companion PR there and was blocked: my
session's write access to that repository was explicitly denied (see below).

What this PR does add, entirely inside contextual-orchestrator, is the
missing primitive that a caller with a fixed external deadline needs and
does not currently have: an opt-in, additive deadline_seconds parameter on
TaskOrchestrator.route_once() (threaded into _invoke() as an absolute
deadline) that bounds the combined wall-clock time across every layer
above (same-agent retry and cross-candidate failover) to a caller-chosen
ceiling, instead of leaving it unbounded. Left None (the default), every
existing caller's behavior is byte-for-byte unchanged -- confirmed by the
full existing test suite passing unmodified.

This intentionally does not touch:

• ModelClient's or TaskOrchestrator's existing default values (timeout,
  max_retries, tool_retry_attempts) -- these are general-purpose product
  defaults consumed by other callers (gyeot, scopeweave) I have no
  visibility into changing unilaterally.
• The immediate_race capability-equivalence branch inside _invoke, which
  already has its own independent deadline_seconds=self.client.timeout
  bound on race_first_valid and does not yet observe the new parameter
  (documented as a known gap in the docstring).

What still needs to happen in ContextualWisdomLab/.github (blocked, not done here)

For PR #946's actual CI symptom to stop recurring, a follow-up change is
needed in scripts/ci/contextual_orchestrator_review_launcher.py's serving
client construction, e.g.:

client = ModelClient(
    max_output_tokens=REVIEW_MAX_OUTPUT_TOKENS,
    temperature=REVIEW_TEMPERATURE,
    timeout=REVIEW_SERVING_TIMEOUT_SECONDS,   # new, e.g. 120
    max_retries=0,
)
orchestrator = TaskOrchestrator(
    agents,
    client=client,
    tool_retry_attempts=0,   # eliminates the redundant same-agent retry
    # once this repo's `deadline_seconds` reaches a vendored pin that has
    # it, route_once(messages, deadline_seconds=<budget below 120s>) is the
    # more precise fix than tool_retry_attempts=0 alone, since it still
    # allows cross-candidate failover within the remaining budget.
)

and, since even a single 120s attempt with zero internal retry cannot
guarantee completion strictly under noema_review_gate.py's own 120s
transport read (there is no margin left for TLS/JSON/GC overhead once the
provider call itself is allowed to run the full 120s), noema_review_gate.py
's opener.open(request, timeout=120) should also grow a small margin
(e.g. 130-150s) to stop failing closed on legitimate near-the-limit
completions -- again derived from the (now much smaller, single-attempt)
internal worst case, not guessed.

I could not make this change myself: attaching ContextualWisdomLab/.github
with push access was explicitly denied by this session's policy classifier.
A maintainer or a session with write access to that repository should apply
it, ideally reviving/rebasing #1415 with the tool_retry_attempts=0 (or
deadline_seconds) addition and the noema_review_gate.py timeout bump
above.

Verification

• python -m pytest tests -q --ignore=tests/test_psychometric_routing.py
  (that one file has a pre-existing, unrelated missing-numpy import gap in
  this sandbox): 2863 passed, 1 skipped in 733.83s.
• coverage run -m pytest ...: same pass count under instrumentation
  (746.04s); interrogate contextual_orchestrator/orchestrator.py: 100%
  (109/109).
• New/changed tests, all passing:
  • tests/test_tool_execution_fallback.py: 11 new deadline cases
    (already-expired deadline before the first attempt, cross-candidate
    cutoff, same-agent-retry cutoff, a generous deadline that doesn't
    interfere, and 7 invalid-deadline_seconds-value rejections).
  • tests/test_measured_routing_evidence.py: 1 new case for route_once's
    own outer judge-driven-failover loop (distinct code path from
    _invoke's cross-candidate/same-agent loops above).
  • Every pre-existing test in both files still passes unmodified.
• git diff --check: clean.

Refs: contextual-orchestrator#946, ContextualWisdomLab/.github#1415,
ContextualWisdomLab/.github#1455, ContextualWisdomLab/.github#1449.

---

Generated by Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 7 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bcf03f16-2516-408a-8817-8c435276e1bb

📥 Commits

Reviewing files that changed from the base of the PR and between c6c3a0c and 8b60676.

📒 Files selected for processing (8)

• .gitignore
• CHANGELOG.md
• contextual_orchestrator/__init__.py
• contextual_orchestrator/orchestrator.py
• docs/adr/0001-tool-execution-fallback-policy.md
• docs/architecture.md
• tests/test_measured_routing_evidence.py
• tests/test_tool_execution_fallback.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[seonghobae]

Picked up the .github-side companion fix your investigation identified — pushed to .github#1415 as 7e07a4e9: tool_retry_attempts=0 and dataclasses.replace(orchestrator.policy, realtime_judge=False) on the serving orchestrator, plus a noema_review_gate.py external timeout bump to CALL_LLM_TIMEOUT_SECONDS=3000 derived from the same enumeration. Also found one more compounding factor your investigation's _send_with_retry/_invoke enumeration didn't explicitly flag: TaskOrchestrator's default policy.realtime_judge=True makes route_once issue a second, independent provider call per candidate to judge the first one's answer — doubling worst-case wall-clock again on top of the retry/failover layers you enumerated. Disabled it for the serving orchestrator specifically (safe here since this sidecar is a fresh, one-shot process with no long-lived quality-ledger learning to preserve). Full details in the .github#1415 comment.

This PR's own deadline_seconds addition is still the more precise long-term primitive once vendored — this immediate fix just closes the gap without waiting for that vendoring round-trip.

---

Generated by Claude Code

[seonghobae]

Update from the .github-side work you flagged as blocked: ContextualWisdomLab/.github#1415 now has a complete fix for the root cause you enumerated here (the serving ModelClient/TaskOrchestrator construction with untuned defaults vs. noema_review_gate.py's fixed external timeout). The final shape differs from what you sketched:

• tool_retry_attempts and policy.realtime_judge are both left at their defaults (not tool_retry_attempts=0) — a Devin Review finding on that PR correctly caught that disabling realtime_judge (and, independently, that tool_retry_attempts=0 also collapses route_once's own outer cross-candidate loop to 1 attempt) breaks real per-request quality gating and judge-rejection failover, not just future-routing learning.
• Instead, a new REVIEW_SERVING_MAX_CANDIDATES=10 caps how many preflight-verified candidates the serving orchestrator draws from (separate from preflight's own larger admission-testing pool), and noema_review_gate.py's external timeout is derived backwards from noema-review.yml's own timeout-minutes: 360 job ceiling: CALL_LLM_TIMEOUT_SECONDS=9600 (up from the original 120).

Your deadline_seconds primitive on route_once()/_invoke() is still a more precise long-term fix than a client-count cap, and worth reviewing/merging on its own architectural merits regardless of the .github-side fix landing — I haven't evaluated it in depth.

Separately: this PR's own noema-review check just failed with a different symptom than #946's original bug — curl: (28) Operation timed out ... 0 bytes received on the gateway preflight smoke request, after healthz/provider-route preflight had already succeeded (job 99481544912). That looks like the sidecar process becoming unresponsive after startup, not the LLM-call timeout mismatch this PR is about, and I haven't investigated it — flagging rather than diagnosing, since this PR is a draft with no immediate urgency.

---

Generated by Claude Code

[seonghobae]

Follow-up on this round's 4 findings (job fc65dc7e.../b8e1fe25...):

• 🔴 "Ranking defeats the route deadline" / "Pre-dispatch work escapes deadline": partially addressed already-pushed (28cdde8): route_once now fails closed with an explicit error instead of silently returning an empty "accepted": False payload when ranking alone exhausts the deadline. The deeper ask — actually bounding _ranked_agents's own execution time (propagating the deadline into any semantic-affinity embedding call, or skipping that work when time is short) — is a separate, larger change than this PR's scope: it touches a method used by every routing path (conduct, tool-fallback candidate selection, etc.), not just this opt-in deadline_seconds feature, and this ADR's own invariant ("a deadline never interrupts an attempt already in flight") means cutting it off mid-flight isn't a small patch either. Tracking as follow-up, not attempting here.

• 🔴 "Busy local judges overrun deadlines": fixed (28cdde8), confirmed resolved by your own follow-up comment.

• 🟡 "Deadlines enlarge provider attempt timeouts": fixed (28cdde8) — chat()/_send_raw_with_retry now cap each attempt at min(self.timeout, remaining deadline).

• 🔍 "Boundary completions lose accounting" + "Deadline errors lack one contract": both addressed together just now — introduced RouteDeadlineExceededError(RuntimeError) for route_once's own three direct deadline raises (backward-compatible: existing except RuntimeError callers, including this repo's own tests matching on RuntimeError, are unaffected since it's a subclass). Its .trace attribute carries the already-recorded trace rows, so a worker completion that landed just before the deadline (and so already incurred real, billable cost) is recoverable by the caller instead of disappearing with the exception. Left _invoke's own deeper deadline RuntimeErrors unchanged — unifying those touches every other _invoke caller, not just this feature.

• 🔍 "Research artifact needs follow-up": added an explicit redistribution-permissibility note to the Dean & Barroso citation (it's a Communications of the ACM article, not an arXiv preprint — ACM retains copyright, so cite+link+summary without vendoring a PDF, matching docs/papers/README.md's existing "PDFs are cited rather than vendored when redistribution is unclear" policy).

New regression test (test_route_once_deadline_exceeded_error_preserves_completed_worker_trace) plus the existing suite, all green. Pushing shortly.

---

Generated by Claude Code

[seonghobae]

Synced to the last few rounds of commits here. 146bb228 ("enforce route deadline boundaries") fully subsumed the judge_usage-in-trace fix I had in progress locally (I discarded my redundant diff via git reset --hard rather than push a no-op), and additionally closed two gaps I had explicitly deferred as out of this PR's original scope: propagating the deadline into _ranked_agents's embedding-based ranking calls, and normalizing every _invoke exception (not just route_once's own three direct raises) to RouteDeadlineExceededError once the deadline has passed. Ran the full local suite against 146bb228: 2876 passed, 1 skipped, 0 failed (PYTHONPATH=. python -m pytest tests -q --ignore=tests/test_psychometric_routing.py, 716s). By the time that run finished, 2b33acfe ("bound deadline resolver capacity") had already landed too — from the diff, that closes the DNS-thread-leak and missing-package-root-export findings from the same review round.

Given the pace here (a new comprehensive commit roughly every 15-40 minutes, each closing several Devin findings faster than my own edit→12-minute-full-suite→push cycle can keep up with without repeatedly producing redundant, discarded work), I'm stepping back from racing to fix each new individual finding on this specific file myself. You're clearly driving this precisely and quickly. I'll keep watching CI status and stay available if a finding stalls or you want a second pair of hands on something specific.

---

Generated by Claude Code

[seonghobae]

Cross-PR integration contract: routing identity is provider-neutral model_group only; do not add or preserve a provider-family abstraction. OpenRouter discovery must retain concrete free model IDs, while the aggregate openrouter/free router is not a serving candidate. OpenCode, Noema, and Strix must call contextual-orchestrator. Do not impose fixed wall-clock deadlines on inference, initial ping, readiness/health, provider discovery, or OpenRouter ZDR-list retrieval; use explicit cancellation and evidence-backed transport failure instead. Reconcile this PR with #971 and central .github #1508 before merge.

[seonghobae]

Flagging a possible tension with the owner's cross-PR architectural contract (posted on #979): "Do not impose fixed wall-clock deadlines on inference, initial ping, readiness/health, provider discovery, or OpenRouter ZDR-list retrieval; use explicit cancellation and evidence-backed transport failure instead."

This PR's deadline_seconds parameter on route_once()/_invoke() is, by design, an opt-in fixed wall-clock deadline mechanism — even though it defaults to None and doesn't change any existing caller's behavior today, its whole purpose is for a future caller (e.g. the noema-review launcher) to opt into a bounded ceiling. #971 ("fix(routing): select concrete free model groups") takes the architecturally different approach the owner is now asking for: it removes fixed wall-clock deadlines from model inference, initial completion probes, and equivalent-endpoint races, and keys routing identity on model_group rather than provider grouping.

Not asking you to change course mid-flight — just making sure this reaches you before deadline_seconds gets wired into a real caller, since #971 may make it unnecessary (or the two approaches may need to be reconciled with each other, similar to the #979/#971 reconciliation the owner asked for). Recorded in ContextualWisdomLab/.github#1530.

---

Generated by Claude Code

[seonghobae]

Superseded by #971 and the organization no-wall-clock-deadline policy. route_once(deadline_seconds=...) and propagated provider/DNS/read deadlines directly contradict the required unbounded default for inference, retry, ping, readiness, discovery, and ZDR lookup. Preserve trace/accounting ideas only in a future cancellation-token design if needed; do not merge this elapsed-time implementation.