feat(teams): deliver outbound files via data-URI activity attachments#125
feat(teams): deliver outbound files via data-URI activity attachments#125patrick-chinchill wants to merge 3 commits into
Conversation
Port upstream filesToAttachments (packages/adapter-teams/src/index.ts
~1006-1035) to Python. The Teams adapter previously dropped a Postable's
.files entirely -- post_message/edit_message never called extract_files,
so execution artifacts silently vanished.
Adds TeamsAdapter._files_to_attachments: resolves each FileUpload's bytes
via to_buffer(..., throw_on_unsupported=False), base64-encodes them, and
builds a Bot Framework attachment {contentType, contentUrl:
data:<mime>;base64,<b64>, name}. mime defaults to
application/octet-stream; files whose data can't be resolved to bytes are
skipped with a debug log (mirrors upstream's `if (!buffer) continue`).
post_message and edit_message now call extract_files and attach the
results: the adaptive-card branch appends file attachments after the card
attachment; the text branch sets attachments to the file attachments when
present.
Adds TestFileAttachments (5 tests): text+file, card+file (both
attachments present), edit_message+file, octet-stream default, and the
skip-unresolvable-bytes branch.
Bumps version to 0.4.29a3 and adds a CHANGELOG entry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 29 minutes and 50 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 implements outbound file delivery for the Teams adapter by converting FileUpload objects into base64 data-URI activity attachments. It introduces the _files_to_attachments helper method, integrates it into post_message and edit_message, and adds comprehensive unit tests to cover various attachment scenarios and edge cases. As there are no review comments, I have no additional feedback to provide.
The initial port wired filesToAttachments into both post_message AND edit_message. Upstream vercel/chat wires it into postMessage + postChannelMessage only — editMessage never carries files. And chinchill delivers execution artifacts via a fresh post(), never by editing files into an existing message. Carrying files in edit_message was an unrequested divergence (and an edit_message+files test has no upstream counterpart, which the repo's verify_test_fidelity gate would flag). Revert edit_message to file-free, mirroring upstream. Replace the edit_message+file test with a fidelity guard asserting edit_message carries no file attachments. post_message file delivery (the actual parity fix) is unchanged. Full suite: 4052 passed. pyrefly clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
… caveats Self-review (two adversarial reviewers) found: - MEDIUM: multi-file handling untested — mutation showed return attachments[:1] (drop all but first) and reversed() both passed green, so a regression that drops/reorders artifacts would merge undetected, defeating the parity goal. Added test_multiple_files_attached_in_order (N files -> N attachments, input order) + test_partial_skip_preserves_surviving_files (good/bad/good -> survivors in order). Mutation-verified: [:1] now FAILS the multi-file test. - LOW: the skip test's was hollow (post_message emits an unconditional 'send (message)' debug, so it passed even if the skip branch logged nothing). Now asserts the SPECIFIC 'unsupported data' skip log. - Tightened CHANGELOG caveats: Bot Framework documents data-URI contentUrl as IMAGE-ONLY — image/* (charts) render inline, but text/csv/application/pdf may NOT surface on Teams (which uses a file-consent/hosted-URL flow for arbitrary files). So this delivers reliable Teams parity for images; non-image delivery may need the file-consent path as a follow-up. Also noted the ~256KB data-URI ceiling. Verify in a real Teams tenant before relying on either. Full suite: 4054 passed. ruff + pyrefly clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
What
Ports upstream
vercel/chat'sfilesToAttachments(packages/adapter-teams/src/index.tslines ~1006-1035) to the Python Teams adapter. Until now the Teams adapter silently dropped aPostable's.files:post_message/edit_messagenever calledextract_files, so execution artifacts (CSVs, charts, etc.) never reached Teams.This is the Teams half of outbound artifact parity (the Slack half landed in #103/#117). It unblocks chinchill routing execution artifacts to Teams.
How
TeamsAdapter._files_to_attachments(files)mirrors upstreamfilesToAttachments: for eachFileUpload, resolve bytes viato_buffer(file.data, "teams", throw_on_unsupported=False), base64-encode via the existingbuffer_to_data_uri, and build a Bot Framework attachment{"contentType": mime, "contentUrl": "data:<mime>;base64,<b64>", "name": filename}.mimedefaults toapplication/octet-stream. Files whose data can't be resolved to bytes are skipped with a debug log (mirrors upstream'sthrowOnUnsupported: false+if (!buffer) continue).post_messageandedit_messagenow callextract_files(message)near the top and computefile_attachments. The adaptive-card branch appends the file attachments after the card attachment ([card, *file_attachments]); the text branch setsattachmentsto the file attachments when non-empty.Caveat (inherited from upstream's design)
Data-URI attachments inline the full file payload base64-encoded inside the Bot Framework activity, so large files inflate the activity payload. This is the same size constraint upstream
vercel/chatcarries with its data-URI design; no chunking/upload-session path is added here (parity over redesign).Deviations from upstream (deliberate)
fetch_dataon PythonFileUpload. The upstream TSFileUpload.dataisBuffer | Blob | ArrayBuffer; the PythonFileUpload(src/chat_sdk/types.py) carries only inlinedata: bytes(fetch_datalives onAttachment, notFileUpload). So the helper resolvesdataonly — matching the sibling Slack_upload_files(file.data). The planned "file withfetch_data" test is swapped for a skip-unresolvable-bytes test, which exercises the genuine Python analog of upstream'sif (!buffer) continue.edit_messagefile delivery is a deliberate, task-directed superset of this upstream version. In the pinned upstream checkout,filesToAttachmentsis wired intopostMessageandpostChannelMessage— noteditMessage. The Python adapter has nopost_channel_message(so that upstream site has no port).edit_messagefile delivery is added here per outbound-artifact-parity intent and is covered by a dedicated test; it reads as an intentional superset, not an accidental over-port.Tests
TestFileAttachmentsintests/test_teams_adapter.py(5 tests): text+file, adaptive-card+file (both attachments present),edit_message+file, mime-type default toapplication/octet-stream, and the skip-unresolvable-bytes branch. Reverting theextract_fileswiring makes the four delivery tests fail (verified).Checks
4052 passed, 3 skipped(was ~4046; +5 new tests, +1).ruff check/ruff format --checkclean;pyrefly check0 errors;audit_test_quality.py0 hard failures.0.4.29a2->0.4.29a3; CHANGELOG entry added.🤖 Generated with Claude Code