Skip to content

fix(sse): optionally retry once when a stream stalls before usable output - #14669

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.51from
maxmad64bis:fix/empty-body-server-rotation
Sep 24, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.51from
maxmad64bis:fix/empty-body-server-rotation

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #14547

Summary

No default change: opt-in via STREAM_READINESS_STALL_RETRY (default off). When enabled and the first upstream body stalls before producing a usable event, the request runs once more through the same routing path with the same readiness budget and no account penalty, and only fails if the second attempt also stays empty. With the flag off, a stalled first body fails exactly as before.

Related Issues

Validation

  • Change type: routing
  • Focused tests: tests/unit/stream-readiness-fallback.test.ts (9 tests), tests/unit/feature-flags-settings.test.ts (62 pass), tests/unit/server-owned-tool-loop-flag.test.ts (8 pass), tests/unit/feature-flags-doc-sync-static.test.ts (4 pass)
  • npm run lint — ESLint on the touched files is clean
  • Reconciled with release/v3.8.51 (rev-list HEAD..upstream/release/v3.8.51 = 0); focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • tests/unit/stream-readiness-fallback.test.ts (9 tests: flag off issues no second request; readiness timeout then retry success, early-EOF same, double failure keeps second diagnostic, aborted client never retries, single-request bound, neighbor empty-turn untouched plus cooldown spy, silent-stream timeout; RED on the guard revert, GREEN with the guard)
  • Neighbours replayed: empty-turn retry path untouched (no extra dispatch when the first attempt succeeds)

Coverage Notes

  • open-sse/handlers/chatCore/streamReadinessFallback.ts covered by the new test, including the flag-off guard; chatCore.ts wiring (import + single call + reassignments) covered via the same test.
  • src/shared/constants/featureFlagDefinitions.ts and src/shared/utils/featureFlags.ts covered by the flag-count suites (registry at 78, fail-closed helper).

Reviewer Notes

  • When enabled, the worst case waits two readiness windows; the retry goes through the executor's normal selection and does not exclude the stalled account.
  • chatCore.ts grows 6402 → 6426 (frozen, +24 wiring lines, own growth; rebaseline entry _rebaseline_2026_09_23_14669_empty_body_server_rotation included).
  • Inherited reds qualified as inherited (failures cite files outside this PR, unchanged since the merge-base, base-red issue 🔴 Release branch not green: release/v3.8.51 #14547, normalized failure lists identical to same-base siblings): API Route Typecheck errors (auggie.ts, projectCombo.ts, empty diff $mb, non-PR files); check-env-doc-sync live-data case cites DEEP_HEALTH_CHECK_ENABLED, present on the untouched base; full npm run lint red on the base (🔴 Release branch not green: release/v3.8.51 #14547).

@maxmad64bis
maxmad64bis force-pushed the fix/empty-body-server-rotation branch 2 times, most recently from 2a6802d to 3e55858 Compare September 23, 2026 16:58
@maxmad64bis
maxmad64bis marked this pull request as ready for review September 23, 2026 16:59
@maxmad64bis
maxmad64bis force-pushed the fix/empty-body-server-rotation branch from 3e55858 to 690fb82 Compare September 23, 2026 22:28
@maxmad64bis maxmad64bis changed the title fix(sse): retry once on another account when a stream stalls before usable output fix(sse): optionally retry once when a stream stalls before usable output Sep 23, 2026
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Solid extraction and a genuinely useful opt-in recovery path. Ran the new
stream-readiness-fallback suite (9/9) together with the rest of your stall/buffered-turn batch
on one merged tree — all green, and your own file-size rebaseline (6402→6426) comfortably
covers the small additions #14681 and #14639 make later, as long as this one lands first in the
batch. No blocking items; flagging for the release captain that #14209/#14314 (other authors)
also touch chatCore.ts and haven't been cross-tested against this set.

…tput

Streaming chat requests whose first upstream body stalls before usable output now get one bounded second attempt through the same routing path when STREAM_READINESS_STALL_RETRY is on (default off).
@maxmad64bis
maxmad64bis force-pushed the fix/empty-body-server-rotation branch from 690fb82 to 1c4096d Compare September 24, 2026 10:43
@maxmad64bis

Copy link
Copy Markdown
Contributor Author

Rebased to a single commit on the current tip and mergeable. Ready to land first as suggested, unblocking #14681.

# Conflicts:
#	tests/unit/server-owned-tool-loop-flag.test.ts
@diegosouzapw
diegosouzapw merged commit 3bfe5fe into cheaper-inference:release/v3.8.51 Sep 24, 2026
4 of 7 checks passed
@maxmad64bis
maxmad64bis deleted the fix/empty-body-server-rotation branch September 24, 2026 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants