feat: add Florence captioning to OmniParser detect‑elements and input validation - #23
feat: add Florence captioning to OmniParser detect‑elements and input validation#23Santosh69 wants to merge 10 commits into
Conversation
- Update code to use model.pt (matches OmniParser release) - Add .gitignore patterns for model weight files - Remove *.rst from .gitignore - Add Prerequisites section to README with download instructions - Fix verify_model_load.py to use model.pt Sub-issue of MVP demo preparation. Fixes FileNotFoundError for contributors.
- generate captions per detected element - added validate_image size/type before inference
There was a problem hiding this comment.
Pull request overview
This pull request adds Florence-2 captioning to the OmniParser element detection pipeline, providing dynamic content descriptions for each detected UI element. The changes enhance the existing YOLO-based detection by generating semantic captions for each element crop, and add infrastructure improvements including a cross-platform development launcher and automated model weight setup.
Changes:
- Integrated Florence-2 model for per-element caption generation in the detect-elements workflow
- Added cross-platform development environment launcher (
start-dev.py) and automated weight setup script - Pinned critical dependency versions (torch, transformers, numpy) and added new dependencies (einops, huggingface-hub, accelerate)
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
app/services/omniparser_client.py |
Core changes: loads Florence-2 model, generates captions for each YOLO detection, adds image validation method |
app/utils/transformers_patches.py |
New utility to patch transformers to skip flash_attn requirement on non-CUDA systems |
start-dev.py |
New cross-platform script to launch backend and Firebase emulators with automated setup |
scripts/setup_omniparser.py |
New script to download and configure OmniParser v2 weights from HuggingFace |
requirements.txt |
Pinned torch (2.1.2), transformers (4.40.0), numpy (1.26.4); added einops, huggingface-hub, accelerate |
README.md |
Extensive documentation updates for model weights, setup procedures, and cross-platform development |
.gitignore |
Removed blanket ignore for .md/.rst files; added weights/ directory and Firebase config files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| try: | ||
| from huggingface_hub import snapshot_download | ||
| except Exception: |
There was a problem hiding this comment.
The exception handler on line 6 catches all exceptions without specifying the exception type, which is considered a broad anti-pattern. It would be better to catch specific exceptions like ImportError since that's what would occur if huggingface_hub is not installed. The current catch-all could hide other unexpected errors during import.
| except Exception: | |
| except ImportError: |
| crop_x2 = min(width, int(x2)) | ||
| crop_y2 = min(height, int(y2)) | ||
|
|
||
| if crop_x2 <= crop_x1 or crop_y2 <= crop_y1: |
There was a problem hiding this comment.
When crop dimensions are invalid (crop_x2 <= crop_x1 or crop_y2 <= crop_y1), the code continues to the next element without adding it to the results (line 294). However, this happens silently - there's no log message indicating that an element was skipped due to invalid dimensions. This makes it difficult to debug cases where expected elements are missing from the output. Consider adding a debug or warning log message when elements are skipped.
| if crop_x2 <= crop_x1 or crop_y2 <= crop_y1: | |
| if crop_x2 <= crop_x1 or crop_y2 <= crop_y1: | |
| self.logger.warning( | |
| "Skipping element due to invalid crop dimensions: " | |
| f"raw_box=({x1}, {y1}, {x2}, {y2}), " | |
| f"crop_box=({crop_x1}, {crop_y1}, {crop_x2}, {crop_y2}), " | |
| f"raw_type={raw_type}" | |
| ) |
| .strip() | ||
| ) | ||
| if element_content: | ||
| self.logger.info(f"Caption: {element_content[:50]}") |
There was a problem hiding this comment.
The caption is truncated to 50 characters in the log message (line 338), but this truncation is only for logging purposes and doesn't affect the actual stored content. This is good practice to avoid cluttering logs with long captions. However, consider also logging the full length of the caption to help diagnose cases where captions might be unexpectedly long or short.
| self.logger.info(f"Caption: {element_content[:50]}") | |
| self.logger.info(f"Caption (len={len(element_content)}): {element_content[:50]}") |
| ).to(device) | ||
|
|
||
| self.logger.info("✅ Florence model loaded") | ||
| self.processor = AutoProcessor.from_pretrained("microsoft/Florence-2-base-ft", trust_remote_code=True) |
There was a problem hiding this comment.
The processor is loaded from "microsoft/Florence-2-base-ft" which is a remote HuggingFace repository, but the model is loaded from the local "weights/icon_caption_florence" directory. This creates an inconsistency and potential version mismatch issue. The processor should also be loaded from the local weights directory to ensure model-processor compatibility and avoid network calls during runtime. Change this to load from "weights/icon_caption_florence" like the model.
| self.processor = AutoProcessor.from_pretrained("microsoft/Florence-2-base-ft", trust_remote_code=True) | |
| self.processor = AutoProcessor.from_pretrained("weights/icon_caption_florence", trust_remote_code=True) |
| @@ -1,3 +1,6 @@ | |||
| from app.utils.transformers_patches import patch_transformers_flash_attn_check | |||
There was a problem hiding this comment.
The patch is applied at module import time (lines 1-2 of omniparser_client.py), which means it's executed as soon as the module is imported, even if OmniParserClient is never instantiated. This is acceptable for a singleton service, but the patch modifies global transformers behavior that could affect other parts of the application if they use transformers differently. Consider documenting this side effect more prominently, or moving the patch call inside the OmniParserClient.initialize() method to ensure it only runs when actually needed.
| from app.utils.transformers_patches import patch_transformers_flash_attn_check | |
| from app.utils.transformers_patches import patch_transformers_flash_attn_check | |
| # NOTE: Importing this module applies a global patch to the transformers library. | |
| # This call modifies transformers' flash attention behavior process-wide and | |
| # will affect any other code using transformers in the same Python process. | |
| # This is intentional for OmniParser usage; do not remove or move without | |
| # carefully considering the impact on global transformers behavior. |
| try: | ||
| # Crop the detected element | ||
| crop_x1 = max(0, int(x1)) | ||
| crop_y1 = max(0, int(y1)) | ||
| crop_x2 = min(width, int(x2)) | ||
| crop_y2 = min(height, int(y2)) | ||
|
|
||
| if crop_x2 <= crop_x1 or crop_y2 <= crop_y1: | ||
| continue | ||
|
|
||
| element_crop = image.crop((crop_x1, crop_y1, crop_x2, crop_y2)) | ||
| self.logger.info(f"Crop format: {element_crop.mode}, size: {element_crop.size}") | ||
|
|
||
| if element_crop.mode != "RGB": | ||
| element_crop = element_crop.convert("RGB") | ||
|
|
||
| # Skip very small elements | ||
| if element_crop.width >= 10 and element_crop.height >= 10: | ||
| # Use Florence to generate caption | ||
| prompt = "<CAPTION>" | ||
| inputs = self.processor( | ||
| text=prompt, | ||
| images=element_crop, | ||
| return_tensors="pt" | ||
| ).to(self.device) | ||
|
|
||
| # Generate caption | ||
| with torch.no_grad(): | ||
| generated_ids = self.caption_model.generate( | ||
| input_ids=inputs["input_ids"], | ||
| pixel_values=inputs["pixel_values"], | ||
| max_new_tokens=50, | ||
| num_beams=3 | ||
| ) | ||
|
|
||
| # Decode caption | ||
| generated_text = self.processor.batch_decode( | ||
| generated_ids, | ||
| skip_special_tokens=False | ||
| )[0] | ||
| self.logger.info(f"Florence raw output: {repr(generated_text)}") | ||
| # Extract caption (remove tags) | ||
| element_content = ( | ||
| generated_text | ||
| .replace("<s>", "") | ||
| .replace("</s>", "") | ||
| .replace("<CAPTION>", "") | ||
| .replace("</CAPTION>", "") | ||
| .replace("<pad>", "") | ||
| .strip() | ||
| ) | ||
| if element_content: | ||
| self.logger.info(f"Caption: {element_content[:50]}") | ||
|
|
||
| except Exception as e: | ||
| self.logger.warning(f"Failed to caption {mapped_type}: {e}") | ||
| element_content = "" |
There was a problem hiding this comment.
The Florence captioning runs synchronously for each detected element (lines 286-342), which could significantly slow down detection for images with many elements. Each caption generation involves model inference which can take hundreds of milliseconds. For an image with 20 elements, this could add 5-10 seconds to processing time. Consider batching the caption generation calls or making them asynchronous to improve throughput, especially for production use cases.
| self.logger.info(f"Caption: {element_content[:50]}") | ||
|
|
||
| except Exception as e: | ||
| self.logger.warning(f"Failed to caption {mapped_type}: {e}") |
There was a problem hiding this comment.
When Florence captioning fails (line 341), the error is logged as a warning and element_content is set to empty string, but the element is still added to the results. This is good for resilience, but the warning message could be more informative. Consider logging the element's bounding box coordinates or position to help debug which specific elements are failing to caption, especially useful when investigating model issues.
| self.logger.warning(f"Failed to caption {mapped_type}: {e}") | |
| self.logger.warning( | |
| f"Failed to caption {mapped_type} at bbox " | |
| f"({x1:.1f}, {y1:.1f}, {x2:.1f}, {y2:.1f}): {e}" | |
| ) |
| trust_remote_code=True | ||
| ) | ||
| trust_remote_code=True, | ||
| torch_dtype=torch.float16 if device == "cuda" else torch.float32, |
There was a problem hiding this comment.
The device selection logic (lines 222-226) sets torch.float16 for CUDA but torch.float32 for CPU/MPS (line 234). However, for MPS (Apple Silicon), torch.float16 is typically supported and recommended for better performance. Consider checking if MPS supports float16 and using it when available to improve performance on Apple Silicon devices.
| torch_dtype=torch.float16 if device == "cuda" else torch.float32, | |
| torch_dtype=torch.float16 if device in ("cuda", "mps") else torch.float32, |
| from ultralytics import YOLO | ||
| import torch | ||
| from transformers import AutoProcessor, AutoModelForCausalLM | ||
| import re # For cleaning Florence output |
There was a problem hiding this comment.
The re module is imported but never used in the code. The comment suggests it's for cleaning Florence output, but the actual cleaning is done using string replace() methods (lines 330-335). This unused import should be removed.
| @@ -0,0 +1,54 @@ | |||
| import os | |||
| import sys | |||
| import shutil | |||
There was a problem hiding this comment.
Import of 'shutil' is not used.
| import shutil |
Summary
Changes
app/services/omniparser_client.pyto run Florence captioning on each YOLO crop.validate_imagechecks for content type and max size.Commit
feat(omniparser): use Florence for detect-elements content)**Note: ** this Branch has followed commits/dependency of Other PRs to Work
Testing