Fix image processing pipeline and ChatRequest validation #351
Conversation
- Updated `process_uploaded_image` in `backend/utils.py` to return `(Image.Image, io.BytesIO)` to support optimization and avoid double-decoding. - Updated `backend/routers/issues.py` to correctly unpack the tuple returned by `process_uploaded_image`. - Fixed `_prepare_image_bytes` in `backend/hf_api_service.py` to handle `io.BytesIO` objects, resolving crashes in detection endpoints. - Added `validate_query` to `ChatRequest` schema in `backend/schemas.py` to reject empty or whitespace-only queries. - Verified changes with existing tests `test_schemas.py` and `test_new_features.py`. Co-authored-by: RohanExploit <178623867+RohanExploit@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
✅ Deploy Preview for fixmybharat canceled.
|
🙏 Thank you for your contribution, @RohanExploit!PR Details:
Quality Checklist:
Review Process:
Note: The maintainers will monitor code quality and ensure the overall project flow isn't broken. |
📝 WalkthroughWalkthroughThe image processing pipeline is refactored to return both the processed PIL Image object and BytesIO representation from utility functions, with corresponding updates to consumers and input handlers to accommodate the new tuple return type. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes mismatches in the image upload/processing pipeline and tightens request validation for chat inputs, stabilizing detection endpoints and issue creation flows.
Changes:
- Updated
process_uploaded_imageto return a(PIL.Image, BytesIO)tuple to match router expectations. - Adjusted issue creation to unpack the new image-processing return signature.
- Made HF image-byte preparation robust to
io.BytesIOinputs. - Added a strict validator to reject whitespace-only
ChatRequest.queryvalues.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
backend/utils.py |
Changes process_uploaded_image* to return both a processed PIL image and a BytesIO buffer. |
backend/routers/issues.py |
Updates issue creation flow to unpack the new (image, bytes) return signature. |
backend/hf_api_service.py |
Extends _prepare_image_bytes to accept io.BytesIO and convert it to raw bytes safely. |
backend/schemas.py |
Adds ChatRequest.query validator to reject whitespace-only queries and normalize input via .strip(). |
Comments suppressed due to low confidence (1)
backend/hf_api_service.py:55
_prepare_image_bytesnow supportsio.BytesIO, but the rest of the module still annotates most public detector helpers as accepting onlyUnion[Image.Image, bytes]. Since router code passes the processed image buffer around, updating those function type hints (and any relevant docstrings) to includeio.BytesIOwould keep the API contract consistent and avoid confusion for callers and static analysis.
def _prepare_image_bytes(image: Union[Image.Image, bytes, io.BytesIO]) -> bytes:
if isinstance(image, bytes):
return image
if isinstance(image, io.BytesIO):
return image.getvalue()
img_byte_arr = io.BytesIO()
fmt = image.format if image.format else 'JPEG'
image.save(img_byte_arr, format=fmt)
return img_byte_arr.getvalue()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -179,15 +179,15 @@ | |||
| # Strip EXIF | |||
| img_no_exif = Image.new(img.mode, img.size) | |||
| img_no_exif.paste(img) | |||
|
|
|||
| # Save to BytesIO | |||
| output = io.BytesIO() | |||
| # Preserve format or default to JPEG | |||
| fmt = img.format or 'JPEG' | |||
| img_no_exif.save(output, format=fmt, quality=85) | |||
| output.seek(0) | |||
|
|
|||
| return output | |||
| return img_no_exif, output | |||
|
|
|||
| except Exception as pil_error: | |||
| logger.error(f"PIL processing failed: {pil_error}") | |||
| @@ -202,7 +202,7 @@ | |||
| logger.error(f"Error processing file: {e}") | |||
| raise HTTPException(status_code=400, detail="Unable to process file.") | |||
|
|
|||
| async def process_uploaded_image(file: UploadFile) -> io.BytesIO: | |||
| async def process_uploaded_image(file: UploadFile) -> tuple[Image.Image, io.BytesIO]: | |||
| return await run_in_threadpool(process_uploaded_image_sync, file) | |||
There was a problem hiding this comment.
process_uploaded_image/process_uploaded_image_sync now return a tuple, but there are no unit tests asserting the new return contract (PIL image + BytesIO) or that the BytesIO is rewound and usable for downstream consumers. Adding a focused test here would prevent regressions like unpacking errors or non-seeked buffers.
| _, processed_image = await process_uploaded_image(image) | ||
|
|
||
| # Save processed image to disk | ||
| await run_in_threadpool(save_processed_image, processed_image, image_path) |
There was a problem hiding this comment.
The local name processed_image is now an io.BytesIO (image bytes), not a PIL image. Renaming it to something like processed_image_io / processed_image_bytes would make the code clearer and reduce the chance of accidentally treating it as an Image.Image later.
| _, processed_image = await process_uploaded_image(image) | |
| # Save processed image to disk | |
| await run_in_threadpool(save_processed_image, processed_image, image_path) | |
| _, processed_image_bytes = await process_uploaded_image(image) | |
| # Save processed image to disk | |
| await run_in_threadpool(save_processed_image, processed_image_bytes, image_path) |
| # Strip EXIF | ||
| img_no_exif = Image.new(img.mode, img.size) | ||
| img_no_exif.paste(img) | ||
|
|
||
| # Save to BytesIO | ||
| output = io.BytesIO() | ||
| # Preserve format or default to JPEG | ||
| fmt = img.format or 'JPEG' | ||
| img_no_exif.save(output, format=fmt, quality=85) | ||
| output.seek(0) | ||
|
|
||
| return output | ||
| return img_no_exif, output |
There was a problem hiding this comment.
The returned img_no_exif is created via Image.new(...), so it will typically have image.format == None even when the original upload was PNG/WebP/etc. Since the function now exposes the PIL image to callers, consider preserving the original format in the returned image (or documenting that callers must not rely on .format and should use the returned BytesIO for encoding).
1 participant
This PR addresses critical bugs in the image processing pipeline and schema validation.
Key Changes:
process_uploaded_imagewas refactored to return both the PIL Image and the BytesIO object. This aligns with the "memory" of optimization (avoiding re-reading files) and fixes theValueError: too many values to unpackerrors inbackend/routers/detection.pywhich was already expecting a tuple.backend/routers/issues.pywas updated to handle the new return signature ofprocess_uploaded_image, preventing a regression in the issue creation flow.backend/hf_api_service.py's_prepare_image_byteshelper was patched to explicitly handleio.BytesIOobjects, preventingAttributeError: '_io.BytesIO' object has no attribute 'format'errors during AI detection calls.ChatRequest.queryto forbid whitespace-only strings, ensuring better data integrity and matching test expectations.These changes restore the stability of detection features and the issue reporting flow.
PR created automatically by Jules for task 12866211084660275983 started by @RohanExploit
Summary by CodeRabbit