feat(core): optional ThinkingChunk StreamChunk variant (default-off, upstream-compatible; supersedes #39)#169
Conversation
…upstream-compatible; supersedes #39) Adds a fourth, opt-in StreamChunk variant for streaming agent reasoning/"thinking" to chat platforms. The whole design is default-off: with no opt-in, the normalized stream and the posted message are byte-for-byte identical to upstream chat@4.31. - Additive type: ThinkingChunk(type="thinking", content=str) added to the StreamChunk union. The existing three variants are unaffected. - Opt-in emit: from_full_stream(stream, emit_thinking=False) and a thread-level emit_thinking config flag (threaded into the internal _from_full_stream) surface AI-SDK reasoning/reasoning-delta (and pydantic-ai part_kind=="thinking") parts as ThinkingChunk only when enabled. Default False drops reasoning exactly as upstream does. - Graceful consume: Thread._handle_stream never accumulates a ThinkingChunk into the posted-message text, and every adapter's stream handler skips it. Slack/Teams expose an optional render_thinking hook (via shared.adapter_utils.maybe_render_thinking); the text-accumulate adapters ignore it structurally — no crash, posted message unchanged. - No state pollution: ThinkingChunk is streaming-only. Message has no thinking field, to_json() is unchanged, and a round-tripped Message is byte-identical, so cross-SDK Redis/Postgres state stays compatible. - Docs: UPSTREAM_SYNC.md Known Non-Parity row added. Rationale: upstream's chat-platform SDK drops reasoning (leaves it to the AI-SDK web UI); chinchill streams thinking to Slack/Teams out-of-band today because there is no path. This gives the SDK a first-class, opt-in one without changing any default behavior. Gauntlet: ruff check + format, audit_test_quality (0 hard failures), verify_test_fidelity --strict (732/732, 0 missing), pyrefly src (0 errors), pytest (5121 passed, 4 pre-existing skips).
|
Warning Review limit reached
More reviews will be available in 10 minutes and 12 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
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 |
| content = thinking_content(chunk) | ||
| result = hook(content) | ||
| if inspect.isawaitable(result): | ||
| await result |
There was a problem hiding this comment.
Code Review
This pull request introduces an opt-in, Python-only ThinkingChunk stream variant to surface AI-SDK and pydantic-ai reasoning parts during streaming, ensuring that thinking content is gracefully skipped by default and never persisted to history. The review feedback highlights a critical omission where emit_thinking is inaccessible in production because it is not exposed in ChatConfig or propagated by Chat. Additionally, the reviewer recommends reducing code duplication in thread.py by reusing the _pick helper to extract thinking content and using explicit is not None checks instead of truthiness checks.
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.
| # Python-only divergence (default-off). When True, raw AI-SDK | ||
| # ``reasoning`` / ``reasoning-delta`` parts passed to ``post()`` are | ||
| # surfaced as ``ThinkingChunk`` objects for adapters that render thinking. | ||
| # Default False keeps the stream byte-for-byte upstream (reasoning dropped). | ||
| emit_thinking: bool = False |
There was a problem hiding this comment.
Critical Omission: emit_thinking is inaccessible in production threads
While emit_thinking has been added to _ThreadImplConfig and threaded into _from_full_stream, there is currently no way for a user to configure emit_thinking=True on any thread instantiated by the SDK.
ChatConfig(insrc/chat_sdk/types.py) does not expose anemit_thinkingoption.Chat(insrc/chat_sdk/chat.py) does not propagate any such setting to_ThreadImplConfigwhen instantiatingThreadImpl.ThreadImpldoes not expose a public property or setter to enableemit_thinkingdynamically.
Consequently, self._emit_thinking will always remain False in production. This causes any pre-built ThinkingChunk yielded by from_full_stream(..., emit_thinking=True) to be silently discarded in _from_full_stream (since emit_thinking is False and "thinking" is not in the allowed passthrough tuple ("markdown_text", "task_update", "plan_update")).
Suggested Fix:
- Add
emit_thinking: bool = FalsetoChatConfiginsrc/chat_sdk/types.py. - Update
chat.pyto passemit_thinkingfromChatConfigto_ThreadImplConfigwhen creatingThreadImplinstances.
| if emit_thinking: | ||
| content = next( | ||
| ( | ||
| v | ||
| for k in ("content", "text", "delta", "textDelta", "text_delta") | ||
| if (v := getattr(item, k, None)) is not None | ||
| ), | ||
| "", | ||
| ) | ||
| if isinstance(content, str) and content: | ||
| yield ThinkingChunk(content=content) | ||
| continue |
There was a problem hiding this comment.
Code Duplication: Use _pick helper to extract thinking content
Instead of duplicating the next(...) extraction logic for ThinkingChunk content, you can import and reuse the _pick helper defined in chat_sdk.from_full_stream. This reduces code duplication and improves maintainability. Additionally, we should check that the extracted content is not None rather than performing a truthiness check, ensuring that falsy but valid values like empty strings are not silently ignored.
if emit_thinking:
from chat_sdk.from_full_stream import _pick
content = _pick(item, ("content", "text", "delta", "textDelta", "text_delta"))
if content is not None and isinstance(content, str):
yield ThinkingChunk(content=content)
continueReferences
- When checking for optional values that can be falsy but valid (e.g., 0, empty string, empty list), use
is not Noneinstead of a truthiness check to avoid silently ignoring them.
| if emit_thinking: | ||
| content = next( | ||
| ( | ||
| v | ||
| for k in ("content", "text", "delta", "textDelta", "text_delta") | ||
| if (v := item.get(k)) is not None | ||
| ), | ||
| "", | ||
| ) | ||
| if isinstance(content, str) and content: | ||
| yield ThinkingChunk(content=content) | ||
| continue |
There was a problem hiding this comment.
Code Duplication: Use _pick helper to extract thinking content
Instead of duplicating the next(...) extraction logic for ThinkingChunk content, you can import and reuse the _pick helper defined in chat_sdk.from_full_stream. This reduces code duplication and improves maintainability. Additionally, we should check that the extracted content is not None rather than performing a truthiness check, ensuring that falsy but valid values like empty strings are not silently ignored.
if emit_thinking:
from chat_sdk.from_full_stream import _pick
content = _pick(item, ("content", "text", "delta", "textDelta", "text_delta"))
if content is not None and isinstance(content, str):
yield ThinkingChunk(content=content)
continueReferences
- When checking for optional values that can be falsy but valid (e.g., 0, empty string, empty list), use
is not Noneinstead of a truthiness check to avoid silently ignoring them.
…-in StreamInput Revert the public ``StreamChunk`` union to upstream's three variants (``MarkdownTextChunk | TaskUpdateChunk | PlanUpdateChunk``) so a consumer doing an exhaustive ``match`` over it sees zero change on upgrade. The Python-only ``ThinkingChunk`` is no longer a member of that union. Introduce a public ``StreamInput = str | StreamChunk | ThinkingChunk`` alias for what a stream may yield, and widen only the stream-input/output boundaries to accept it: the ``Adapter.stream()`` protocol signature, ``from_full_stream``/``_from_full_stream`` returns, ``Thread._wrapped_stream``, and each receiving adapter's ``stream()`` signature. A producer can yield ``ThinkingChunk`` (opt-in) and the adapters type-check; code referencing ``StreamChunk`` itself is unaffected. All other thinking behavior is preserved byte-for-byte: ``emit_thinking`` defaults to False (default stream identical to upstream), the ``_handle_stream`` graceful skip, the adapter skip / ``render_thinking`` branches, ``maybe_render_thinking``, and the no-``Message.thinking``-field / round-trip-identical guarantee. The UPSTREAM_SYNC divergence row is updated to state that ``StreamChunk`` is NOT widened and ``ThinkingChunk`` is a separate opt-in input type. Tests now assert ``get_args(StreamChunk)`` has exactly the three upstream variants and that ``ThinkingChunk`` is excluded from ``StreamChunk`` but accepted by ``StreamInput``.
There was a problem hiding this comment.
💡 Codex Review
chat-sdk-python/src/chat_sdk/from_full_stream.py
Lines 84 to 88 in 0c52cbb
The new docstring says emit_thinking=True handles pydantic-ai parts with part_kind == "thinking", but this discriminator only checks event["type"] / .type and drops objects that do not expose type. For a pydantic-ai stream that yields parts with part_kind="thinking" and content, from_full_stream(..., emit_thinking=True) never reaches _REASONING_TYPES, so the thinking content is silently lost; the internal thread normalizer mirrors the same type-only check. Please fall back to part_kind when type is absent.
ℹ️ 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".
| # ``reasoning`` / ``reasoning-delta`` parts passed to ``post()`` are | ||
| # surfaced as ``ThinkingChunk`` objects for adapters that render thinking. | ||
| # Default False keeps the stream byte-for-byte upstream (reasoning dropped). | ||
| emit_thinking: bool = False |
There was a problem hiding this comment.
Expose the thinking opt-in through ChatConfig
This flag is never populated on threads created by the normal Chat path: ChatConfig has no emit_thinking field and Chat._create_thread constructs _ThreadImplConfig without passing it, so handler-created threads always keep the default False. In that common path, raw reasoning parts -- and even prebuilt ThinkingChunks produced by from_full_stream(..., emit_thinking=True) -- are dropped by _from_full_stream before Slack/Teams render hooks can see them. Please thread a public config/post option into this field if the feature is intended to be usable outside direct internal ThreadImpl construction.
Useful? React with 👍 / 👎.
* chore(release): cut 0.4.31 — sync to chat@4.31.0 Bumps version 0.4.30 → 0.4.31 and UPSTREAM_PARITY → 4.31.0; re-pins the fidelity check + CI clone to chat@4.31.0 (string-only — the mapped-core test files are byte-identical 4.30→4.31, 732/732 still pass). Wave content (already merged): Linear agent-sessions L1–L5 (#151), Teams SDK-free primitive subpaths, Slack 4.31 (#155), Telegram rich messages, LinkButton stable id, and the opt-in ThinkingChunk stream type (#169). Documents the new chat/adapters static catalog as an intentional non-port (npm-addressed + vendor adapters not shipped here; no consumer need). * docs(upstream-sync): correct adapters-catalog counts (19 tests, ~25 adapters)
Summary
Adds an opt-in, default-off fourth
StreamChunkvariant —ThinkingChunk— for streaming agent reasoning/"thinking" to chat platforms. Rebuilds tony's PR #39 to our standards as a clean Python-only divergence.The overriding design constraint: default behavior is BYTE-FOR-BYTE identical to upstream chat@4.31. The divergence is only active when explicitly enabled. Additive type, opt-in emit, graceful/skip consume, zero state pollution.
Why
Upstream's chat-platform SDK (
from-full-stream.ts) forwards onlytext-delta+finish-step; AI-SDKreasoning/reasoning-deltaparts fall through and are discarded — reasoning is left to the AI-SDK web UI. chinchill actively streams thinking to Slack/Teams but does it out-of-band today (intercepting the model stream before the SDK) because there is no path. This gives the SDK a first-class, opt-in one without changing any default behavior.Design
1. Additive type —
ThinkingChunk(type="thinking", content=str)added tosrc/chat_sdk/types.py; union is nowStreamChunk = MarkdownTextChunk | TaskUpdateChunk | PlanUpdateChunk | ThinkingChunk. Existing code matching the three variants is unaffected. Shape mirrors chinchill's thinking-delta model (part_kind == "thinking"with acontentstring).2. Opt-in emit (default = upstream) —
from_full_stream(stream, emit_thinking=False)gains the flag; a thread-levelemit_thinkingconfig flag threads into the internal_from_full_stream. WhenTrue, AI-SDKreasoning/reasoning-delta(and pydantic-aithinking) parts yield aThinkingChunk. WhenFalse(default), reasoning is dropped exactly as upstream — noThinkingChunkemitted, output byte-for-byte upstream.3. Graceful consume —
Thread._handle_streamnever accumulates aThinkingChunkinto the posted-message text. Every adapter's stream handler skips it: Slack/Teams expose an optionalrender_thinkinghook (viachat_sdk.shared.adapter_utils.maybe_render_thinking); the text-accumulate adapters (Discord/GitHub/Linear/WhatsApp/Google Chat/Telegram/Messenger/Twilio) ignore it structurally — no crash, posted message unchanged.4. No state pollution —
ThinkingChunkis streaming-only.Messagehas nothinkingfield,to_json()is unchanged, and a round-trippedMessageis byte-identical, so cross-SDK Redis/Postgres state shared with the TS SDK stays compatible.5. Docs —
docs/UPSTREAM_SYNC.mdKnown Non-Parity row added.Tests (each fail-on-mutation)
tests/test_from_full_stream.py::TestThinkingOptIn— flag OFF drops reasoning identically (with_reasoning == without_reasoning); flag ON yieldsThinkingChunk; interleaving; empty-reasoning skip.tests/test_thinking_chunk.py—_handle_streamgraceful skip (posted message identical with/without thinking);emit_thinkingthreading default-off vs opt-in;Messageround-trip byte-identical (no thinking in state); per-adapter no-crash sweep.tests/test_types.py::TestThinkingChunk— dataclass shape + union membership.Gauntlet
ruff format --check: cleanaudit_test_quality.py: 0 hard failuresverify_test_fidelity.py --strict(TS_ROOT=chat@4.30.0): 732/732 matched, 0 missingpyrefly check src/: 0 errorspytest: 5121 passed, 4 skipped (pre-existing skips) — no regressionSupersedes #39.