feat(web-search): opt-in live streaming of routed-model output + Dashboard toggle - #1500
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds an opt-in ChangesWeb-search sidecar live streaming
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant SettingsAPI
participant WebSearchPlan
participant WebSearchLoop
participant SSEClient
Dashboard->>SettingsAPI: Save streamRoutedModelOutput
SettingsAPI->>WebSearchPlan: Persisted streaming setting
WebSearchPlan->>WebSearchLoop: Resolved streaming flag
WebSearchLoop->>SSEClient: Leading allowlisted text and thinking events
WebSearchLoop->>WebSearchLoop: Buffer events for web-search scanning
WebSearchLoop->>SSEClient: Deduplicated replay after tool-call boundary
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 |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
`@devlog/_plan/260812_websearch_sidecar_live_streaming/000_findings_and_design.md`:
- Around line 42-44: Update the final sentence in the documented accepted
tradeoff to qualify the reasoning-first model claim: state that these models may
avoid pre-search text repetition, while acknowledging that leading
thinking_delta and reasoning events still become client-visible when the option
is enabled.
In `@docs-site/src/content/docs/guides/sidecars.md`:
- Around line 37-43: Update the streamRoutedModelOutput documentation near its
description to explicitly cover Kiro behavior: sidecar commentary may stream
before the terminal event, while only search-decision events remain buffered.
Preserve the existing explanation of the live window, atomic web_search
interception, and terminal replay.
In `@tests/web-search.test.ts`:
- Around line 2191-2198: Update tests/web-search.test.ts lines 2191-2198 to gate
the adapter after “prefix ”, use frameReader.readUntil to confirm that delta
reaches the client before releasing the tool call, and assert the function-call
frame precedes the replayed “suffix”. Update lines 2233-2237 to gate the first
pass after “Let me check. ”, require client receipt before allowing web_search,
and assert the post-search answer is emitted exactly once.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 52d2cbcb-0b07-40b2-a9a6-868a3ab47950
📒 Files selected for processing (7)
devlog/_plan/260812_websearch_sidecar_live_streaming/000_findings_and_design.mddocs-site/src/content/docs/guides/sidecars.mdsrc/server/responses/core.tssrc/types.tssrc/web-search/index.tssrc/web-search/loop.tstests/web-search.test.ts
|
Addressed all three review findings in 2e9e514:
Re-verified: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/web-search.test.ts`:
- Around line 2165-2167: Update both live-delivery tests around releaseToolCall
and releaseWebSearch so their 5-second timeout rejects or otherwise fails the
readUntil wait instead of invoking the release gate. Ensure each release
function is called only after readUntil observes the prefix, preventing buffered
terminal replay from satisfying the test.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 87c79557-80a7-4bdf-a411-f8600dc7c482
📒 Files selected for processing (3)
devlog/_plan/260812_websearch_sidecar_live_streaming/000_findings_and_design.mddocs-site/src/content/docs/guides/sidecars.mdtests/web-search.test.ts
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedYou are interacting with an AI system. |
|
|
|
|
RobinBially
changed the title
|
🧠 Learnings used✅ Action performedReview finished.
|
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedYou are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/web-search.test.ts`:
- Around line 2165-2205: Add a sibling gated streaming test near the existing
“leading text deltas stream live” test that uses the adapter’s parseStream to
yield a thinking_delta, waits until the client observes the corresponding
reasoning SSE frame, then yields done. Assert the reasoning content is delivered
live and terminal replay does not duplicate it, while preserving the existing
completion behavior.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4e6bbf65-4aed-4588-ae00-4fd59be1668b
📒 Files selected for processing (11)
gui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/tr.tsgui/src/i18n/zh-TW.tsgui/src/i18n/zh.tssrc/server/responses/core.tssrc/types.tstests/web-search.test.ts
|
🧠 Learnings used
|
CodeRabbit round 6: the live-window tests only covered text_delta, so a regression that buffers or drops thinking_delta would pass. The new gated test blocks the adapter until the client has observed the leading response.reasoning_summary_text.delta on the wire (rejecting 5s deadline), then asserts exactly-once delivery across the terminal replay. Requires reasoning.summary=auto in the request — without it the parser sets hideThinkingSummary and reasoning is never client-visible by design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Re-review after the branch update: approved.
The blocker from my previous review is resolved. This head is now based on c125b612, and current dev is only two commits ahead (#1541 and #1548). I compared those two commits against this PR's changed-file set and there is no overlap, so the earlier integration-risk concern in responses/core, types, and the web-search tests no longer applies.
I re-checked the current live-streaming path, terminal replay accounting, sidecar-settings persistence/validation, Dashboard state merge, and the gated streaming tests. I found no correctness blocker.
CodeRabbit's current suggestion for an additional gated thinking_delta test is reasonable coverage hardening but not a blocker: all allowlisted event types use the same live-window branch in consumeIterationEvents, and the existing bridge tests already pin thinking_delta to the reasoning-summary SSE mapping. The new text-gated tests exercise the timing/replay semantics shared by that branch.
Required GitHub Actions are still showing action_required for this fork head rather than a test failure; the PR reports a full local suite of 11,310 pass / 0 fail. Merge should still follow the repository's normal required-CI policy.
|
✅ Action performedReview finished.
|
|
🧠 Learnings used
|
|
Heads-up on the red
Happy to absorb the fix here if you prefer, but it seems cleaner to patch the contract test (or the extra call) directly on |
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Correction to my approval: there is a current-base integration failure that I missed. PR #1500's own head still has the expected 6 provider-route convergence calls and passes this inventory contract, but current dev gained a seventh await convergeCodexCatalog() in provider-routes.ts from #1541's new attested provider-reload route while tests/codex-convergence-contract.test.ts still expects 6. So this is not caused by #1500, but I would not merge #1500 into the current red base until that stale inventory contract is repaired (most likely provider-routes 7 and the test title/expected map updated) and the integration suite is green.
|
@Wibias Opened #1554 against |
|
🧠 Learnings used✅ Action performedReview finished.
|
|
🧠 Learnings used
|
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedYou are interacting with an AI system. |
|
🧠 Learnings used
|
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedYou are interacting with an AI system. |
|
Thanks @RobinBially - this is a genuinely useful improvement. The web-search sidecar could make routed models feel like they were not streaming at all, with a long silent wait followed by one end-of-turn burst. This keeps the existing buffered behaviour as the safe default, while giving users an opt-in path to see leading text and reasoning live without weakening atomic The Dashboard toggle matters too: this fixes the problem where users actually encounter it instead of hiding the solution behind a config-only flag. Nice work on the gated text/reasoning tests and the end-to-end investigation as well. Thanks for the contribution! |
2 participants
feat(web-search): opt-in live streaming of routed-model output (
streamRoutedModelOutput) + Dashboard toggleProblem
With the web-search sidecar engaged (Codex sends a hosted
web_searchtool on every real turn, anda usable ChatGPT credential exists),
runWithWebSearchfully buffers every semantic adapter eventof an iteration before scanning for
web_searchcalls. The client sees nothing until the turnfinishes — on reasoning-heavy turns 6–50 s of silence, then the whole answer as one burst. This is
the "chat doesn't stream through opencodex" experience for every routed model.
Reproduced end-to-end: a byte-identical replay of a captured real
codex execrequest buffers on asidecar-enabled instance and streams on an identical instance without ChatGPT auth; a field bisect
shows removing only the
web_searchtool from the request restores streaming (448 deltas, first at3.5 s), while removing the other hosted tools does not.
Fix
New config option
webSearchSidecar.streamRoutedModelOutput(defaultfalse— behavior unchangedwithout opt-in). When enabled, each iteration's leading output streams live and the live window
closes permanently at the first buffer-only event:
loop.ts:text_delta,thinking_delta,reasoning_raw_delta,thinking_signature,redacted_thinking,kiro_redacted_reasoning—exactly what the sidecar-less path would deliver identically. Anything else (tool calls above
all) closes the window, so the
web_searchinterception decision stays atomic and live eventsare exactly the first N passthrough entries.
streams live again.
extractIterationThinkingand theforced-answer output check ([Bug]: Hosted web-search forced-answer pass accepts a malformed tool call and completes without an assistant message #1001) are unchanged.
Documented tradeoff (docs-site/guides/sidecars.md): text the model emits before deciding to
search — which buffered mode silently drops — becomes visible and may partially repeat in the
post-search answer. Reasoning-first models avoid the text-repetition case (their leading reasoning
deltas become client-visible — that visibility is the point of the option).
Discoverability: Dashboard toggle
A config-file-only flag would be hard to find for anyone hitting "why doesn't chat stream?", so the
option is also surfaced in the UI:
GET/PUT /api/sidecar-settingsnow carrywebSearch.streamRoutedModelOutput(boolean;non-boolean values are rejected with 400;
falseremoves the key so config files stay minimal).switch (strings in all eight locales).
Tests
bun test tests/web-search.test.ts— 55 pass (4 new). The live-delivery tests are gated: thefake adapter withholds its next event until the test has observed the leading delta on the wire,
and the 5 s deadline rejects instead of releasing the gate, so a buffered implementation
fails rather than passing via the terminal replay. Covered: live first delta mid-turn; default
(flag unset) still buffers; window closes at
tool_call_startwith exactly-once, in-order replay(prefix delta → function_call → suffix); search loop delivers pre-search text and final answer
exactly once.
bun test tests/sidecar-settings-web-search-stream.test.ts— 4 pass (new): GET reflection,PUT true/false round-trip with key removal, 400 on non-boolean, PUT without the flag leaves an
enabled value untouched.
bun test tests/web-search-*.test.ts— 78 pass. GUI:cd gui && bun test tests— 764 pass;eslint .clean.bun x tsc --noEmitclean in root and gui (TypeScript 7.0.2).bun run testsuite (after merging currentdev): 11310 pass / 0 fail / 8 skip,699 files, 509 s.
opencode-go/deepseek-v4-flash, identical text-forcing request):without restart via
PUT /api/sidecar-settings.Devlog:
devlog/_plan/260812_websearch_sidecar_live_streaming/000_findings_and_design.mdcarriesthe full investigation (bit-identical-instance comparison, capture, bisect).
🤖 Generated with Claude Code
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
Documentation
Tests