feat(nestbot): Add image extraction support for NestBot AI assistant#3563
feat(nestbot): Add image extraction support for NestBot AI assistant#3563Ani07-05 wants to merge 5 commits intoOWASP:feature/nestbot-ai-assistantfrom
Conversation
github-actions
bot
added
frontend
docker
|
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:
WalkthroughAdds end-to-end image-to-text extraction for Slack messages: validates and downloads image files, extracts text via OpenAI, stores per-file results on Message.raw_data, and routes messages to either an image-extraction job or the existing AI reply flow; Message gains a text_with_images property used for AI queries. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![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/slack/events/message_posted.py`:
- Around line 38-47: The current try/except around
Message.objects.filter(...).update(...) is incorrect because filter().update()
does not raise Message.DoesNotExist; change the logic in the thread-reply
handling so you call
Message.objects.filter(slack_message_id=event.get("thread_ts"),
conversation__slack_channel_id=event.get("channel")).update(has_replies=True"),
capture the returned count, and if count == 0 call logger.info("Parent message
not found for thread reply.") instead of relying on an exception; remove the
unused except Message.DoesNotExist block and keep the existing return after this
handling.
In `@docker/backend/Dockerfile.local`:
- Around line 1-2: The comment above the base image line contains a stray
closing parenthesis; edit the commented line that currently reads "# python:slim
) is used as base image to keep the image size small" (the Dockerfile header
comment) to remove the extra ")" so it reads without the stray character.
🧹 Nitpick comments (5)
docker-compose/local/compose.yaml (2)
45-51: Potential race condition: nestbot may start before backend migrations complete.The
condition: service_startedonly waits for the backend container to start, not for migrations to finish. Since the backend command runsmigrate && runserver, nestbot could attempt to start while migrations are still in progress.Consider either:
- Adding a healthcheck to backend and using
condition: service_healthy- Adding a startup delay or wait script in nestbot's command
- Accepting this as a local dev limitation (migrations are typically fast)
Option: Add wait for backend readiness
nestbot: container_name: nest-nestbot command: > sh -c ' + sleep 5 && python manage.py runserver 0.0.0.0:8001 '
118-118: Breaking change: docs service now accessible on port 8002.The port change from
8001to8002is necessary to accommodate the new nestbot service, but this may break existing developer workflows. Consider documenting this change in the PR description or updating any relevant documentation.backend/apps/slack/services/image_extraction.py (3)
85-88: Hardcoded MIME type may cause issues with non-JPEG images.The data URL always uses
image/jpegregardless of actual image type. While GPT-4o may handle this gracefully, it's technically incorrect and could cause subtle issues with PNG, GIF, or WebP images.♻️ Proposed fix to infer MIME type
+def _get_image_mime_type(image_data: bytes) -> str: + """Infer MIME type from image data magic bytes.""" + if image_data[:8] == b'\x89PNG\r\n\x1a\n': + return "image/png" + if image_data[:3] == b'GIF': + return "image/gif" + if image_data[:4] == b'RIFF' and image_data[8:12] == b'WEBP': + return "image/webp" + return "image/jpeg" # Default fallback + + def extract_text_from_image(image_data: bytes) -> str: """Extract text from image using GPT-4o.""" ... base64_image = base64.b64encode(image_data).decode("utf-8") + mime_type = _get_image_mime_type(image_data) response = client.chat.completions.create( ... { "type": "image_url", - "image_url": {"url": f"data:image/jpeg;base64,{base64_image}"}, + "image_url": {"url": f"data:{mime_type};base64,{base64_image}"}, },
94-96: Add defensive check for potential None content.
response.choices[0].message.contentcould beNonein edge cases (e.g., content filtered by safety systems), which would cause.strip()to throwAttributeError.♻️ Proposed fix
- return response.choices[0].message.content.strip() + content = response.choices[0].message.content + return content.strip() if content else ""
104-108: Consider usinglogger.warninginstead oflogger.exceptionforDoesNotExist.
logger.exceptionlogs a full stack trace, which is typically reserved for unexpected exceptions.Message.DoesNotExistrepresents expected data absence (e.g., message was deleted). A warning-level log without traceback is more appropriate.♻️ Proposed fix
try: message = Message.objects.get(pk=message_id) except Message.DoesNotExist: - logger.exception("Message %s not found for image extraction", message_id) + logger.warning("Message %s not found for image extraction", message_id) return
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/apps/slack/events/message_posted.py`:
- Around line 63-71: The current bypass uses image_files derived only from
mimetype which allows invalid images to skip the question detector; update the
logic that builds image_files to include only images that also pass your image
validation (e.g. call the existing image validator such as
self.image_validator.validate(file) or your validate_image(file) helper and only
append files that return valid), then only skip the question detector when the
resulting image_files list is non-empty; keep references to image_files,
event.get("files", []), and self.question_detector.is_owasp_question(text) and
preserve the logger.info("Question detector rejected message") behavior when the
detector blocks the message.
- Around line 38-48: The logger.info call inside the thread-reply handling block
drops the thread_ts value because the "%s" placeholder isn't passed; update the
log invocation in the if not updated branch (near
Message.objects.filter(...).update(...) and the surrounding thread_ts check) to
include the thread_ts (e.g., pass event.get("thread_ts") or interpolate it) so
the log prints the missing parent message id.
Ani07-05
changed the title
Ani07-05
changed the title
|
I think I need to get up to speed with NestBot development to suggest any changes. I see there are some changes to decouple the Optionally, it would be great to target the PR to the |
|
@rudransh-shrivastava I'm going to try merging the feature branch this weekend. @Ani07-05 please split this PR but still use the feature branch. Thank you. |
- Handle file_share subtype to process messages with images - Collect up to 3 valid image files and validate before processing - Add image extraction service with Slack API integration using GPT-4o - Add text_with_images property to Message model - Persist extraction results to Message.raw_data - Use extracted text when generating AI replies Resolves #2188
Ani07-05
force-pushed
the
feature/nestbot-ai-assistant
branch
from
8ddc0ca to
4b36b3c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@backend/apps/slack/services/image_extraction.py`:
- Around line 85-96: The code hardcodes "data:image/jpeg" for the data URL and
calls .strip() on response.choices[0].message.content which may be None; update
the image URL construction to detect the actual MIME type from the raw image
bytes (e.g., using imghdr.what or Pillow) and substitute that MIME (image/png,
image/gif, image/webp, etc.) when building the data URL (the variable referenced
is base64_image). Also guard the model response access in the return path
(response.choices[0].message.content) by treating None safely — e.g., default to
an empty string or raise a clear error if content is None — before calling
.strip(), so the function never attempts to call .strip() on None.
- Around line 112-120: Replace the global settings.SLACK_BOT_TOKEN usage with
the workspace-specific token from the message: use
message.conversation.workspace.bot_token when building the Slack client in
image_extraction.py (replace references to settings.SLACK_BOT_TOKEN in the block
that currently sets bot_token). Also guard against
response.choices[0].message.content being None before calling .strip() (check
for None and handle empty content), and change the "not found" logging at the
earlier handler from logger.exception to logger.error to avoid emitting a
traceback for expected missing-resource cases; locate these changes around the
functions handling the OpenAI response and the Slack bot token logic in this
file.
🧹 Nitpick comments (7)
backend/apps/slack/services/image_extraction.py (4)
104-108:logger.exceptionis unnecessarily noisy for a simple "not found" case.
Message.DoesNotExistis a controlled lookup failure, not an unexpected error.logger.exceptionlogs a full traceback, which adds noise. Uselogger.warningorlogger.errorinstead.Proposed fix
except Message.DoesNotExist: - logger.exception("Message %s not found for image extraction", message_id) + logger.warning("Message %s not found for image extraction", message_id) return
19-21:image/jpgis not a valid IANA MIME type.The official MIME type is
image/jpeg. While includingimage/jpgis defensive, Slack's API usesimage/jpeg. Having it here could be misleading or mask a bug if some other source incorrectly reportsimage/jpg.
59-96: OpenAI client is instantiated on every call — consider reuse or at least guarding against large payloads.A new
openai.OpenAIclient is created per image. For up to 3 images per message this is fine, but a module-level or cached client would be slightly cleaner. Also, there's nomax_tokensbudget awareness — the 1000-token cap is reasonable, but the prompt doesn't instruct the model to truncate if the image contains very dense text. This is acceptable for now.
171-174: Potential race condition when updatingraw_dataconcurrently.If two jobs (e.g., retries or duplicate events) run for the same message simultaneously, the read-modify-write on
raw_datacan cause one job's results to overwrite the other's. This is unlikely in practice since RQ serializes within a single worker, but worth noting for robustness.Consider using
F()expressions orselect_for_update()if concurrent access becomes realistic.backend/apps/slack/events/message_posted.py (3)
64-74: Redundant MIME prefix check alongsideis_valid_image_file.
is_valid_image_filealready validates the mimetype againstSUPPORTED_IMAGE_TYPES(which are allimage/*types). Thef.get("mimetype", "").startswith("image/")check on line 73 is redundant — it only filters out non-image files thatis_valid_image_filewould reject anyway.Simplification (optional)
image_files = [ f for f in event.get("files", []) - if f.get("mimetype", "").startswith("image/") and is_valid_image_file(f) + if is_valid_image_file(f) ][:3]
65-68: Late import insidehandle_event— confirm this is for circular import avoidance.This import runs on every message event. If it's to break a circular dependency, add a brief comment. If there's no circular dependency, move it to the top of the file with the other imports.
76-79: Image-only messages bypass the question detector entirely — be aware of cost implications.Any image posted to a NestBot-enabled channel will trigger GPT-4o vision extraction regardless of relevance. If users share unrelated screenshots, this burns OpenAI credits with no value. Consider whether images should also pass through a lightweight relevance check (even just requiring non-empty accompanying text) or at minimum track/alert on extraction volume.
- Handle file_share subtype to process messages with images - Collect up to 3 valid image files and validate before processing - Add image extraction service with Slack API integration using GPT-4o - Add text_with_images property to Message model - Persist extraction results to Message.raw_data - Use extracted text when generating AI replies - Use workspace-specific bot token instead of global settings - Detect actual MIME type for image data URLs using PIL - Handle None content from OpenAI API responses - Change exception logging to error logging for expected cases Resolves #2188
Ani07-05
dismissed stale reviews from coderabbitai[bot] and cubic-dev-ai[bot]
via
a0bbcba
Ani07-05
force-pushed
the
feature/nestbot-ai-assistant
branch
from
a0bbcba to
7169e63
Compare
|
Hi @arkid15r, I've split the PR as requested. This PR now contains only the image extraction feature (resolves #2188). I've stashed the Docker decoupling changes locally. After this PR is merged, I'll create a follow-up PR with the Docker changes (#3565) using the same Thanks! |
rudransh-shrivastava
left a comment
rudransh-shrivastava
left a comment
There was a problem hiding this comment.
Hey @Ani07-05
I'm getting this error:
nest-worker | Failed to detect image format, defaulting to image/jpeg
nest-worker | OpenAI API error for image.png
nest-worker | Traceback (most recent call last):
nest-worker | File "/home/owasp/apps/slack/services/image_extraction.py", line 163, in extract_images_then_maybe_reply
nest-worker | extracted_text = extract_text_from_image(image_data)
nest-worker | File "/home/owasp/apps/slack/services/image_extraction.py", line 80, in extract_text_from_image
nest-worker | response = client.chat.completions.create(
nest-worker | model="gpt-4o",
nest-worker | ...<22 lines>...
nest-worker | temperature=0.0,
nest-worker | )
nest-worker | File "/home/owasp/.venv/lib/python3.13/site-packages/openai/_utils/_utils.py", line 287, in wrapper
nest-worker | return func(*args, **kwargs)
nest-worker | File "/home/owasp/.venv/lib/python3.13/site-packages/openai/resources/chat/completions/completions.py", line 925, in create
nest-worker | return self._post(
nest-worker | ~~~~~~~~~~^
nest-worker | "/chat/completions",
nest-worker | ^^^^^^^^^^^^^^^^^^^^
nest-worker | ...<43 lines>...
nest-worker | stream_cls=Stream[ChatCompletionChunk],
nest-worker | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
nest-worker | )
nest-worker | ^
nest-worker | File "/home/owasp/.venv/lib/python3.13/site-packages/openai/_base_client.py", line 1242, in post
nest-worker | return cast(ResponseT, self.request(cast_to, opts, stream=stream, stream_cls=stream_cls))
nest-worker | ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
nest-worker | File "/home/owasp/.venv/lib/python3.13/site-packages/openai/_base_client.py", line 1037, in request
nest-worker | raise self._make_status_error_from_response(err.response) from None
nest-worker | openai.BadRequestError: Error code: 400 - {'error': {'message': "You uploaded an unsupported image. Please make sure your image has of one the following formats: ['png', 'jpeg', 'gif', 'webp'].", 'type': 'invalid_request_error', 'param': None, 'code': 'invalid_image_format'}}
nest-worker | 19:10:37 Successfully completed apps.slack.services.image_extraction.extract_images_then_maybe_reply(88343, [{'id': 'F0ADH19TZHD', 'created': 1770664224, 'timestamp': 1770664224, 'nam...) job in 0:00:02.121472s on worker 9c792b634c424fb4b14fadc5e518b2db
nest-worker | 19:10:37 ai: Job OK (554d089b-2153-4350-887d-f1dba3a9af18)
nest-worker | 19:10:37 Result is kept for 500 seconds
| return response.content | ||
|
|
||
|
|
||
| def extract_text_from_image(image_data: bytes) -> str: |
There was a problem hiding this comment.
I don't think this logic belongs here. You may use the OpenAi class.
…lack tests - Replace direct vision API call in image extraction with shared OpenAI wrapper - Add image format/mime validation and graceful handling for invalid/unsupported images - Align Slack AI handler, app mention, auto-reply, and question detector tests with current call signatures/flow
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 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/services/image_extraction.py">
<violation number="1" location="backend/apps/slack/services/image_extraction.py:164">
P2: The instruction `"Return only the extracted text without any preamble."` was present in the old prompt but dropped during the refactor. Without it, the model may include conversational preamble (e.g., "Here is the extracted text:") in the extraction output, degrading downstream usage. Consider restoring it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Added instruction to return only extracted text.
|
@rudransh-shrivastava I have addressed the review comment and resolved the error , please do check and let me know |
There was a problem hiding this comment.
1 issue found across 1 file (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/services/image_extraction.py">
<violation number="1" location="backend/apps/slack/services/image_extraction.py:165">
P2: Missing space between concatenated string literals. The previous line ends with `information."` and this line starts with `"Return`, producing `"...key information.Return only..."` in the final prompt. Add a leading space or a trailing space to the previous line to match the pattern used by the other lines.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
Implemented in #3925 |
3 participants
Resolves #2188
Add image-to-text extraction for Slack-attached images:
Checklist
make check-testlocally: all warnings addressed, tests passed