You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(combo): honor recovery for pre-content transport failures - #18610
⚠️ base-red inherited: #17219 — upstream’s release-branch failure tracker is still open.
Summary
A structured transport read error before useful output can be accepted by combo
quality validation, returning a broken stream instead of using retry or fallback.
For streaming requests with existing recovery enabled, recognize four transport codes and use
existing attempt budgets across priority, round-robin, execute-mode model units
and context-cache pins. Preserve default passthrough, cancellation, pins/ACLs and
observed output, terminal and error boundaries. Bounded inspection retains passthrough
when it cannot safely decide. Already-started output and clean EOF keep their behavior.
#18258 handles the separate cooldown retry path; #15725’s default policy remains.
Validation
Routing fix; active release/v3.8.53, original base 689b1eb.
Original frozen reproduction: 343 pass/266 fail on pristine base; 609 pass with fix.
Fresh focused regressions: 322/322, natural exit.
Earlier affected selection on unchanged code: 96/96 (repository force-exit).
Scoped lint, formatting, whitespace and normal Git hooks pass.
The additional test-only commit runs shared tables once, retaining real wiring
checks on every route and specialized cancellation, pin and cleanup cases.
Production code is unchanged. Tests remain in the four
tests/unit/combo-precontent-transport-recovery*.test.ts files.
An earlier broader selection had a known cleanup timeout: 150 pass, 1 cancelled.
Neither affected selection was rerun for the final revision.
Full repository checks, coverage, typecheck and build remain for CI; no live-provider
or production tests.
…complexity ratchets
Split the scanner frame table out of the 1447-line recovery test (322 cases kept), extract dispatchHonoredPin/recoveryAborted from tryPinnedModelDispatch, observeCompletedReplayLines from SseOutputScanner and inspectReadFailureHop from classifyPreContentReadFailure. No behavior change.
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Auto incremental reviews are disabled on this repository.
Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
No actionable comments were generated in the recent review.
Summary
Summary by CodeRabbit
Bug Fixes
Streaming combo requests can now recover from recognized connection-read failures that occur before usable output, retrying within configured limits.
Recovery avoids retrying once output or other response content indicates the stream has progressed.
Client cancellations are handled promptly during dispatch and retries, without triggering additional provider attempts or failure counts.
Existing retry behavior for non-streaming requests remains unchanged.
Combo streaming routes now support bounded retries for recognized pre-content transport read failures when stream recovery is enabled. SSE inspection records replay vetoes, quality validation determines whether a failure is retryable, and route handlers stop dispatching when the client disconnects.
Changes: Combo stream recovery
Layer / File(s)
Summary
Transport classification and replay inspection open-sse/services/combo/preContentTransportFailure.ts, open-sse/services/combo/firstOutputStream.ts, tests/unit/combo-precontent-transport-recovery-scanner.test.ts, tests/unit/combo-precontent-transport-recovery-framing.test.ts
Bounded error-chain classification and SSE frame inspection identify transport failures and replay vetoes. Scanner tests cover framing, inspection limits, and observer behavior.
Quality validation and retryable read failures open-sse/services/combo/validateQuality.ts, tests/unit/combo-precontent-transport-recovery.test.ts
Opt-in quality validation treats pre-content transport failures as retryable only when no content, terminator, replay boundary, or replay veto has been observed. It also cleans up peek readers.
Recovery and cancellation across combo routes open-sse/services/combo.ts, open-sse/services/combo/{dispatchPrelude,executeTargetAttempt,roundRobinCombo,runtimeUnits}.ts, tests/unit/combo-precontent-transport-recovery*.test.ts, stryker.conf.json, changelog.d/fixes/0-combo-precontent-transport-recovery.md
Priority, round-robin, runtime, and pinned dispatch paths pass recovery context into quality validation, check cancellation, and use cooldown-aware waits after transport read failures. Tests cover retry policy, cancellation, and non-streaming behavior.
cheaper-inference/OmniRoute#18258: Adds cooldown-aware waiting for authorized combo retries, which this change uses after transport read failures.
Merge Risk: Minimal · previous review ffed1
The change adds bounded retries for transport read failures that occur before any usable streamed output, and only when stream recovery is enabled. Default behavior, non-streaming requests, and streams that have already started output are unchanged. No actionable merge-blocking issue was found. The author's remaining checks (full CI, typecheck, build) are normal pre-merge steps.
Security architecture risk: Low · up to ffed1
Recovery is opt-in and restricted to recognized failures before usable output or replay-sensitive events. Existing attempt limits and cancellation checks contain its impact. No introduced security defect was established, although access-control enforcement on an existing pinned-request path remains incompletely verified.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The changed outcome is additional dispatch of an existing request to its current target or configured fallback targets after an eligible stream failure. Inspected consumers retain their existing retry limits, including priority routing's global attempt accounting; recovery does not introduce a new target resolver.
Trust Boundaries and Controls
observed — Normal target execution still evaluates gates before entering the attempt loop, including credential and per-target admission checks. The new cancellation wrapper delegates to the existing handler rather than granting additional dispatch authority.
observed — The pinned fast path's direct model dispatch predates this PR. Pin membership and durable-health eligibility remain in place, including the existing exception when the combo collection is not authoritative. This comparison does not establish independent connection-ACL or admission enforcement inside the direct handler.
Resilience and Maintainability Implications
observed — Recovery-mode validation cancels and releases its peek reader on abort or read failure. Discarding callers cancel response branches and return 499 when the request is aborted. Replay cancellation records its terminal state before cancelling the reader, preventing an outstanding read from subsequently enqueueing data.
Pre-merge checks: 6 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 36.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 6 passed
Check name
Status
Explanation
Title check
The title uses the required Conventional Commits format, fix(combo): subject, and accurately describes the pre-content transport recovery change.
Description check
The description clearly covers the user-facing change, target branch, validation results, tests, known cleanup timeout, and reviewer-relevant limitations. It does not use every template heading and om…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Tests For Production Code
PASS. The PR changes production code under open-sse/. It also adds five focused test files under tests/unit/, with imports and assertions covering combo routing, validateResponseQuality, SseOutputScan…
No Ai Attribution
PASS. The eight commit messages contain only Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> trailers. This identifies a human contributor and is allowed. No AI or bot a…
Full details: Description check
Explanation
The description clearly covers the user-facing change, target branch, validation results, tests, known cleanup timeout, and reviewer-relevant limitations. It does not use every template heading and omits explicit Related Issues, Coverage Notes, and Reviewer Notes sections, but the required information is mostly present.
Full details: Tests For Production Code
Explanation
PASS. The PR changes production code under open-sse/. It also adds five focused test files under tests/unit/, with imports and assertions covering combo routing, validateResponseQuality, SseOutputScanner, and pre-content transport recovery. This satisfies the stated requirement to add or update tests when production code changes.
Full details: No Ai Attribution
Explanation
PASS. The eight commit messages contain only Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> trailers. This identifies a human contributor and is allowed. No AI or bot attribution and no Generated with <AI tool> footer appears in the commit messages. The authored PR body also contains no prohibited trailer or footer.
Full details: Docstring Coverage
Explanation
Docstring coverage is 36.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 36 functions across 13 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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
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
A structured transport read error before useful output can be accepted by combo
quality validation, returning a broken stream instead of using retry or fallback.
For streaming requests with existing recovery enabled, recognize four transport codes and use
existing attempt budgets across priority, round-robin, execute-mode model units
and context-cache pins. Preserve default passthrough, cancellation, pins/ACLs and
observed output, terminal and error boundaries. Bounded inspection retains passthrough
when it cannot safely decide. Already-started output and clean EOF keep their behavior.
#18258 handles the separate cooldown retry path; #15725’s default policy remains.
Validation
The additional test-only commit runs shared tables once, retaining real wiring
checks on every route and specialized cancellation, pin and cleanup cases.
Production code is unchanged. Tests remain in the four
tests/unit/combo-precontent-transport-recovery*.test.ts files.
An earlier broader selection had a known cleanup timeout: 150 pass, 1 cancelled.
Neither affected selection was rerun for the final revision.
Full repository checks, coverage, typecheck and build remain for CI; no live-provider
or production tests.