Docs: agents and assistants#2295
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2295 +/- ##
=======================================
Coverage 92.49% 92.49%
=======================================
Files 36 36
Lines 7472 7472
Branches 651 651
=======================================
Hits 6911 6911
Misses 553 553
Partials 8 8 ☔ View full report in Codecov by Sentry. |
| You can store context through the `threadContextStore` property but it must feature `get` and `save` methods. | ||
|
|
||
| ```js | ||
| threadContextStore: { |
There was a problem hiding this comment.
I would put this property either at the top or the bottom of the instantiation: it's not part of the "main" events, and is an addition to make life easier.
Co-authored-by: Alissa Renz <[email protected]>
filmaj
left a comment
There was a problem hiding this comment.
Left a first round of comments, thanks for tackling this!
docs/content/basic/app-assistant.md
Outdated
| }); | ||
| ``` | ||
|
|
||
| You can store context through the `threadContextStore` property but it must feature `get` and `save` methods. If not provided, a `DefaultThreadContextStore` instance is utilized instead, which is a reference implementation that relies on storing and retrieving message metadata as the context changes. |
There was a problem hiding this comment.
I think we need to define what 'context' means here. It is such a loaded term in programming in general but also just in Bolt: there is a separate advanced Bolt concept "Adding context" that is different - but maybe related? - from the context concept in this doc! I'd probably break out defining context into its own sub-section.
@misscoded how should we reconcile the two ideas of "context" here? Maybe it's the same idea just extended? Not sure, but we probably need to elaborate somewhat.
Another thing: we should describe why developers might need a context store. This was one of the first questions that came up for me when I reviewed this functionality in Bolt earlier this week, and @seratch provided a great answer that I think we should leverage in these docs. The key for me was: while the two assistant_* events do provide the Slack-client context information (that is, where is the end-user situated within the workspace when they interact with the Assistant view, e.g. what channel do they have open side-by-side with the assistant view?), crucially the assistant-thread-message events delivered to the app do not. Therefore, as a developer, if you're trying to build a nice experience that flows across all three assistant events, you likely have to keep track of this context as different combinations of these three assistant events get delivered to your app. The context store, then, is more about filling that context gap when assistant message events get delivered to an app.
There was a problem hiding this comment.
I think it's important whenever talking about context as it relates to an Assistant as "thread context" to distinguish it from the event context that is otherwise provided.
docs/content/basic/app-assistant.md
Outdated
|
|
||
| ```ts | ||
| const assistant = new Assistant({ | ||
| threadContextStore: { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
The default one is purely in-memory, right? If so, it will not work well when used with the AwsLambdaReceiver, since each event invocation spins up a fresh Bolt instance, meaning the default thread context store will be empty each time. If so, we might need a callout here to instruct users of the AwsLambdaReceiver to not use the default one.
There was a problem hiding this comment.
@filmaj the default one uses the bot's first reply's message metadata to remember the latest context data, so you can safely use it for any situation.
seratch
left a comment
There was a problem hiding this comment.
My final comments: as mentioned in one of the comments, let's merge this once the bolt-js release enabling the metadata setting for the say method is done.
filmaj
left a comment
There was a problem hiding this comment.
LGTM, left a few comments.
docs/content/basic/app-assistant.md
Outdated
|
|
||
| ```ts | ||
| const assistant = new Assistant({ | ||
| threadContextStore: { |
There was a problem hiding this comment.
The default one is purely in-memory, right? If so, it will not work well when used with the AwsLambdaReceiver, since each event invocation spins up a fresh Bolt instance, meaning the default thread context store will be empty each time. If so, we might need a callout here to instruct users of the AwsLambdaReceiver to not use the default one.
Summary
This adds a page for utilizing assistants/agents. I ended up writing a lot, then removing most of it, and now we're here. I think this might still be too much but I am open to adding/removing/modifying/etcing
I built this off of words from Alissa and Kaz — once we're good to go here, however this ends up, I'll tweak the Python page to align
To get a local version of the site:
git checkout docs-luke-assistantscd docsnpm inpm run startor just read the markdown
Requirements (place an
xin each[ ])