Skip to content

feat(ai): keep attachment/link-only messages in to_ai_messages, add ChatTool.name (#198) - #281

Merged
patrick-chinchill merged 5 commits into
mainfrom
sync/4.41-c9
Oct 1, 2026
Merged

patrick-chinchill merged 5 commits into
mainfrom
sync/4.41-c9

Conversation

@patrick-chinchill

@patrick-chinchill patrick-chinchill commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

to_ai_messages used to drop every message whose text was empty or whitespace-only, so caption-less images, text-file uploads and link-only messages never reached the model. It now keeps any message with usable content (text, a fetchable image or text file, or a link preview) and skips only messages whose built content is empty, before transform_message runs. ChatTool gains name (upstream's framework-agnostic ChatToolSpec.name).

Upstream commits mapped

Upstream Python
25f30998 fix(chat): keep attachment/link-only messages in toAiMessages (#713, chat@4.35.0) Pre-filter removed. Name prefix only when there is text. Link-only content is Links:\n… with no leading blank line. Leading text part only when there is text or links. Empty-content skip (whitespace-only string / empty list) before transform_message. on_unsupported_attachment still fires for video/audio on text-less messages.
21dc60c3 refactor(chat): define chat/ai tools once as framework-agnostic specs (#935, chat@4.41.0) Adapted: ChatTool.name: str = "" (last field), set by every factory to its camelCase id. Shared helpers (_render_link_for_prompt, MIME helpers, _sort_by_date_sent, _build_message_text, _is_unsupported_attachment, _attachment_to_part) moved to private chat_sdk.ai.message_content (upstream ai/message-content.ts). The AI-SDK toAiTool / TanStack wrappers are TS-only (#203).
eddcd7e4 fix(telegram): return portable file data (#828, chat@4.39.0) messages.test.ts case only: ported with bytearray and memoryview fetch_data results, asserting data:image/png;base64,AQID. Telegram adapter half is #228.

Tests ported

packages/chat/src/ai/messages.test.ts → tests/test_ai_messages.py:

  • keeps image-only messages that have no text
  • keeps interleaved text and image-only messages
  • keeps link-only messages that have no text
  • skips video-only messages with no text and reports the attachment
  • skips image-only messages with no text when fetchData is unavailable
  • keeps link-only assistant messages with no text
  • does not add a text part when an image-only message has no text
  • skips messages with no text, attachments, or links
  • uses ArrayBuffer attachment data without Buffer conversion (parametrized over bytearray / memoryview)

Python-specific:

  • test_ai_messages.py: the transform never sees a skipped empty message; no [name]: prefix on link-only messages with include_names; text-less text-file + link message leads with the Links: part; the whitespace check uses JS trim's set (BOM-only skipped, NEL-only kept); a BOM-only message is also skipped with include_names (catches a str.strip() regression in the per-message text check directly).
  • test_ai_tools.py TestToolNames: every built tool's name equals its key; a name override is ignored and not stashed in extras; positional ChatTool(...) construction is unchanged.

The existing "filters out empty and whitespace-only text" and "returns empty array when all messages have empty text" tests pass unchanged. The 9 new upstream-named tests fail against origin/main's messages.py (checked by swapping the file in): 8 behavior tests plus the JS-whitespace one. The ArrayBuffer test already passed there, as the issue expected.

Fidelity

Delta vs committed report (HEAD): missing 167 -> 158 (-9)
packages/chat/src/ai/messages.test.ts: 9 -> 0 (-9)

Strict pin (chat@4.31.0): 733/733 (all TS tests have Python equivalents).

Divergences

No new non-parity table rows. Recorded in docs/UPSTREAM_SYNC.md under "AI messages without text and tool names (#198)":

  • "name" is added to _PROTECTED_TOOL_FIELDS. This is Python-only, because upstream's AI SDK tools have no name. It keeps name in sync with the dict key.
  • Bytes-like attachment data is always inlined as a base64 data: URL. Upstream passes an ArrayBuffer through raw. As before, an unnamed attachment gets filename="".
  • "Whitespace-only" uses JS trim's code-point set (shared._js_compat.JS_WHITESPACE) rather than str.strip(), to match upstream exactly.

Consumer impact

Low. This only affects chat_sdk.ai users; there is no Slack/Teams streaming impact.

  • to_ai_messages can return more messages than before: image-, text-file- and link-only messages are now included.
  • Multipart content can now start with a file part when the message has no text.
  • New ChatTool.name field. It is the last field, so positional and keyword construction keeps working. overrides cannot change it.
  • TEXT_MIME_PREFIXES is still importable from chat_sdk.ai and chat_sdk.ai.messages.

Closes #198
Part of #184

Merge gate

Review findings (2 reviewers, both on tests/test_ai_messages.py:875)

  • Fixed: the BOM case was a raw, invisible U+FEFF inside the string literal. It is now spelled "\ufeff", like the neighbouring "\x85".
  • Fixed: the BOM case alone did not kill a has_text = msg.text.strip() != "" mutation, because the downstream empty-content check strips it again. Added test_bom_only_message_skipped_with_include_names; with the mutation applied it fails (content would be "[testuser]: \ufeff"), and it passes on the real code.
  • Declined: none.

gpt-6-astra: 2 rounds, both clean.

  • Round 1 on 8b08fc8: "No actionable regressions were found against the supplied merge base."
  • Round 2 on final HEAD 3737469 (empty commit to re-trigger Lint on the non-draft PR; same tree as 8b08fc8): "No actionable regressions were found."

Bots: CodeRabbit skipped the draft and was rate-limited after ready-for-review; no inline comments or reviews from CodeRabbit or gemini.

CI on 3737469: all green (Lint & Type Check incl. strict fidelity + pyrefly, test 3.12/3.13, CodeQL).

Local full validation: ruff check/format, test-quality audit, --check-docs, strict fidelity 733/733, pytest 6692 passed / 24 skipped, pyrefly 0 errors. origin/main (f2cf21b) already merged; fidelity_target.json regenerated (Delta vs committed report (HEAD): missing 158 -> 158 (+0); one extra Python test).

Pre-merge main integration (e24a4a2): merged origin/main (1ee200f: #272 lifecycle events, #229 Discord, #273 WhatsApp). Only conflict was scripts/fidelity_target.json, regenerated per convention: vs main's committed report, target missing 160 -> 151 (all 9 ai/messages.test.ts gaps closed by this PR). The PR's src/+tests/ diff vs origin/main is byte-identical to the astra-reviewed diff on 3737469; main's changes are disjoint from chat_sdk/ai/ (types.py changes are additive event types only). Local full validation: ruff, format, audit, --check-docs, strict fidelity 733/733, pytest 6869 passed / 24 skipped, pyrefly 0 errors.

…hatTool.name (#198)

Ports vercel/chat 25f30998 (#713, chat@4.35.0), adapts 21dc60c3 (#935,
chat@4.41.0) as ChatTool.name + private ai/message_content module, and
mirrors the messages.test.ts case of eddcd7e4 (#828, chat@4.39.0) with
bytes-like fetch_data results.

Part of #184
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 18 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: ebb676f9-95ee-41d8-9052-3a17e5d16596

📥 Commits

Reviewing files that changed from the base of the PR and between 1ee200f and e24a4a2.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • README.md
  • docs/UPSTREAM_SYNC.md
  • scripts/fidelity_target.json
  • src/chat_sdk/ai/message_content.py
  • src/chat_sdk/ai/messages.py
  • src/chat_sdk/ai/tools.py
  • tests/test_ai_messages.py
  • tests/test_ai_tools.py
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@patrick-chinchill
patrick-chinchill marked this pull request as ready for review October 1, 2026 05:26
@patrick-chinchill

Copy link
Copy Markdown
Collaborator Author

Merge gate: CI green on e24a4a2 (Lint & Type Check, test (3.12), test (3.13), CodeQL, Analyze (python), Analyze (actions)); local Codex review (gpt-6-astra, xhigh, --base origin/main) on 3737469: "No actionable regressions were found. All 143 targeted tests passed, along with focused Ruff checks and project-wide Pyrefly checking."; 3 astra rounds, all clean. e24a4a2 only merges origin/main (#272/#229/#273) and regenerates fidelity_target.json; the PR's src/+tests/ diff vs origin/main is byte-identical to the reviewed one and main's changes are disjoint from chat_sdk/ai/, so no re-review was needed. Local full validation on e24a4a2: pytest 6869 passed / 24 skipped, strict fidelity 733/733, pyrefly 0 errors. Bots: CodeRabbit rate-limited (no review). Merging with --admin (Protect Main requires a code-owner approval).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[4.41/C9] AI messages: keep attachment/link-only messages, framework-agnostic tool specs

1 participant