-
Notifications
You must be signed in to change notification settings - Fork 39
feat: assistant class #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔭 @srtaalej This is a solid start but request quick changes since I'm not finding the assistant class is used in testing when conversing in the assistant "pane"?
Will soon continue the review but wanted to share this first!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@srtaalej This is looking great! I left one more comment in request of some changes, but am thinking the assistant class implementation is solid 👾 ✨
manifest.json
Outdated
"assistant_thread_started", | ||
"assistant_thread_context_changed" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👁️🗨️ Nit: Can these be moved to alphabetical order for fast inspection later?
channel=channel_id, | ||
ts=waiting_message["ts"], | ||
text=f"Received an error from Bolty:\n{e}", | ||
text=f":warning: Received an error from Bolty:\n{e}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⭐ praise: This was helpful for me in testing just now!
if e.response["error"] == "not_in_channel": | ||
# If this app's bot user is not in the public channel, | ||
# we'll try joining the channel and then calling the same API again | ||
client.conversations_join(channel=referred_channel_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 suggestion: The channels:join
scope is required to use this method and might be good to include in the app manifest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for catching this!
# Initialization | ||
app = App(token=os.environ.get("SLACK_BOT_TOKEN")) | ||
|
||
logging.basicConfig(level=logging.DEBUG) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 question: I notice debug logs aren't appearing for me... Does the ordering of logger initialization matter for this?
🗣️ thought: We might hold off on such changes for another PR but this diff
caught my eye for now-
prompts: List[Dict[str, str]] = [ | ||
{ | ||
"title": "What does Slack stand for?", | ||
"message": "Slack, a business communication service, was named after an acronym. Can you guess what it stands for?", | ||
}, | ||
{ | ||
"title": "Write a draft announcement", | ||
"message": "Can you write a draft announcement about a new feature my team just released? It must include how impactful it is.", | ||
}, | ||
{ | ||
"title": "Suggest names for my Slack app", | ||
"message": "Can you suggest a few names for my Slack app? The app helps my teammates better organize information and plan priorities and action items.", | ||
}, | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⭐ praise: Nice alignment with the assistant template - it makes me wonder if we wonder how maintenance and updates of both projects might continue going forward...
Type of change (place an x in the [ ] that applies)
Summary
Enables Assistant class features while keeping the other options for messaging available. Updates the README to highlight assistant implementation in
listeners/assistant
.Requirements (place an x in each [ ] that applies)