Add Image Extraction Support to NestBot#3925
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary by CodeRabbit
WalkthroughAdds image-based context to the Slack AI assistant: downloads and encodes images from app_mention events, obtains concise image descriptions via a vision-capable OpenAi client, appends that context to user queries before routing, adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
rudransh-shrivastava
changed the title
| - channels:read | ||
| - chat:write | ||
| - commands | ||
| - files:read |
There was a problem hiding this comment.
This requires app re-installation.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 13 files
Confidence score: 3/5
- Potential runtime error in
backend/apps/slack/events/app_mention.py: missingsizecan yieldNone <= MAX_IMAGE_SIZEand raiseTypeError, which could break image handling in Slack mentions. - Score reflects a concrete, user-impacting bug risk despite overall limited scope of changes.
- Pay close attention to
backend/apps/slack/events/app_mention.pyandbackend/apps/common/open_ai.py- fix missing size default and align docstring return type.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="backend/apps/common/open_ai.py">
<violation number="1" location="backend/apps/common/open_ai.py:103">
P3: Docstring return type `list[dict]` doesn't match the actual return type annotation `list[ChatCompletionContentPartParam]`. Update the docstring to match.</violation>
</file>
<file name="backend/apps/slack/events/app_mention.py">
<violation number="1" location="backend/apps/slack/events/app_mention.py:38">
P1: `file.get("size")` returns `None` when the key is absent, and `None <= MAX_IMAGE_SIZE` raises `TypeError` in Python 3. Provide a default value to handle missing size gracefully.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/apps/common/open_ai.py`:
- Around line 10-13: Remove the duplicate conditional import: keep the runtime
import "from openai.types.chat import ChatCompletionContentPartParam" (since
ChatCompletionContentPartParam is used at runtime for the annotation on the
function around line 106) and delete the TYPE_CHECKING guarded re-import block;
ensure the TYPE_CHECKING symbol and its conditional import are removed so
there's only one import of ChatCompletionContentPartParam.
In `@backend/apps/slack/events/app_mention.py`:
- Around line 35-39: The list comprehension that builds images_raw can raise
TypeError when a file dict lacks "size"; change the predicate in images_raw to
guard/default the size (e.g., use file.get("size", 0) or check "size" in file
before comparing) and ensure you still filter by mimetype using
ALLOWED_MIMETYPES and the MAX_IMAGE_SIZE constant; update the comprehension that
references files, ALLOWED_MIMETYPES, and MAX_IMAGE_SIZE so missing size keys do
not cause None <= MAX_IMAGE_SIZE comparisons.
🧹 Nitpick comments (3)
backend/apps/slack/events/app_mention.py (1)
97-104: Consider limiting the number of images processed.There's no cap on how many images are downloaded and base64-encoded. A user attaching many images (e.g., 20 small PNGs) could cause significant latency and large payloads sent to OpenAI. Consider adding a reasonable limit (e.g., 5 images).
♻️ Suggested change
+MAX_IMAGES = 5 + image_uris = [] - for image in images_raw: + for image in images_raw[:MAX_IMAGES]: content = download_file(image.get("url_private"), client.token)backend/apps/ai/flows/assistant.py (1)
80-89: Image context augmentation looks solid.The flow correctly guards against empty/None images, handles
complete()returningNone, and appends context only when available. One consideration: this makes an additional synchronous OpenAI API call (togpt-4o) on the request path before routing even begins. For messages with images, this adds latency and cost.Consider setting a lower
max_tokensfor the description (e.g., 300) since the prompt asks for conciseness, to reduce latency and cost:image_context = ( - OpenAi(model="gpt-4o") + OpenAi(model="gpt-4o", max_tokens=300) .set_prompt(IMAGE_DESCRIPTION_PROMPT)backend/tests/apps/slack/events/app_mention_test.py (1)
89-117: LGTM — oversized image filtering test.Good boundary test with a 3MB file exceeding the 2MB limit. Consider adding an edge-case test at exactly the boundary (2MB) to verify whether the limit is inclusive or exclusive.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="backend/apps/slack/events/app_mention.py">
<violation number="1" location="backend/apps/slack/events/app_mention.py:38">
P2: Defaulting to `0` when `size` is missing means files with unknown sizes will *pass* the size check (since `0 <= MAX_IMAGE_SIZE`). This defeats the purpose of the size guard. If the size is unknown, it's safer to exclude the file by defaulting to a value that exceeds the limit.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/apps/slack/events/app_mention.py`:
- Line 100: images_raw may contain file entries missing the "url_private" key,
so calling download_file(image.get("url_private"), client.token) can pass None
and cause a downstream requests error; update the filter that builds images_raw
(or add a short guard before the call) to require image.get("url_private") is
truthy, and only call download_file when image.get("url_private") returns a
non-None string; reference the images_raw construction and the
download_file(...) invocation to locate and change the predicate or add the
guard.
🧹 Nitpick comments (1)
backend/apps/slack/events/app_mention.py (1)
98-105: Consider capping the number of images to bound memory and API cost.There's no limit on how many images are downloaded, base64-encoded (each up to ~2.67 MB after encoding), and sent to the vision model. A user could attach many files to a single message. Consider adding a cap, e.g., process at most 3–5 images.
♻️ Suggested change
+MAX_IMAGES = 5 + ALLOWED_MIMETYPES = ("image/jpeg", "image/png", "image/webp") MAX_IMAGE_SIZE = 2 * 1024 * 1024 # 2 MBimage_uris = [] - for image in images_raw: + for image in images_raw[:MAX_IMAGES]: content = download_file(image.get("url_private"), client.token)
rudransh-shrivastava
requested review from
arkid15r and
kasya
as code owners
|
arkid15r
merged commit 74b4467
into
OWASP:feature/nestbot-ai-assistant
2 participants
Proposed change
Resolves #2188
Add image extraction support to NestBot
Problem: This seems out of scope for this PR. I am just adding additional context using images.
Checklist
make check-testlocally: all warnings addressed, tests passed