fix(adapters): accept dict StreamChunks in slack/github/google_chat streams#105
Conversation
…treams
`_from_full_stream` (thread.py:1218-1226) explicitly forwards
`{"type": "markdown_text" | "task_update" | "plan_update", ...}` dict
chunks unchanged, so downstream adapters MUST accept both the dataclass
and dict shapes of `StreamChunk`. The Teams adapter already does
(adapter.py:927), but slack/github/google_chat only check
`hasattr(chunk, "type")` — which is False for dicts in Python — so dict
chunks were silently dropped or, worse, raised AttributeError inside
Slack's `send_structured_chunk` and downgraded the entire stream to
text-only with a misleading warning about missing assistant_view scopes.
Changes:
- slack/adapter.py: route dict markdown_text through the renderer; add
a `_read(name)` helper in `send_structured_chunk` so dict and dataclass
field access share one code path; tighten the fallback warning to
enumerate the actual possible causes (missing scope OR malformed
chunk OR transient Slack API error) instead of pointing solely at
the manifest.
- github/adapter.py: accept dict markdown_text chunks in the accumulator.
- google_chat/adapter.py: same.
- test_slack_api.py: two regression tests in TestStream covering the two
distinct adapter branches affected (renderer dispatch + structured
chunk forwarding). Both fail when the dict branches are reverted.
This brings the Python adapters in line with the SDK's own internal
contract — adapters do not enforce a dataclass-only shape for chunks
that `_from_full_stream` is documented and tested to pass through as
dicts.
|
Warning Review limit reached
More reviews will be available in 20 minutes and 29 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. 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 (4)
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 |
![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 adds support for dict-shaped stream chunks (such as markdown_text and task_update) across the GitHub, Google Chat, and Slack adapters, ensuring they are handled similarly to their dataclass counterparts. It also refactors Slack's structured chunk processing to handle both dictionary and object shapes and adds corresponding unit tests. Feedback on the changes suggests using is not None instead of a truthiness check on text_value in the Slack adapter to avoid silently ignoring empty strings.
| if isinstance(text_value, str) and text_value: | ||
| await push_text_and_flush(text_value) |
There was a problem hiding this comment.
The truthiness check and text_value silently ignores empty strings (""), which are falsy but valid string values. To avoid silently ignoring valid falsy values, we should use is not None instead of a truthiness check.
| if isinstance(text_value, str) and text_value: | |
| await push_text_and_flush(text_value) | |
| if text_value is not None: | |
| await push_text_and_flush(text_value) |
References
- 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.
![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: ed1deaf0ab
ℹ️ 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".
| elif isinstance(chunk, dict) and chunk.get("type") == "markdown_text": | ||
| # Dict-shaped StreamChunks are part of the contract: the | ||
| # `_from_full_stream` normalizer in thread.py forwards dict | ||
| # `{type: "markdown_text", ...}` items unchanged, so adapters | ||
| # must handle them the same as the dataclass form. | ||
| text_value = chunk.get("text") | ||
| if isinstance(text_value, str) and text_value: | ||
| await push_text_and_flush(text_value) |
There was a problem hiding this comment.
When Slack streaming is invoked for a top-level DM/channel id with no thread_ts, this new dict handling is never reached because the method returns through the earlier post_message fallback, whose accumulator still only accepts strings and dataclass chunks. Since _from_full_stream forwards {"type":"markdown_text", "text": ...} dict chunks unchanged, those streams post an empty/partial fallback message instead of the rendered text; mirror this branch in the no-thread accumulator as well.
Useful? React with 👍 / 👎.
Synced to upstream vercel/chat@4.27.0 (release commit f55378a). Upstream parity ports: chat.get_user across 8 adapters (#90), Slack Socket Mode (#86, closes #68), Slack dynamic bot_token resolver + webhook_verifier (#87), Teams native DM streaming (#88), Telegram MarkdownV2 (#89), Discord card text dedup (#89), Slack streaming team_id + Teams DM Graph conversation IDs (#85), Slack adapter bug-fix sweep (#89), ExternalSelect.initial_option + option_groups (#84), Slack @mention email-safe regex (#91). Python-only improvements: markdown parser completeness (#101), streaming markdown list/table chunk-boundary (#99), Slack files_upload_v2 kwarg fix (#103), dict-shaped StreamChunk support (#105), Google Chat card-text markdown rendering (#92), adapter init logs + adapter-list in not-found errors (#104). Sync-process documentation captures the review-loop discipline lessons from this wave (docs/UPSTREAM_SYNC.md + docs/SELF_REVIEW.md). Fidelity workflow stays pinned to chat@4.26.0 — upstream did not publish a chat@4.27.0 tag for the Apr 30 monorepo cut. 4036 tests pass, 3 skipped, 0 failed.
3 participants
Summary
Replacement PR for #97 (could not push merge resolution to tony's fork from the automated session). Original commits by @tony-chinchill-ai preserved in branch history. Adds:
style: ruff format on test_slack_api.py— the only blocker on the CI lint step that's been failing on fix(adapters): accept dict StreamChunks in slack/github/google_chat streams #97main(post-fix(markdown): chat-scoped completeness — escaped chars, task lists, math/escape in_remend#101) into the branchChanges (unchanged from #97)
Aligns slack/github/google_chat stream loops with the dict-shaped
StreamChunkcontract thatthread.py(line 1218) and the Teams adapter already honor.isinstance(chunk, dict) and chunk.get("type") == "markdown_text"branch;send_structured_chunkrewritten to use a_read()helper that handles both dict and dataclass shapes; fallback warning rewritten to name the actual possible causes (missing scopes / malformed payload / transient API error) instead of pinning the blame on the app manifest.isinstance(chunk, dict)branch in each stream accumulator.Why this is the right shape
Port-correctness fix, not a new contract. In TS chat-sdk, dict-vs-dataclass distinction doesn't exist; Python's
hasattrcheck is object-only, so the port must add the dict branch explicitly. Mirrors dict branches already present inthread.py:640/661/1218andadapters/teams/adapter.py:927.Test plan
uv run ruff check src/ tests/ scripts/— cleanuv run ruff format --check src/ tests/ scripts/— clean (197/197)uv run python scripts/audit_test_quality.py— 0 hard failuresuv run pytest tests/test_slack_api.py tests/test_github_adapter.py tests/test_google_chat_adapter.py -q— 132 passedTestStream(dict markdown_text routes through renderer; dict task_update forwarded as structured chunk)Closes #97.
Generated by Claude Code
Generated by Claude Code