feat: streaming dep optional, non streaming path, streaming flag, rel… - #65
Merged
Conversation
Contributor
Author
|
@CodeRabbit review |
Contributor
✅ Action performedReview finished.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughChangesTurn execution and feature configuration
Sequence Diagram(s)sequenceDiagram
participant User
participant chat
participant BareLoop
participant ProviderClient
User->>chat: Set NO_STREAM=1
chat->>BareLoop: Set TurnMode::NonStreaming
BareLoop->>ProviderClient: Send create_message request
ProviderClient-->>BareLoop: Return assembled response
BareLoop-->>chat: Return completed turn
chat-->>User: Print assembled response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/engine/bare.rs (2)
2708-2716: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the test to match what it asserts.
turn_mode_default_is_nonstreaming_without_featureasserts both branches:NonStreamingwithout the feature andStreamingwith it. The name describes only one branch.📝 Proposed rename
- fn turn_mode_default_is_nonstreaming_without_feature() { + fn turn_mode_default_follows_streaming_feature() {🤖 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/engine/bare.rs` around lines 2708 - 2716, Rename the test function turn_mode_default_is_nonstreaming_without_feature to reflect that it verifies the default turn mode for both streaming and non-streaming feature configurations. Keep the assertions and test behavior unchanged.
2469-2494: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared mock response assembly.
MockClient::create_messagehere andRecordingClient::create_messageat lines 6062-6092 contain the same event-to-response assembly: pop queued events, track the lastMessageDeltastop reason, feed each event toStreamAccumulator, then build theNonStreamingResponse. The two copies will drift as the accumulator contract evolves.Extract one test helper, for example
fn assemble_response(events: Vec<StreamEvent>) -> Result<NonStreamingResponse, ApiError>, and call it from both clients.🤖 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/engine/bare.rs` around lines 2469 - 2494, Extract the duplicated event-to-response logic from MockClient::create_message and RecordingClient::create_message into a shared test helper such as assemble_response. The helper should accept queued StreamEvent values, track the latest MessageDelta stop reason, process events through StreamAccumulator, and return the constructed NonStreamingResponse or ApiError; update both client methods to call it after popping their events.src/provider.rs (1)
130-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the repeated provider gate into one named feature.
The predicate
any(feature = "openai", feature = "anthropic", feature = "gemini")now appears at lines 58, 62, 64, 130, 198, 213, 326, and 333. Each new provider requires editing every occurrence, and a missed site produces a feature-combination build break rather than a visible error.Add an internal aggregate feature in
Cargo.toml, for example_http_provider = [], haveopenai,anthropic, andgeminienable it, then gate on#[cfg(feature = "_http_provider")]at each site.Also applies to: 198-198, 213-213, 326-333
🤖 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/provider.rs` at line 130, Replace the repeated provider cfg predicate in src/provider.rs, including the sites around provider definitions and HTTP-provider code, with #[cfg(feature = "_http_provider")]. Define the internal _http_provider aggregate feature in Cargo.toml and update the openai, anthropic, and gemini features to enable it, preserving their existing feature behavior.
🤖 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 `@Cargo.toml`:
- Around line 99-104: Remove panic = "abort" from the [profile.release]
configuration so dispatch_tool’s catch_unwind-based tool-panic isolation remains
effective in release builds. Preserve the other release profile settings
unchanged.
In `@examples/chat.rs`:
- Around line 336-353: Update the runtime mode setup around `no_stream` so it is
true whenever the `streaming` feature is unavailable, ensuring the later
result-printing logic displays non-streaming responses. In the feature-enabled
live-streaming branch, explicitly select `TurnMode::Streaming` before
configuring the text streamer, while preserving the existing `NO_STREAM=1`
non-streaming behavior.
In `@README.md`:
- Around line 170-175: Update the README paragraphs describing streaming so they
distinguish feature availability from runtime mode selection: state that the
streaming feature enables TurnMode::Streaming, but turns use it only when
selected, including via set_turn_mode, while TurnMode::NonStreaming remains
available. Apply the same clarification to the referenced paragraph.
In `@src/engine/bare.rs`:
- Around line 122-134: Resolve the conflicting defaults for TurnMode by removing
its derived Default implementation and relying solely on default_turn_mode() for
the feature-dependent constructor default. Update the associated public
documentation to describe this single default contract accurately, while
preserving the enum variants and mode-switching behavior.
- Around line 136-144: Update the documentation for the cfg-gated Streaming
variant to remove the claim that disabling the streaming feature produces a
LoopError::Config at turn time, while retaining only the accurate streaming
feature requirement.
---
Nitpick comments:
In `@src/engine/bare.rs`:
- Around line 2708-2716: Rename the test function
turn_mode_default_is_nonstreaming_without_feature to reflect that it verifies
the default turn mode for both streaming and non-streaming feature
configurations. Keep the assertions and test behavior unchanged.
- Around line 2469-2494: Extract the duplicated event-to-response logic from
MockClient::create_message and RecordingClient::create_message into a shared
test helper such as assemble_response. The helper should accept queued
StreamEvent values, track the latest MessageDelta stop reason, process events
through StreamAccumulator, and return the constructed NonStreamingResponse or
ApiError; update both client methods to call it after popping their events.
In `@src/provider.rs`:
- Line 130: Replace the repeated provider cfg predicate in src/provider.rs,
including the sites around provider definitions and HTTP-provider code, with
#[cfg(feature = "_http_provider")]. Define the internal _http_provider aggregate
feature in Cargo.toml and update the openai, anthropic, and gemini features to
enable it, preserving their existing feature behavior.
🪄 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 Plus
Run ID: 6a114cd1-01ed-41f8-943b-b6b87b293fa5
📒 Files selected for processing (17)
CHANGELOG.mdCargo.tomlREADME.mdexamples/chat.rssrc/capabilities.rssrc/engine/bare.rssrc/engine/bare/compact.rssrc/engine/bare/dispatch.rssrc/engine/bare/emission.rssrc/engine/bare/stream.rssrc/managers.rssrc/observer.rssrc/provider.rssrc/reflection/llm.rssrc/stream.rssrc/stream/rate_limit.rssrc/structured.rs
This was referenced Aug 3, 2026
Merged
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.
…ease profile