docs: Document smart-fallback streaming behavior for sync chat (fixes #484)#487
Conversation
- Add auto-detect (Default) subsection showing stream=None default - Expand Streaming vs Non-Streaming table to include three modes - Add Sync vs Async Adapters callout explaining behavior differences - Update Troubleshooting section with Deepseek crash fix entry - Update Best Practices with recommendation to let SDK pick mode - Update method table to highlight auto-detect as recommended approach Fixes #484 Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughUpdated ChangesStreaming Defaults and Smart-Fallback Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request updates the streaming documentation to explain the new auto-detect streaming behavior, which automatically falls back to non-streaming if a provider's sync client does not support it. It updates the comparison tables, adds a best practices section, and includes troubleshooting steps for sync adapter crashes. The feedback suggests removing the internal method _execute_unified_achat_completion from the user-facing documentation to avoid exposing implementation details.
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.
| | `stream=False` | Force non-streaming | Batch jobs, structured output, sync providers | | ||
|
|
||
| <Note> | ||
| **Sync vs Async Adapters**: Async methods (`achat`, `astart`, `_execute_unified_achat_completion`) still default to `stream=True` because async adapters universally support streaming. Sync methods (`chat`, `start`, `run`) use the new smart-fallback default. Some adapters (e.g., sync OpenAI/Deepseek adapter) currently do NOT support sync streaming and will trigger the fallback. |
There was a problem hiding this comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/features/streaming.mdx (1)
354-356: ⚡ Quick winConsider removing internal method reference from user-facing docs.
Line 355 mentions
_execute_unified_achat_completion(leading underscore = private/internal method) in the async methods list. Exposing internal method names in user documentation can confuse users who search for it in public API docs and may constrain future refactoring.Users interact with
achat()andastart()—the internal implementation detail doesn't help them use the feature.📝 Proposed simplification
-**Sync vs Async Adapters**: Async methods (`achat`, `astart`, `_execute_unified_achat_completion`) still default to `stream=True` because async adapters universally support streaming. Sync methods (`chat`, `start`, `run`) use the new smart-fallback default. Some adapters (e.g., sync OpenAI/Deepseek adapter) currently do NOT support sync streaming and will trigger the fallback. +**Sync vs Async Adapters**: Async methods (`achat`, `astart`) still default to `stream=True` because async adapters universally support streaming. Sync methods (`chat`, `start`, `run`) use the new smart-fallback default. Some adapters (e.g., sync OpenAI/Deepseek adapter) currently do NOT support sync streaming and will trigger the fallback.🤖 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 `@docs/features/streaming.mdx` around lines 354 - 356, Remove the internal method reference from the docs: replace the mention of the private method `_execute_unified_achat_completion` with the public async APIs `achat()` and `astart()` (and keep the note that async methods default to stream=True), so users only see public symbols (`achat`, `astart`) and the sync counterparts (`chat`, `start`, `run`) when describing streaming behavior; this avoids exposing private implementation details and prevents coupling docs to `_execute_unified_achat_completion`.
🤖 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 `@docs/features/streaming.mdx`:
- Around line 354-356: Update the note so it no longer claims "async adapters
universally support streaming" and instead explains that async methods (`achat`,
`astart`, `_execute_unified_achat_completion`) default to `stream=True` only
when the adapter reports streaming capability; document that sync methods
(`chat`, `start`, `run`) use the smart-fallback default and that the runtime
uses capability checks and provider-specific disablements to trigger fallback
(e.g., Anthropic streaming disabled due to a litellm bug, Gemini disables
streaming when `tools` are present, and Ollama streaming is unreliable with
tools); replace the universal claim with language emphasizing capability-based
fallback and list those provider-specific exceptions so readers understand when
streaming may be disabled.
---
Nitpick comments:
In `@docs/features/streaming.mdx`:
- Around line 354-356: Remove the internal method reference from the docs:
replace the mention of the private method `_execute_unified_achat_completion`
with the public async APIs `achat()` and `astart()` (and keep the note that
async methods default to stream=True), so users only see public symbols
(`achat`, `astart`) and the sync counterparts (`chat`, `start`, `run`) when
describing streaming behavior; this avoids exposing private implementation
details and prevents coupling docs to `_execute_unified_achat_completion`.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6f1e1c52-15c8-43a8-aa4a-21176d10cd2c
📒 Files selected for processing (1)
docs/features/streaming.mdx
| <Note> | ||
| **Sync vs Async Adapters**: Async methods (`achat`, `astart`, `_execute_unified_achat_completion`) still default to `stream=True` because async adapters universally support streaming. Sync methods (`chat`, `start`, `run`) use the new smart-fallback default. Some adapters (e.g., sync OpenAI/Deepseek adapter) currently do NOT support sync streaming and will trigger the fallback. | ||
| </Note> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for async adapter implementations to confirm streaming support
# Find async adapter class definitions and check for streaming-related NotImplementedError or exceptions
rg -nP -A10 'class\s+\w*Adapter.*:' --type=py -g '!*test*' | rg -C3 'async.*chat|achat|streaming'
# Search for any documentation or comments indicating async streaming limitations
rg -nP 'async.*stream.*not.*support|async.*adapter.*stream.*unsupport' --type=py --type=mdRepository: MervinPraison/PraisonAIDocs
Length of output: 6991
Fix the streaming docs: async adapters aren’t universally streaming-capable (qualify the claim).
docs/features/streaming.mdx lines 354-356 say “async adapters universally support streaming,” but the adapter capability logic includes provider-specific disablement (e.g., Anthropic streaming disabled due to a litellm bug; Gemini streaming disabled when tools are present; Ollama streaming with tools is unreliable). Update the note to describe capability-based fallback rather than universal async streaming support.
🤖 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 `@docs/features/streaming.mdx` around lines 354 - 356, Update the note so it no
longer claims "async adapters universally support streaming" and instead
explains that async methods (`achat`, `astart`,
`_execute_unified_achat_completion`) default to `stream=True` only when the
adapter reports streaming capability; document that sync methods (`chat`,
`start`, `run`) use the smart-fallback default and that the runtime uses
capability checks and provider-specific disablements to trigger fallback (e.g.,
Anthropic streaming disabled due to a litellm bug, Gemini disables streaming
when `tools` are present, and Ollama streaming is unreliable with tools);
replace the universal claim with language emphasizing capability-based fallback
and list those provider-specific exceptions so readers understand when streaming
may be disabled.
Summary
This PR documents the smart-fallback streaming behavior introduced in PraisonAI PR #1734 to fix Issue #1733.
Changes
Updated docs/features/streaming.mdx:
✅ Quick Start leads with auto-detect example - Shows the new default stream=None behavior
✅ Expanded Streaming vs Non-Streaming table - Includes three modes: None (default), True, False
✅ Added Sync vs Async Adapters callout - Explains async vs sync streaming behavior differences
✅ Updated Troubleshooting section - Added entry for Streaming is not supported in sync OpenAIAdapter error
✅ Enhanced Best Practices - Recommends letting SDK pick streaming mode automatically
✅ Updated method comparison table - Highlights auto-detect as the recommended approach
Key Changes
Smart fallback behavior (sync only): When stream=None (new default), the SDK tries streaming first. If the adapter raises ValueError(Streaming is not supported...), it logs debug and retries with stream=False.
Behavior matrix:
References
🤖 Generated with Claude Code
Summary by CodeRabbit