-
Notifications
You must be signed in to change notification settings - Fork 1
Flesh out Slack frontend, with help from Cursor (Sorcery) #5
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?
Changes from all commits
e87c5b8
894c9ac
d25efa3
6a55ae7
30cd640
5fb7212
b90dbba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,8 @@ | ||||||||||||||||||||||||||||||
| """Slack frontend for Dreambot.""" | ||||||||||||||||||||||||||||||
| import asyncio | ||||||||||||||||||||||||||||||
| import base64 | ||||||||||||||||||||||||||||||
| import io | ||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||
| import traceback | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||||||||||
|
|
@@ -49,9 +52,23 @@ async def on_message(body: dict[str, Any]): # type: ignore | |||||||||||||||||||||||||||||
| await self.on_message(body) | ||||||||||||||||||||||||||||||
| except Exception as exc: | ||||||||||||||||||||||||||||||
| self.logger.error("Error handling Slack message: %s", exc) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| self.is_booted = True # FIXME: This really should happen in response to some kind of "connected" event, but Slack doesn't seem to have one | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Add an event handler for when the app is ready | ||||||||||||||||||||||||||||||
| @self.slack.event("app_mention") # type: ignore | ||||||||||||||||||||||||||||||
| async def on_app_mention(body: dict[str, Any]): # type: ignore | ||||||||||||||||||||||||||||||
| # This is just a dummy handler to ensure the app is ready | ||||||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Start the handler | ||||||||||||||||||||||||||||||
| await self.handler.start_async() | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Verify the connection by making a test API call | ||||||||||||||||||||||||||||||
| auth_test = await self.slack.client.auth_test() # type: ignore | ||||||||||||||||||||||||||||||
| self.logger.info("Connected to Slack workspace: %s", auth_test["team"]) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Set the booted flag | ||||||||||||||||||||||||||||||
| self.is_booted = True | ||||||||||||||||||||||||||||||
| self.logger.info("Slack connection established") | ||||||||||||||||||||||||||||||
| except SlackApiError as exc: | ||||||||||||||||||||||||||||||
| self.logger.error("Slack API error: %s", exc) | ||||||||||||||||||||||||||||||
| except Exception as exc: | ||||||||||||||||||||||||||||||
|
|
@@ -78,7 +95,112 @@ async def callback_receive_workload(self, queue_name: str, message: dict[str, An | |||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||
| bool: True if the message should be ack'd in NATS, False otherwise. | ||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||
| # FIXME: Implement | ||||||||||||||||||||||||||||||
| self.logger.info("Received message for queue %s", queue_name) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Get the channel to send the message to | ||||||||||||||||||||||||||||||
| channel_id = message["channel"] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Prepare the message content | ||||||||||||||||||||||||||||||
| reply_content = "" | ||||||||||||||||||||||||||||||
| reply_blocks = [] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if "reply-image" in message: | ||||||||||||||||||||||||||||||
| # For image replies, we need to upload the image to Slack | ||||||||||||||||||||||||||||||
| image_bytes = base64.standard_b64decode(message["reply-image"]) | ||||||||||||||||||||||||||||||
| filename = self.clean_filename(message["prompt"], suffix=".png", output_dir=self.options["output_dir"]) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Save the image to disk | ||||||||||||||||||||||||||||||
| with open(os.path.join(self.options["output_dir"], filename), "wb") as image_file: | ||||||||||||||||||||||||||||||
| image_file.write(image_bytes) | ||||||||||||||||||||||||||||||
|
Comment on lines
+110
to
+114
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Avoid repeating file path assembly and consider cleanup. The file path (os.path.join(self.options["output_dir"], filename)) is computed multiple times. It may improve clarity and reduce potential errors to compute it once and reuse it. Additionally, if the file isn’t needed after the upload, consider removing it to prevent clutter in the output directory.
Suggested change
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Upload the image to Slack | ||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| with open(os.path.join(self.options["output_dir"], filename), "rb") as image_file: | ||||||||||||||||||||||||||||||
| upload_result = await self.slack.client.files_upload_v2( | ||||||||||||||||||||||||||||||
| file=image_file, | ||||||||||||||||||||||||||||||
| filename=filename, | ||||||||||||||||||||||||||||||
| title="Dream result", | ||||||||||||||||||||||||||||||
| initial_comment="I dreamed this:" | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Get the file URL from the upload result | ||||||||||||||||||||||||||||||
| file_url = upload_result["file"]["url_private"] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Create a message with the image | ||||||||||||||||||||||||||||||
| reply_blocks = [ | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| "type": "section", | ||||||||||||||||||||||||||||||
| "text": { | ||||||||||||||||||||||||||||||
| "type": "mrkdwn", | ||||||||||||||||||||||||||||||
| "text": "I dreamed this:" | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| "type": "image", | ||||||||||||||||||||||||||||||
| "image_url": file_url, | ||||||||||||||||||||||||||||||
| "alt_text": "Dream result" | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| self.logger.info("OUTPUT: %s %s", self.log_slug(message), "Image uploaded") | ||||||||||||||||||||||||||||||
| except Exception as exc: | ||||||||||||||||||||||||||||||
| self.logger.error("Failed to upload image: %s", exc) | ||||||||||||||||||||||||||||||
| reply_content = "Failed to upload dream image." | ||||||||||||||||||||||||||||||
| self.logger.error("OUTPUT: %s %s", self.log_slug(message), reply_content) | ||||||||||||||||||||||||||||||
| elif "reply-text" in message: | ||||||||||||||||||||||||||||||
| reply_content = message["reply-text"] | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (code-quality): Use previously assigned local variable ( |
||||||||||||||||||||||||||||||
| self.logger.info("OUTPUT: %s %s", self.log_slug(message), message["reply-text"]) | ||||||||||||||||||||||||||||||
| elif "reply-none" in message: | ||||||||||||||||||||||||||||||
| self.logger.info("SILENCE FOR %s %s", self.log_slug(message), message["reply-none"]) | ||||||||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||||||||
| elif "error" in message: | ||||||||||||||||||||||||||||||
| reply_content = f"Dream sequence collapsed: {message['error']}" | ||||||||||||||||||||||||||||||
| self.logger.error("OUTPUT: %s %s", self.log_slug(message), reply_content) | ||||||||||||||||||||||||||||||
| elif "usage" in message: | ||||||||||||||||||||||||||||||
| reply_content = f"{message['usage']}" | ||||||||||||||||||||||||||||||
| self.logger.info("OUTPUT: %s %s", self.log_slug(message), message["usage"]) | ||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||
| reply_content = "Dream sequence collapsed, unknown reason." | ||||||||||||||||||||||||||||||
| self.logger.error("Unknown workload message: %s", message) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Send the message to the channel | ||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| if reply_blocks: | ||||||||||||||||||||||||||||||
| await self.slack.client.chat_postMessage( | ||||||||||||||||||||||||||||||
| channel=channel_id, | ||||||||||||||||||||||||||||||
| blocks=reply_blocks | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||
| await self.slack.client.chat_postMessage( | ||||||||||||||||||||||||||||||
| channel=channel_id, | ||||||||||||||||||||||||||||||
| text=reply_content | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Add a reaction to the original message if we have the message ID | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Consider abstracting reaction additions into a helper function. The code for adding reactions appears in both the successful and exception branches. Extracting this logic into a dedicated helper could reduce duplication, making the error-handling paths more concise and easier to maintain. Suggested implementation: class YourSlackClient:
# existing methods & properties
async def _add_reaction(self, message, reaction):
if "origin_message" not in message:
return
try:
await self.slack.client.reactions_add(
channel=message["origin_message"]["channel"],
timestamp=message["origin_message"]["ts"],
name=reaction
)
except Exception as e:
# Optionally log the error if needed, e.g. self.logger.error(f"Failed to add reaction: {e}")
pass
# ... other methods continue # Add a reaction to the original message
await self._add_reaction(message, "white_check_mark")Make sure to update any other branches of code that repeat similar reaction addition logic to use the _add_reaction helper method. Adjust method/class names (like YourSlackClient) as needed to fit your actual codebase. |
||||||||||||||||||||||||||||||
| if "origin_message" in message: | ||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| await self.slack.client.reactions_add( | ||||||||||||||||||||||||||||||
| channel=channel_id, | ||||||||||||||||||||||||||||||
| timestamp=message["origin_message"], | ||||||||||||||||||||||||||||||
| name="thumbsup" | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
| except Exception as exc: | ||||||||||||||||||||||||||||||
| self.logger.error("Failed to add reaction: %s", exc) | ||||||||||||||||||||||||||||||
| except Exception as exc: | ||||||||||||||||||||||||||||||
| self.logger.error("Failed to send reply: %s", exc) | ||||||||||||||||||||||||||||||
| traceback.print_exc() | ||||||||||||||||||||||||||||||
|
Comment on lines
+190
to
+191
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Prefer logging exceptions over printing tracebacks. Using traceback.print_exc() outside of your logging framework can lead to inconsistent error reporting. It might be clearer to integrate full exception logging within the logger to maintain unified error output and control over log levels.
Suggested change
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Try to add a thumbs down reaction to the original message | ||||||||||||||||||||||||||||||
| if "origin_message" in message: | ||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| await self.slack.client.reactions_add( | ||||||||||||||||||||||||||||||
| channel=channel_id, | ||||||||||||||||||||||||||||||
| timestamp=message["origin_message"], | ||||||||||||||||||||||||||||||
| name="thumbsdown" | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
| except Exception as exc: | ||||||||||||||||||||||||||||||
| self.logger.error("Failed to add reaction: %s", exc) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| async def on_message(self, msg: dict[str, Any]): | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (code-quality): We've found these issues:
ExplanationThe quality score for this function is below the quality threshold of 25%. How can you solve this? It might be worth refactoring this function to make it shorter and more readable.
|
||||||||||||||||||||||||||||||
|
|
@@ -109,27 +231,65 @@ async def on_message(self, msg: dict[str, Any]): | |||||||||||||||||||||||||||||
| "frontend": "slack", | ||||||||||||||||||||||||||||||
| "channel": msg["event"]["channel"], | ||||||||||||||||||||||||||||||
| "user": msg["event"]["user"], | ||||||||||||||||||||||||||||||
| "origin_message": msg["event"]["ts"], # Slack uses timestamps as message IDs | ||||||||||||||||||||||||||||||
| "trigger": trigger, | ||||||||||||||||||||||||||||||
| "prompt": prompt, | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Get user information | ||||||||||||||||||||||||||||||
| if reply["user"] not in self.user_name_cache: | ||||||||||||||||||||||||||||||
| user_info = await self.slack.client.users_info(user=reply["user"]) # type: ignore | ||||||||||||||||||||||||||||||
| self.logger.error("********* FOUND USER INFO: %s", user_info) | ||||||||||||||||||||||||||||||
| self.user_name_cache[reply["user"]] = user_info["user"]["real_name"] | ||||||||||||||||||||||||||||||
| reply["user_name"] = self.user_name_cache[reply["user"]] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # FIXME: Get channel_name and server_name here | ||||||||||||||||||||||||||||||
| # Get channel information | ||||||||||||||||||||||||||||||
| channel_id = reply["channel"] | ||||||||||||||||||||||||||||||
| if channel_id not in self.channel_name_cache: | ||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| channel_info = await self.slack.client.conversations_info(channel=channel_id) # type: ignore | ||||||||||||||||||||||||||||||
| self.channel_name_cache[channel_id] = channel_info["channel"]["name"] | ||||||||||||||||||||||||||||||
| except Exception as exc: | ||||||||||||||||||||||||||||||
| self.logger.error("Failed to get channel info: %s", exc) | ||||||||||||||||||||||||||||||
| self.channel_name_cache[channel_id] = "DM" | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| reply["channel_name"] = self.channel_name_cache[channel_id] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Get workspace (server) information | ||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| workspace_info = await self.slack.client.team_info() # type: ignore | ||||||||||||||||||||||||||||||
| reply["server_name"] = workspace_info["team"]["name"] | ||||||||||||||||||||||||||||||
| reply["server_id"] = workspace_info["team"]["id"] | ||||||||||||||||||||||||||||||
| except Exception as exc: | ||||||||||||||||||||||||||||||
| self.logger.error("Failed to get workspace info: %s", exc) | ||||||||||||||||||||||||||||||
| reply["server_name"] = "Slack" | ||||||||||||||||||||||||||||||
| reply["server_id"] = "unknown" | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Check if the message has an image | ||||||||||||||||||||||||||||||
| if "files" in msg["event"] and msg["event"]["files"]: | ||||||||||||||||||||||||||||||
| for file in msg["event"]["files"]: | ||||||||||||||||||||||||||||||
| if file["filetype"] in ["png", "jpg", "jpeg", "gif"]: | ||||||||||||||||||||||||||||||
| reply["image_url"] = file["url_private"] | ||||||||||||||||||||||||||||||
| break | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| self.logger.info("INPUT: %s %s", self.log_slug(reply), text) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Publish the trigger | ||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| await self.callback_send_workload(reply) | ||||||||||||||||||||||||||||||
| # FIXME: Add thumbs-up reaction | ||||||||||||||||||||||||||||||
| # Add a thumbs-up reaction to the message | ||||||||||||||||||||||||||||||
| await self.slack.client.reactions_add( | ||||||||||||||||||||||||||||||
| channel=channel_id, | ||||||||||||||||||||||||||||||
| timestamp=msg["event"]["ts"], | ||||||||||||||||||||||||||||||
| name="thumbsup" | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
| except Exception: | ||||||||||||||||||||||||||||||
| traceback.print_exc() | ||||||||||||||||||||||||||||||
| # FIXME: Add thumbs-down reaction | ||||||||||||||||||||||||||||||
| # Add a thumbs-down reaction to the message | ||||||||||||||||||||||||||||||
| await self.slack.client.reactions_add( | ||||||||||||||||||||||||||||||
| channel=channel_id, | ||||||||||||||||||||||||||||||
| timestamp=msg["event"]["ts"], | ||||||||||||||||||||||||||||||
| name="thumbsdown" | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def log_slug(self, resp: dict[str, str]) -> str: | ||||||||||||||||||||||||||||||
| """Return a string to identify a message in logs.""" | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
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.
issue (complexity): Consider extracting the image uploading and reaction handling logic into helper functions to reduce nesting and improve testability.
Consider extracting the logic for image uploading (and similarly for reaction handling) into helper functions. This not only reduces deep nesting in
callback_receive_workloadbut also makes each unit of work easier to test and maintain.For example, extract the image-upload flow into its own helper:
Then update your
callback_receive_workloadas follows:Similarly, you can encapsulate the reaction addition into a helper if similar patterns emerge. This refactoring will simplify the main callback logic without altering functionality.