feat(types): add streaming_state to bot_message event#2371
Conversation
zimeg
added
semver:minor
enhancement
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ai-apps #2371 +/- ##
==========================================
Coverage ? 92.78%
==========================================
Files ? 39
Lines ? 10711
Branches ? 692
==========================================
Hits ? 9938
Misses ? 761
Partials ? 12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
mwbrooks
left a comment
mwbrooks
left a comment
There was a problem hiding this comment.
✅ Thanks a bunch for adding this event type!
🧪 Confirmed that streaming_state appears as a type for the message event.
❓ Should this belong to more than just the BotMessageEvent? For example, the MessageChangedEvent?
| event_ts: string; | ||
| channel: string; | ||
| channel_type: ChannelTypes; | ||
| streaming_state?: 'in_progress' | 'completed' | 'errored'; |
There was a problem hiding this comment.
✅ Matches the schema! 👌🏻
nit: We may want to pull the values out into an enum at the top of the file, similar to ChannelTypes.
There was a problem hiding this comment.
❓ Should this belong to more than just the
BotMessageEvent? For example, theMessageChangedEvent?nit: We may want to pull the values out into an enum at the top of the file, similar to
ChannelTypes.
@mwbrooks I was curious about this as well - at the moment I find a "bot_message" subtype within a "message_changed" event as the message but I'm not sure if it can be found in other events...
node-slack-sdk/packages/types/src/events/message.ts
Lines 230 to 240 in 4b8bfec
Am curious about this and think we could adjust to a separate enum if this is found elsewhere? 👻
There was a problem hiding this comment.
note: @zimeg and I chatted offline to make sure that message_changed events from bots will include the streaming_state arg. This arg isn't available on non-bot message changes, so it's looking good.
note: We won't bother turning the values into an enum until we repeat it in 2 places. 👌🏻
|
Merge away @zimeg! 🚀 |
|
@mwbrooks Thanks so much for sharing in these discussions and the kind review! I like what we have for now but am so open to revisiting in a change of events. |
2 participants
Summary
This PR adds
streaming_stateto thebot_messagemessage event subtype.Reviewers
Confirm that this field appears in message changed event 🤖
Requirements