feat: accept chunks as arguments to chat.{start,append,stop}Stream methods#2467
feat: accept chunks as arguments to chat.{start,append,stop}Stream methods#2467srtaalej merged 10 commits intofeat-ai-apps-thinking-stepsfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat-ai-apps-thinking-steps #2467 +/- ##
============================================================
Coverage 93.09% 93.09%
============================================================
Files 40 40
Lines 11240 11240
Branches 713 713
============================================================
Hits 10464 10464
Misses 764 764
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
zimeg
left a comment
There was a problem hiding this comment.
@srtaalej Woooo! This'll be nice to land soon but I'm leaving a few comments on changes perhaps to make beforehand.
The tests are most confusing to me since I think all changes are contained in this PR... Am looking into this separate but do let me know if questions on these comments appear please!
packages/types/src/chunk.ts
Outdated
| if(type === 'plan_update') { | ||
| if (typeof chunkObj.title === 'string') { | ||
| return chunkObj as unknown as PlanUpdateChunk; | ||
| } | ||
| console.warn('Invalid PlanUpdateChunk (missing title property)', chunk); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Co-authored-by: Eden Zimbelman <[email protected]>
Co-authored-by: Eden Zimbelman <[email protected]>
mwbrooks
left a comment
There was a problem hiding this comment.
🙌🏻 This is looking great @srtaalej! Thanks for aligning so closely with the Python SDK implement as well.
🧪 I've double-checked that the params are correct and thanks for adding the detailed tests to chat.test-d.ts. 🙇🏻
Here is my modification to our sample app's listeners/assistant/message.ts method to test the chat.startStream, chat.appendStream, and chat.stopStream with markdown_text, and task_update. I don't think I can test plan_update until the PR that adds task_display_mode.
listeners/assistant/message.ts
import { DEFAULT_SYSTEM_CONTENT, openai } from '../../ai/index.js';
import { feedbackBlock } from '../views/feedback_block.js';
const delay = (ms) => new Promise(resolve => setTimeout(resolve, ms));
/**
* The `message` event is sent when the user direct messages the app in a DM or Assistant container.
*
* @param {Object} params
* @param {import("@slack/web-api").WebClient} params.client - Slack web client.
* @param {import("@slack/bolt").Context} params.context - Event context.
* @param {import("@slack/logger").Logger} params.logger - Logger instance.
* @param {import("@slack/types").MessageEvent} params.message - The incoming message.
* @param {Function} params.getThreadContext - Function to get thread context.
* @param {import("@slack/bolt").SayFn} params.say - Function to send messages.
* @param {Function} params.setTitle - Function to set assistant thread title.
* @param {Function} params.setStatus - Function to set assistant status.
*
* @see {@link https://docs.slack.dev/reference/events/message}
*/
export const message = async ({ client, context, logger, message, getThreadContext, say, setTitle, setStatus }) => {
/**
* Messages sent to the Assistant can have a specific message subtype.
*
* Here we check that the message has "text" and was sent to a thread to
* skip unexpected message subtypes.
*
* @see {@link https://docs.slack.dev/reference/events/message#subtypes}
*/
if (!('text' in message) || !('thread_ts' in message) || !message.text || !message.thread_ts) {
return;
}
const { channel, thread_ts } = message;
const { userId, teamId } = context;
try {
/**
* Set the title of the Assistant thread to capture the initial topic/question
* as a way to facilitate future reference by the user.
*
* @see {@link https://docs.slack.dev/reference/methods/assistant.threads.setTitle}
*/
await setTitle(message.text);
/**
* Set the status of the Assistant to give the appearance of active processing.
*
* @see {@link https://docs.slack.dev/reference/methods/assistant.threads.setStatus}
*/
await setStatus({
status: 'thinking...',
loading_messages: [
'Teaching the hamsters to type faster…',
'Untangling the internet cables…',
'Consulting the office goldfish…',
'Polishing up the response just for you…',
'Convincing the AI to stop overthinking…',
],
});
const streamResponse = await client.chat.startStream({
channel: channel,
thread_ts: thread_ts,
recipient_team_id: teamId,
recipient_user_id: userId,
chunks: [{
type: 'markdown_text',
text: '**Hello!** I am starting to process your request...\n\n',
}, {
type: 'task_update',
id: '12',
title: 'counting bytes...',
status: 'in_progress',
}],
});
await delay(4000);
await client.chat.appendStream({
channel: channel,
ts: streamResponse.ts,
// markdown_text: '',
chunks: [{
type: 'task_update',
id: '12',
title: 'adding numbers...',
status: 'in_progress',
details: 'sums have increased',
}],
});
await delay(4000);
await client.chat.stopStream({
channel: channel,
ts: streamResponse.ts,
chunks: [{
type: 'task_update',
id: '12',
title: 'solved equation!',
status: 'complete',
sources: [{
type: 'url',
url: 'https://oeis.org',
text: 'The On-Line Encyclopedia of Integer Sequences (OEIS)',
}],
}, {
type: 'markdown_text',
text: 'that computes.',
}],
});
} catch (e) {
logger.error(e);
// Send message to advise user and clear processing status if a failure occurs
await say({ text: `Sorry, something went wrong! ${e}` });
}
};📝 I left a suggestion to remove the parseChunk and npm dependency. After that's done, I think we can ✅ and merge! ![]()
packages/types/src/chunk.ts
Outdated
| /** | ||
| * Parse a chunk object and return the appropriate typed chunk. | ||
| * Returns null if the chunk is invalid or unknown. | ||
| */ | ||
| export function parseChunk(chunk: unknown): AnyChunk | null { |
There was a problem hiding this comment.
suggestion: This is a nice implementation, but I don't think we need it for the @slack/types package. This parsing was necessary for Python, but in TypeScript I think the AnyChunk union that you created will serve well! :two-cents:
There was a problem hiding this comment.
ahh okay thank you for the clarification 🙌
packages/types/package.json
Outdated
| "dependencies": { | ||
| "@slack/logger": "^4.0.0" | ||
| }, |
There was a problem hiding this comment.
suggestion: If we remove the parseChunk method, then we can also remove this dependency 🎉
There was a problem hiding this comment.
suggestion: @srtaalej I manually linked the new @slack/types into this package and ran npm test. It looks like this test now no longer errors because markdown_text is now optional. However, I'd still expect it to error when markdown_text and chunks are both missing. 🤔 @zimeg Do you think this error should come from the Slack API or should our SDK check that at least one of these two params exist?
> @slack/[email protected] test:types
> tsd
test/types/methods/chat.test-d.ts:21:0
✖ 21:0 Expected an error, but found none.
1 errorThere was a problem hiding this comment.
👁️🗨️ @mwbrooks Nice catch! I'd lean toward documenting that either markdown_text or chunks is required and letting the API error instead of leaning on typescript options.
🗣️ ramble: That might be personal preference, but IMHO these options don't catch runtime nuance that the call to an API would anyways. An empty markdown_text string IIRC causes a no_text error from API but the required markdown_text appears alright.
…thods (#2467) Co-authored-by: Eden Zimbelman <[email protected]>

Summary
This PR introduces the chunks argument to the following methods:
chat.startStream
chat.appendStream
chat.stopStream
Code Snippet
Requirements (place an
xin each[ ])