feat: add support for sayStream listener argument#2841
feat: add support for sayStream listener argument#2841WilliamBergamin wants to merge 12 commits intomainfrom
Conversation
| export interface SayStreamArguments { | ||
| buffer_size?: number; | ||
| channel?: string; | ||
| thread_ts?: string; | ||
| recipient_team_id?: string; | ||
| recipient_user_id?: string; | ||
| } |
There was a problem hiding this comment.
I made Bolt define its own type but maybe it better is we reused the one from WebClient.chatStream, what do y'all think?
There was a problem hiding this comment.
@WilliamBergamin Is it possible to define this as something like:
export interface SayStreamArguments = ArgumentType<WebClient['chatStream']>;Since I agree it's clear to export this as SayStreamArguments but it'd be nice to find matching @jsdoc perhaps too?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2841 +/- ##
==========================================
+ Coverage 93.50% 93.58% +0.07%
==========================================
Files 42 43 +1
Lines 7747 7841 +94
Branches 678 689 +11
==========================================
+ Hits 7244 7338 +94
Misses 498 498
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zimeg
left a comment
There was a problem hiding this comment.
@WilliamBergamin Amazing efforts more toward surfacing the latest 🤖 ✨
I'm approving this now with a few thoughts toward exported types and tests but this is good stuff 🚢
| export interface SayStreamArguments { | ||
| buffer_size?: number; | ||
| channel?: string; | ||
| thread_ts?: string; | ||
| recipient_team_id?: string; | ||
| recipient_user_id?: string; | ||
| } |
There was a problem hiding this comment.
@WilliamBergamin Is it possible to define this as something like:
export interface SayStreamArguments = ArgumentType<WebClient['chatStream']>;Since I agree it's clear to export this as SayStreamArguments but it'd be nice to find matching @jsdoc perhaps too?
|
|
||
| /** | ||
| * Creates utility `sayStream()` to stream responses in the assistant thread. | ||
| * https://api.slack.com/methods/chat.stream |
There was a problem hiding this comment.
| * https://api.slack.com/methods/chat.stream | |
| * https://docs.slack.dev/tools/bolt-js/concepts/message-sending#streaming-messages |
📚 suggestion: I'm uncertain of the absolute link we use but this might be most current?
| assert.exists(assistantArgs.sayStream); | ||
| assert.exists(assistantArgs.setStatus); |
There was a problem hiding this comment.
🧮 quibble: Here and in a few other places the alphabetics are off. No blocker but might be a quick change?
There was a problem hiding this comment.
🧪 suggestion: It'd be nice to include another listener type here for perhaps expected asserts? I'm not certain what correct behavior is at a glance, but I find the action listeners don't include this argument:
app.action('button_click', async ({ ack, sayStream }) => {
await ack();
const stream = sayStream({ buffer_size: 100 });
// ...
});| /** | ||
| * Extracts ts from the event payload. | ||
| */ | ||
| export function extractEventTs<T extends string>(event: KnownEventFromType<T>): string | undefined { | ||
| if (hasStringProperty(event, 'ts')) { | ||
| return event.ts; | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
🔍 praise: I like the simple implementation to begin! Let's continue to enhance these extractions as edge cases appear I think?
Summary
This PR aims to introduce a new listener argument utility
sayStream, it enables developers to easily use a WebClient.chatStream object initialized with logical default values.sayStreamisWebClient.chatStreaminitialized withchannel_id: from the event payloadthread_ts:thread_tsfrom the event payload or falls back to thetsvalue if it is availablerecipient_team_id: theteam_idfrom the event received or theenterprise_idif the app is installed on the orgrecipient_user_id: theuser_idfrom the event receivedsayStreamis available onapp.eventand theassistantlisteners, if Bolt fails to extractchannel_idorthread_tsthensayStreamwill beundefinedTesting
npm pack .sayStreamSample app.ts
manifest.json
{ "_metadata": { "major_version": 1, "minor_version": 1 }, "display_information": { "name": "SayStreamTest" }, "features": { "app_home": { "home_tab_enabled": false, "messages_tab_enabled": true, "messages_tab_read_only_enabled": false }, "bot_user": { "display_name": "SayStreamTest", "always_online": false } }, "oauth_config": { "scopes": { "bot": [ "app_mentions:read", "chat:write", "im:read", "im:write", "channels:history", "im:history" ] } }, "settings": { "event_subscriptions": { "bot_events": [ "app_mention", "message.im" ] }, "interactivity": { "is_enabled": true }, "org_deploy_enabled": true, "socket_mode_enabled": true, "token_rotation_enabled": false } }Requirements (place an
xin each[ ])