feat(proxy): add bounded streaming discovery - #1348
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough新增有界流式 Discovery 状态机、跨协议有效性解析器及租约生命周期，并将流式代理接入候选竞速与 fallback。会话 provider 绑定迁移至 Redis 版本化 canonical/legacy 双镜像、CAS、清理和终止流程，同时补充配置、观测及测试覆盖。 Changes发现与代理编排
版本化 Session Binding
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a bounded streaming discovery path alongside the legacy hedging mechanism, adding a pure state machine coordinator, protocol-specific validity chunk parsers, and versioned session bindings in Redis. The review highlights several critical issues, including a potential deadlock in the discovery coordinator when fallback attempts become ready after normal attempts fail, an O(N^2) performance bottleneck in stream chunk parsing, and a potential runtime TypeError due to missing optional chaining on the request message. Additionally, the feedback points out unhandled promise rejections in background provider launches and a regression in session clearing when the API key owner is missing, recommending a fire-and-forget approach for non-critical Redis operations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/v1/_lib/proxy/discovery-coordinator.ts`:
- Around line 148-213: 统一 forwarder.ts 与 DiscoveryCoordinator 的 discovery
边界状态机，避免两处分别处理轮次推进、fallback 晋升和终止失败；优先让 forwarder 的边界与截止路径调用
DiscoveryCoordinator.onRoundBoundary、onDeadline 及相关取消 API，并移除重复决策逻辑，或删除生产路径未使用的
onRoundBoundary、onDeadline、cancelRequest 等协调器接口。确保所有状态转换和 action 生成只由单一实现负责。
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 5099-5130: 在处理 attempt.reader.read() 返回值的循环中，确保 commit(attempt)
完成接管后立即停止当前读取流程，避免继续将数据写入已快照的 attempt.chunks；在 response 读取逻辑和 commit
函数之间增加接管状态检查，或在接管时取消/转移 reader，并保留正常提交前的读取行为。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 990b50fe-7f18-4da1-9a86-db4b79068e76
📒 Files selected for processing (31)
package.jsonsrc/app/v1/_lib/proxy/discovery-coordinator.tssrc/app/v1/_lib/proxy/discovery-validity.tssrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/provider-selector.tssrc/app/v1/_lib/proxy/response-handler.tssrc/app/v1/_lib/proxy/session.tssrc/app/v1/_lib/proxy/stream-finalization.tssrc/lib/config/system-settings-cache.tssrc/lib/redis/client.tssrc/lib/redis/lua-scripts.tssrc/lib/redis/session-binding.tssrc/lib/session-manager.tssrc/lib/session-tracker.tssrc/types/system-config.tstests/configs/integration.config.tstests/configs/session-binding.config.tstests/integration/session-binding-versioning-redis.test.tstests/unit/lib/redis/client.test.tstests/unit/lib/redis/session-binding.test.tstests/unit/lib/session-manager-binding-smart.test.tstests/unit/lib/session-manager-terminate-provider-sessions.test.tstests/unit/lib/session-manager-terminate-session.test.tstests/unit/lib/session-manager-versioned-binding.test.tstests/unit/lib/session-tracker-cleanup.test.tstests/unit/proxy/discovery-coordinator.test.tstests/unit/proxy/discovery-validity.test.tstests/unit/proxy/provider-selector-cross-type-model.test.tstests/unit/proxy/provider-selector-model-mismatch-binding.test.tstests/unit/proxy/proxy-forwarder-hedge-first-byte.test.tstests/unit/proxy/response-handler-endpoint-circuit-isolation.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/app/v1/_lib/proxy/discovery-coordinator.ts (3)
244-253: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win保持 fallback 晋升后的内部状态一致。
当
commitWinner提交的 attempt 类型为fallback时，应将内部状态置为FALLBACK_ACTIVE，以与markReady和onRoundBoundary中的状态变迁语义保持一致。这也确保了它能与修正后的isTerminal属性配合正常工作。修复建议
commitWinner(id: string): DiscoveryAction { const attempt = this.attempts.get(id); if (!attempt || this.isTerminal) return { type: "none" }; attempt.pending = false; - this.state = "WINNER_COMMITTED"; + this.state = attempt.kind === "fallback" ? "FALLBACK_ACTIVE" : "WINNER_COMMITTED"; return { type: attempt.kind === "fallback" ? "promote_fallback" : "commit_normal", attemptId: id, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/v1/_lib/proxy/discovery-coordinator.ts` around lines 244 - 253, Update commitWinner so fallback attempts set the coordinator state to FALLBACK_ACTIVE, while non-fallback attempts retain WINNER_COMMITTED. Keep the existing promote_fallback/commit_normal action selection and attempt handling unchanged.
274-283: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win修正新轮次启动时的槽位计算。
finishOrLaunch仅在当前没有任何活动的尝试（既无 pending normal，也无 fallback）时被调用。既然没有保留 fallback 占用并发槽位，新轮次应当可以使用完整的this.concurrency槽位，而不是this.concurrency - 1。修复建议
private finishOrLaunch(): DiscoveryAction { if (this.round >= this.maxRounds) { this.state = "TERMINAL_FAILED"; return { type: "terminal_failure" }; } this.round += 1; this.roundEpoch += 1; this.state = "DISCOVERY_RACING"; - return { type: "launch", slots: Math.max(1, this.concurrency - 1) }; + return { type: "launch", slots: this.concurrency }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/v1/_lib/proxy/discovery-coordinator.ts` around lines 274 - 283, 更新 DiscoveryCoordinator.finishOrLaunch 中新轮次的 launch 动作，使 slots 使用完整的 this.concurrency，而不是 this.concurrency - 1；保留其余轮次递增、状态切换和终止失败逻辑不变。
85-87: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win将
FALLBACK_ACTIVE纳入终止状态。
FALLBACK_ACTIVE表示 fallback 已经胜出并晋升，此时该请求的协调器应当处于终止状态。由于当前isTerminal未包含此状态，一旦脱离外部调用方的严格防重入保护，状态机可能会在 fallback 晋升后错误地继续处理其他延迟到达的事件，甚至导致状态被意外重置。修复建议
- get isTerminal(): boolean { - return this.state === "WINNER_COMMITTED" || this.state === "TERMINAL_FAILED"; - } + get isTerminal(): boolean { + return ( + this.state === "WINNER_COMMITTED" || + this.state === "FALLBACK_ACTIVE" || + this.state === "TERMINAL_FAILED" + ); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/v1/_lib/proxy/discovery-coordinator.ts` around lines 85 - 87, Update the isTerminal getter in the coordinator state machine to include FALLBACK_ACTIVE alongside WINNER_COMMITTED and TERMINAL_FAILED, so fallback promotion is treated as terminal and later events cannot continue processing or reset state.
🧹 Nitpick comments (1)
src/app/v1/_lib/proxy/discovery-coordinator.ts (1)
155-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value简化最佳优先级的筛选逻辑。
readyNormal数组在此前已经按照compareAttempts进行了排序，因此readyNormal[0]必然是当前具有最高优先级的最佳候选尝试，无需再次通过filter遍历数组。重构建议
- const sameTier = readyNormal.filter((attempt) => attempt.priority === bestPriority); - const winner = sameTier[0]; + const winner = readyNormal[0];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/v1/_lib/proxy/discovery-coordinator.ts` around lines 155 - 156, 简化最佳候选选择逻辑：在发现协调流程中移除围绕 sameTier 的 filter 操作，直接使用已按 compareAttempts 排序的 readyNormal[0] 作为 winner，并保持后续对 winner 的处理不变。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/app/v1/_lib/proxy/discovery-coordinator.ts`:
- Around line 244-253: Update commitWinner so fallback attempts set the
coordinator state to FALLBACK_ACTIVE, while non-fallback attempts retain
WINNER_COMMITTED. Keep the existing promote_fallback/commit_normal action
selection and attempt handling unchanged.
- Around line 274-283: 更新 DiscoveryCoordinator.finishOrLaunch 中新轮次的 launch 动作，使
slots 使用完整的 this.concurrency，而不是 this.concurrency - 1；保留其余轮次递增、状态切换和终止失败逻辑不变。
- Around line 85-87: Update the isTerminal getter in the coordinator state
machine to include FALLBACK_ACTIVE alongside WINNER_COMMITTED and
TERMINAL_FAILED, so fallback promotion is treated as terminal and later events
cannot continue processing or reset state.
---
Nitpick comments:
In `@src/app/v1/_lib/proxy/discovery-coordinator.ts`:
- Around line 155-156: 简化最佳候选选择逻辑：在发现协调流程中移除围绕 sameTier 的 filter 操作，直接使用已按
compareAttempts 排序的 readyNormal[0] 作为 winner，并保持后续对 winner 的处理不变。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 55279730-6ccb-4b40-af3e-ff6dc0512dfb
📒 Files selected for processing (5)
src/app/v1/_lib/proxy/discovery-coordinator.tssrc/app/v1/_lib/proxy/discovery-validity.tssrc/app/v1/_lib/proxy/forwarder.tstests/unit/proxy/discovery-coordinator.test.tstests/unit/proxy/discovery-validity.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/unit/proxy/discovery-validity.test.ts
- tests/unit/proxy/discovery-coordinator.test.ts
- src/app/v1/_lib/proxy/discovery-validity.ts
- src/app/v1/_lib/proxy/forwarder.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee5557ecaf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43c6733e43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
Issues Found
| # | Severity | File | Line | Description |
|---|---|---|---|---|
| 1 | Medium | response-handler.ts |
1155-1159 | Versioned binding clear uses wrong expectedProviderId — silently fails for Discovery |
| 2 | Medium | forwarder.ts |
4841 | No unit tests for sendStreamingWithDiscovery (~400 lines of complex async logic) |
No Critical or High severity issues found.
Finding Details
1. [Medium] clearSessionBinding passes wrong expectedProviderId for versioned clear
In the completionMarkerMissing failure path (and all Discovery failure paths that call clearSessionBinding), clearVersionedSessionProvider is called with providerIdForPersistence (the winner's provider ID). But the binding CAS-set only runs in the success path after the completion marker check passes — so at the point of clearing, the binding was never updated to the winner. The CLEAR_SESSION_BINDING Lua script detects the provider mismatch and returns {conflict, provider_mismatch}, causing the clear to silently no-op. The return value is discarded with no logging.
The forwarder's settleFailure in sendStreamingWithDiscovery (~line 4955) correctly uses bindingSnapshot.providerId — confirming the intended pattern. Fix: use meta.bindingSnapshot.providerId instead of providerIdForPersistence.
2. [Medium] sendStreamingWithDiscovery has zero unit test coverage
The method implements the full Discovery streaming pipeline: multi-round provider racing, three independent timers (total/round/sticky), coordinator state machine integration, reader-loop cancellation, winner commit, and binding management. The coordinator and validity parser have isolated tests, but the integration layer is completely untested. CLAUDE.md requires 80% coverage for new features.
Review Coverage
- Execution paths and error handling
- Test coverage gaps
- Silent failure detection (return values, swallowed errors)
- Type safety and casts
- Race conditions and async coordination
- Redis Lua script correctness
- Session binding lifecycle (read/CAS/clear/terminate)
- Dead code analysis
PR Size: XL (5,283+ / 314- across 32 files)
For a PR of this size, consider splitting into focused PRs to reduce review risk:
- Discovery primitives —
discovery-coordinator.ts+discovery-validity.ts(with tests) - Session binding versioning —
session-binding.ts, Lua scripts,session-manager.tschanges - Forwarder integration —
sendStreamingWithDiscovery+response-handler.tschanges + provider selector
The core architecture (pure coordinator state machine, protocol-aware validity parsing, generation-CAS binding with capability probing) is well-structured. The main risks are in the untested integration layer and the binding-clear correctness issue noted above.
Automated review by Claude AI
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e0cedba26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
# Conflicts: # src/app/v1/_lib/proxy/forwarder.ts # src/app/v1/_lib/proxy/response-handler.ts # src/app/v1/_lib/proxy/stream-finalization.ts # tests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts # tests/unit/proxy/response-handler-endpoint-circuit-isolation.test.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e24438b3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f9cca1f0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d23d6cbc1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c1f166281
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c82d521a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…0723' into agent/discovery-1348-on-integration # Conflicts: # tests/unit/proxy/response-handler-endpoint-circuit-isolation.test.ts
46315cf
into
ding113:integration/discovery-stack-20260723
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a885e0c251
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| endpointId: activeEndpoint.endpointId, | ||
| endpointUrl: endpointAudit.endpointUrl, | ||
| upstreamStatusCode: response.status, | ||
| bindingIntent: session.isSessionBindingAllowed() ? undefined : "none", |
There was a problem hiding this comment.
When prepareStreamingDiscovery() disables session binding after a Discovery lease/binding conflict, ProviderSelector.ensure() may already have recorded a provider concurrent-session ref for this request. This deferred metadata only suppresses the binding write; because it does not mark the ref as owned, releaseOwnedProviderSessionRef() never consumes it on a successful single-upstream SSE, so the provider's active-session count remains inflated until TTL and later requests can incorrectly hit limitConcurrentSessions.
Useful? React with 👍 / 👎.
| event.event === "response.completed" && | ||
| isRecord(event.data) && | ||
| event.data.type === "response.completed" && |
There was a problem hiding this comment.
For Responses-format SSE streams whose terminal arrives as a data-only event (data: {"type":"response.completed", ...} without an event: line), parseSSEData() assigns the default event name message and DiscoveryValidityParser accepts the same payload as terminal. This strict event-name gate makes finalization classify an otherwise complete Discovery winner as STREAM_COMPLETION_MARKER_MISSING, avoiding/clearing Sticky and recording a provider failure after a valid response.
Useful? React with 👍 / 👎.
| (object.type === "response.output_text.delta" && hasContent(object.delta)) || | ||
| (object.type === "response.function_call_arguments.delta" && hasContent(object.delta)) || | ||
| (object.type === "response.output_item.added" && hasOpenAIResponsesOutputItem(object.item)), |
There was a problem hiding this comment.
When a Responses stream's first or only user-visible payload is delivered through response.output_item.done, response.function_call_arguments.done, response.refusal.delta/done, or reasoning-summary events, this readiness predicate never marks the candidate ready even though those event types carry finalized text/tool arguments per the Responses streaming events reference. The subsequent response.completed is then treated as terminal-without-ready by the Discovery forwarder, causing a valid provider to be failed over or returned as unavailable.
Useful? React with 👍 / 👎.
| if (!choice || typeof choice !== "object") return false; | ||
| const choiceObject = choice as Record<string, unknown>; | ||
| const delta = choiceObject.delta; | ||
| return hasContent(delta) || hasContent(choiceObject.message); |
There was a problem hiding this comment.
When an OpenAI-compatible Chat stream returns a refusal-only answer, the streamed delta can carry refusal instead of content per the Chat Completions streaming schema. Because this path delegates to hasContent(delta), which does not inspect refusal, Discovery never marks that valid refusal as ready and then fails the provider when the stream terminates without a recognized content delta.
Useful? React with 👍 / 👎.
| const orchestrate = async () => { | ||
| let initialLaunchFailed = false; | ||
| try { | ||
| await launch(initialProvider, "normal"); |
There was a problem hiding this comment.
After a Sticky timeout writes a provider cooldown, the next request can still have the normal selector choose that same provider as initialProvider; the only cooldown check is inside pickDiscoveryProviders(), but this line launches the initial provider before that helper is used. In sessions with a recently timed-out Sticky provider, this bypasses stickyTimeoutCooldownMs and can immediately retry the provider that was supposed to be cooled down.
Useful? React with 👍 / 👎.
2 participants
Summary
This is PR 2/3 for #1340 and is stacked on #1347. It adds a default-off bounded streaming Discovery path beside the existing Hedge path.
The existing Hedge path remains unchanged when Discovery is disabled, the protocol/endpoint is ineligible, or the Redis capability gate is unavailable.
Validation
bun run typecheckbun run format:checkbun run buildStack
Refs #1340.
Greptile Summary
This PR adds a bounded streaming Discovery path that races multiple providers in parallel behind a default-off feature flag, sitting beside the existing Hedge path without altering its behavior. The core is a pure state machine (
DiscoveryCoordinator) and an incremental validity parser (DiscoveryValidityParser) that validates SSE/JSON prefixes before committing a winner.discovery-coordinator.ts): pure state machine with priority gating, round management, epoch-validated events, and sticky-probe to fallback demotion.discovery-validity.ts): protocol-aware incremental SSE/JSON parser that rejects error, fake-200, empty, and malformed prefixes, with a 1 MiB byte cap and 1 024-event cap.forwarder.ts): addsprepareStreamingDiscovery(lease acquisition, rollout gate) andsendStreamingWithDiscovery(the main race loop, sticky SLA timer, round timer, total deadline, lease transfer, provider-session-ref tracking).response-handler.ts): addsstartDiscoveryLeaseLifecycle(renewal + heartbeat during stream finalization), CAS-based binding on success,finalizeFailedDiscoveryBindingin all failure paths, and a stricter format-awarehasStreamCompletionMarkerreplacing the old regex approach.Confidence Score: 5/5
Safe to merge — the Discovery path is default-off, the existing Hedge path is untouched, and the new code is well-guarded with epoch validation, idempotent settlement, and a rollout-percent gate.
All findings are non-blocking style and observability nits. The coordinator state machine is epoch-validated and covers all edge cases (sticky demotion, priority gating, round boundaries, total deadline, client abort). The validity parser correctly handles byte/event limits. The response-handler failure paths consistently call
finalizeFailedDiscoveryBinding, closing the gap flagged in the previous review cycle.No files require special attention. The three changed core files (forwarder, response-handler, coordinator) each have unit/integration test coverage added in this PR.
Important Files Changed
this.state = "DISCOVERY_RACING"writes afterbeginRound()calls inonRoundBoundaryandfinishOrLaunch.new TextEncoder()is allocated on every string-chunk push; usingnew Blob([chunk]).sizeis a drop-in improvement.prepareStreamingDiscoveryandsendStreamingWithDiscovery. Logic is well-guarded: sticky probe, round timer, total deadline, epoch validation,noMoreCandidateshandling, and lease transfer on winner commit all appear correct.finalizeFailedDiscoveryBindingcovering all failure paths (completion-marker missing, fake-200, non-2xx, stream-abort).hasStreamCompletionMarkerrefactored from regex to format-aware SSE parsing.ttfbMsfield is set toelapsedMsat winner-commit time, which is closer to "time to winner selection" than true TTFB from the client's perspective.ProxySessionwithsessionBindingSnapshot,sessionBindingAllowed,streamingHedgeDisabledflags, and convertsproviderSessionRefsfrom a Set to a Map supportingretainOnSuccessmetadata. Changes are additive and backward-compatible.pickDiscoveryProvidersstatic method andresolveEffectivePriorityForSessionhelper; replacesclearSessionProvidercalls with the generation-awareclearRejectedProviderBindingclosure to avoid ABA on stale binding clears.DeferredStreamingDiscoveryLeasetype and Discovery-specific fields (bindingIntent,discoveryLease,providerSessionRefOwned, etc.) toDeferredStreamingFinalization. Clean additive change.Sequence Diagram
sequenceDiagram participant C as Client participant F as Forwarder participant DC as DiscoveryCoordinator participant P1 as Provider (Sticky) participant P2 as Provider (Normal) participant RH as ResponseHandler participant SM as SessionManager C->>F: streaming request F->>SM: acquireSessionDiscoveryLease SM-->>F: lease (ownerToken) F->>DC: new DiscoveryCoordinator F->>DC: startStickyProbe() F->>P1: launch(stickyProvider, normal) note over F: stickyTimer = stickySlaMs alt Sticky responds before SLA P1-->>F: "valid chunk ready=true" F->>DC: markReady(sticky.id) DC-->>F: commit_normal F->>RH: commit(winner, lease) RH->>SM: renewSessionDiscoveryLease RH->>SM: compareAndSetSessionProvider CAS RH->>SM: releaseSessionDiscoveryLease else Sticky SLA timeout note over F: stickyTimer fires F->>DC: demoteToFallback(sticky.id) F->>DC: startDiscoveryAfterSticky() F->>SM: clearVersionedSessionProvider cooldown F->>P2: launch(discoveryProvider, normal) note over F: roundTimer = discoverySlaMs alt Normal wins P2-->>F: "valid chunk ready=true" F->>DC: markReady(normal.id) DC-->>F: commit_normal F->>P1: cancel discovery_sla_timeout F->>RH: commit(winner, lease) RH->>SM: "compareAndSetSessionProvider bindingIntent=create" else Round boundary note over F: roundTimer fires F->>DC: onRoundBoundary() DC-->>F: launch next round F->>P2: cancel losers else Total deadline F->>DC: onDeadline() DC-->>F: terminal_failure or promote_fallback end end F-->>C: Response streamReviews (12): Last reviewed commit: "Merge remote-tracking branch 'origin/int..." | Re-trigger Greptile