docs(gap-baseline): root-cause item 4's underlying provider-pool stall - #1804
Closed
seonghobae wants to merge 8 commits into
Closed
docs(gap-baseline): root-cause item 4's underlying provider-pool stall#1804seonghobae wants to merge 8 commits into
seonghobae wants to merge 8 commits into
Conversation
Four same-day cross-repo data points (fast-mlsirm#1518, .github#1689, naruon#1539, mightyETL#330) show noema-review's gateway stall growing 649.5s -> 1332.6s -> 1462.9s -> 2161.9s. Traced through contextual-orchestrator's TaskOrchestrator._invoke: the free-tier candidate pool failover loop has no fast-fail path for a candidate recently known to be failing, so pool saturation compounds as serial per-candidate timeout budget rather than a bounded worst case -- and each stalled run holds an Actions job slot for 20-36 minutes, worsening the same 60-job ceiling this doc's queue-congestion entries already track. Proposes a direction (skip via #911's routing observations, or race top candidates) without implementing it -- that's shared production routing code in contextual-orchestrator with a 100% coverage bar and deserves its own PR and owner sign-off.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 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. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
… claim contextual-orchestrator#1028's own noema-review check hit the identical gateway stall (duration=2296.6s, the largest yet) while triaging an unrelated autofix event -- notable because it's contextual-orchestrator's own CI blocked by contextual-orchestrator's own gateway saturation. Also corrects an overclaim in the first draft: the repo's real fail-under=100 coverage gate is scoped to nim_benchmark.py, not the whole package, verified against .github/workflows/ci.yml directly.
…already exist and are structurally inert Traced the actual code: a real circuit breaker (_record_failure/ _circuit_open, wired into _failover_candidates) and a real concurrent racing path (_equivalent_race_members) both already exist in contextual-orchestrator. Neither can help the observed stall: the circuit breaker is in-memory per-process state, but the review sidecar boots a fresh process per CI job and makes exactly one request before exiting, so there's never a second request to benefit from a circuit that only opens after 3 accumulated failures. #911's routing-observation data has the identical structural problem -- also in-memory, also reset per process -- so the skip-fast design this entry originally proposed would have been just as inert, not a fix. Racing is inert for a different reason: it requires candidates to share a group_name and a declared endpoint_equivalence contract proving they're redundant replicas of the same model; the free-tier pool has neither -- its members are different models entirely, so racing them is a real speed-vs-quality product tradeoff, not a mechanical extension. Splits the revised direction into two genuinely different fixes (persist state across the ephemeral process boundary, vs. extend racing to non-equivalent candidates with an explicit tradeoff decision) instead of the two originally proposed, neither of which would have worked.
…911 contextual-orchestrator#911 (open, unmerged) already builds real, tested cross-process persistence infrastructure (routing_observation_ store.py, SQLite-backed, time-windowed) -- exactly what item 4's "persist state across the ephemeral process boundary" direction calls for. But it wires that store into the ranking ledger (_group_router), which per this entry's own earlier finding is never consulted for the free-tier pool's ungrouped candidates anyway. The mechanism that *is* consulted regardless of grouping -- the circuit breaker (self._circuit) -- isn't touched by #911 at all. Flags the concrete next step (route the circuit breaker through the same already-built store) without implementing it, since #911 itself is still unmerged and could change shape before landing.
…strator#1032, 2604.3s)
…and stale corrected-premise text
- Retry math was wrong: the 286s worst-case only counted ModelClient's
own 3-attempt internal retry, missing that _invoke's outer
RETRY_SAME_AGENT loop (default tool_retry_attempts=1) re-invokes the
whole 3-attempt call up to once more -- 6 total HTTP attempts per
candidate, not 3, worst case ~540s not 286s. Verified directly
against contextual-orchestrator's source (classify_provider_
transport_failure always returns RETRY_SAME_AGENT for a retryable
transport failure). Updated the illustrative candidate-count math to
match.
- The "Why this is a real gap" paragraph still asserted the original,
since-superseded premise (that the loop simply doesn't consult
existing tracked state) even though a later "Correction" section in
the same entry explains that consulting it wouldn't have helped
anyway, for a deeper reason (ephemeral per-process state). Added an
explicit forward-pointer so the two sections don't read as
contradicting each other.
- Restated the one HTTP 500 exception directly on its own bullet
(previously only in the intro sentence) to remove any ambiguity a
reader scanning the bullets alone might have.
- Fixed two stale counts left over from earlier revisions of this
entry ("five" runs/"four" repositories -> six runs/five repositories,
matching the two data points added after this text was first written).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…strator#1028 later push, 3913.0s)
Contributor
Author
|
Superseded by the implementation-bearing ContextualWisdomLab/contextual-orchestrator#1049, which now has fresh exact-head checks against the current central workflows. This docs-only branch conflicts with the newer baseline, its historical runtime-quality failure came from unrelated pre-existing main regressions, and keeping its queued checks consumes capacity without delivering the gateway fix. The RCA remains recoverable from this PR history. |
github-project-automation
Bot
moved this from In Progress
to Done
in naruon Platform Roadmap
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.
Summary
fast-mlsirm#1518, 649.5s stall) explicitly left the underlying provider-pool stall itself unresolved. This adds three more same-day, cross-repo data points showing the duration growing (649.5s -> 1332.6s -> 1462.9s -> 2161.9s acrossfast-mlsirm#1518,naruon#1539,.github#1689,mightyETL#330) and traces the root cause throughcontextual-orchestrator'sTaskOrchestrator._invokefailover loop..github#1689's own sidecar preflight) is walked serially with no fast-fail path for a candidate recently known to be failing --#911's routing-observation EWMA data reorders candidates but doesn't exclude a saturated one, so each stalled request pays close to the full per-candidate retry/timeout budget (ModelClientdefaults:timeout=90s,max_retries=2, backoff cap8s) for however many pool members are simultaneously slow rather than fast-failing.#911's observation data, or race top candidates concurrently -- arace_memberspath already exists a few lines above the serial loop for a different condition) without implementing it: this is shared production routing code incontextual-orchestratorwith a 100% coverage bar and needs its own PR, tests, and owner sign-off givendocs/product-goal-directive.mdsection 8's explicit no-fixed-inference-cap policy.Test plan
docs/product-technical-gap-baseline.md; no code/workflow touched.contextual-orchestrator@464da4715b495b5eaaa593eba3796e2d976ee0c9source (not paraphrased from memory).served_model=unknownfindings there are unchanged).🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com