sketchfab preview/model size normalisation by JinTanba · Pull Request #163 · ahujasid/blender-mcp

JinTanba · 2025-12-02T09:53:18Z

User description

Enable the LLM utilising this MCP to select 3D models for visualisation and download.
By having the LLM explicitly specify the size of the 3D model to download, enable efficient size normalisation between objects.

PR Type

Enhancement

Description

Add model preview functionality with thumbnail retrieval from Sketchfab API
Implement automatic size normalization for imported 3D models with explicit target_size parameter
Calculate and return detailed bounding box and dimension information for imported models
Make target_size a required parameter to force LLM consideration of model scale

Diagram Walkthrough

flowchart LR
  A["Sketchfab API"] -->|thumbnail| B["get_sketchfab_model_preview"]
  B -->|base64 Image| C["LLM Preview"]
  A -->|glTF model| D["download_sketchfab_model"]
  D -->|calculate AABB| E["Normalize Size"]
  E -->|apply scale| F["Imported Model"]
  F -->|dimensions + bbox| G["LLM Result"]

File Walkthrough

Relevant files

Enhancement

addon.py `Add preview and size normalization for Sketchfab models` addon.py Add `get_sketchfab_model_preview` handler to fetch and encode model thumbnails as base64 Extend `download_sketchfab_model` with `normalize_size` and `target_size` parameters Implement bounding box calculation using `_get_aabb` method for all mesh objects Apply uniform scaling to normalize largest dimension to target_size Return detailed import metadata including dimensions, bounding box, and scale factor	+192/-6
server.py `Add preview tool and require explicit size specification` src/blender_mcp/server.py Add new `get_sketchfab_model_preview` MCP tool that returns Image type for visual confirmation Modify `download_sketchfab_model` to require `target_size` parameter with practical examples Always enable normalization (remove optional `normalize_size` parameter) Enhance output formatting with dimensions, bounding box, and normalization details Add comprehensive docstring with size examples for common object types	+78/-6

Summary by CodeRabbit

New Features
- Preview Sketchfab model thumbnails before downloading.
- Optional model size normalization on download (scale to target largest dimension).
Improvements
- Enhanced error handling and clearer failure messages for Sketchfab operations.
- Download responses include richer asset metadata (object names, dimensions, world bounding box, scale/normalization status).

_{✏️ Tip: You can customize this high-level summary in your review settings.}

- Add get_sketchfab_model_preview handler in addon.py - Fetches thumbnail from Sketchfab API - Returns base64-encoded image with model info - Add get_sketchfab_model_preview MCP tool in server.py - Returns Image type for visual confirmation - Allows users to preview models before downloading

- Add normalize_size and target_size parameters to download_sketchfab_model - Calculate combined bounding box for all imported mesh objects - Apply uniform scale to normalize largest dimension to target_size - Return detailed import info: dimensions, bounding box, scale factor - Default: normalize_size=True, target_size=1.0 (1 meter) This prevents extremely large or small models from being imported, making it easier for LLMs to work with consistent object sizes.

- Remove normalize_size parameter (always normalize now) - Make target_size a required parameter with no default value - Add helpful examples in docstring for common object sizes - Forces LLM to explicitly consider and specify model size

coderabbitai · 2025-12-02T09:53:27Z

Walkthrough

Added a new public endpoint get_sketchfab_model_preview(uid) to fetch and return a base64-encoded Sketchfab thumbnail with metadata, and extended download_sketchfab_model(uid, normalize_size=False, target_size=1.0) to optionally normalize imported model size, recalculate world bounds, and return richer metadata and bounding box information.

Changes

Cohort / File(s)	Summary
Sketchfab preview & model download logic `addon.py`, `src/blender_mcp/server.py`	Added `get_sketchfab_model_preview(uid)` to fetch model metadata and thumbnail from Sketchfab (auth, select preferred thumbnail, download, base64-encode, return metadata). Extended `download_sketchfab_model(uid, normalize_size=False, target_size=1.0)` to support optional size normalization, compute/apply scale to root objects, recompute world bounding box and dimensions, and return richer result payload (imported object names, world_bounding_box, dimensions, scale_applied, normalized) with enhanced error handling. Updated command routing to expose the preview endpoint and propagate enhanced result fields.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server as src/blender_mcp/server.py
    participant Addon as addon.py
    participant Sketchfab
    participant Blender

    rect rgb(220,235,255)
    Note over Client,Server: Preview request flow
    Client->>Server: get_sketchfab_model_preview(uid)
    Server->>Addon: get_sketchfab_model_preview(uid)
    Addon->>Sketchfab: GET model metadata (auth) -> get thumbnail URLs
    Sketchfab-->>Addon: metadata + thumbnail URLs
    Addon->>Sketchfab: Download chosen thumbnail
    Sketchfab-->>Addon: Image bytes
    Addon->>Addon: Encode image to base64 + attach metadata
    Addon-->>Server: base64 image + metadata
    Server-->>Client: Image object / metadata
    end

    rect rgb(255,245,220)
    Note over Client,Server: Download + optional normalization flow
    Client->>Server: download_sketchfab_model(uid, target_size)
    Server->>Addon: download_sketchfab_model(uid, normalize_size=True/False, target_size)
    Addon->>Sketchfab: Fetch model files (auth)
    Sketchfab-->>Addon: Model archive/files
    Addon->>Blender: Import model into scene
    Addon->>Blender: Query world bounding box & dimensions
    Blender-->>Addon: bbox & dimensions
    alt normalize_size == True
        Addon->>Addon: Compute scale factor to match target_size
        Addon->>Blender: Apply scale to imported root objects
        Addon->>Blender: Recompute world bbox & dimensions
        Blender-->>Addon: updated bbox & dimensions
    end
    Addon-->>Server: Result payload (object names, bbox, dims, scale_applied, normalized)
    Server-->>Client: Formatted success message + metadata
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Inspect bounding box computation and coordinate-frame handling in addon.py.
Verify scale application across object hierarchies and that recomputed world bounds are correct.
Review Sketchfab API authentication, timeout/error paths, and thumbnail selection logic.
Validate propagation and formatting of enriched result fields in src/blender_mcp/server.py.

Possibly related PRs

Add Sketchfab integration #108 — Overlaps Sketchfab integration changes and earlier edits to download_sketchfab_model, likely modifies related command handling and download logic.

Suggested labels

Review effort 4/5

Poem

🐰 I hop through code and nibble bytes,

Thumbnails fetched in cozy nights,
Models scaled to snug and neat,
Bounding boxes snug and sweet,
A rabbit cheers — imports complete! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title accurately summarizes both main features added: Sketchfab preview fetching and model size normalization for downloads.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

qodo-code-review · 2025-12-02T09:53:46Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	API token exposure Description: The Sketchfab API key is read directly from Blender scene context and sent in an Authorization header without any masking or transport validation; if logs, exceptions, or proxies capture headers, the token could be exposed—consider avoiding logging sensitive headers and ensuring HTTPS is enforced. addon.py [1577-1582] Referred Code api_key = bpy.context.scene.blendermcp_sketchfab_api_key if not api_key: return {"error": "Sketchfab API key is not configured"} headers = {"Authorization": f"Token {api_key}"}
	Untrusted content fetch Description: The thumbnail download and format detection rely on Content-Type and URL extension without validating or constraining image size/type; fetching untrusted URLs can lead to large file downloads (DoS) or unexpected formats—add size limits and stricter content-type checks before base64 encoding. addon.py [1622-1635] Referred Code img_response = requests.get(thumbnail_url, timeout=30) if img_response.status_code != 200: return {"error": f"Failed to download thumbnail: {img_response.status_code}"} # Encode image as base64 image_data = base64.b64encode(img_response.content).decode('ascii') # Determine format from content type or URL content_type = img_response.headers.get("Content-Type", "") if "png" in content_type or thumbnail_url.endswith(".png"): img_format = "png" else: img_format = "jpeg"
	Resource exhaustion risk Description: Applying scale factors from external model data to all imported objects without constraints could produce extremely large or tiny transforms causing resource exhaustion or UI lockups; validate target_size and clamp scale_factor to a sane range before applying. addon.py [1806-1819] Referred Code scale_factor = target_size / max_dimension scale_applied = scale_factor # Apply scale to all imported objects (not just meshes) for obj in imported_objects: obj.scale = ( obj.scale.x * scale_factor, obj.scale.y * scale_factor, obj.scale.z * scale_factor ) # Update the scene to recalculate bounding boxes bpy.context.view_layer.update()
	Error info leakage Description: Errors from Blender are raised as generic Exceptions and may propagate raw backend messages; if backend includes sensitive info (paths, tokens), this could leak to clients—sanitize error messages before surfacing them. server.py [696-699] Referred Code except Exception as e: logger.error(f"Error getting Sketchfab preview: {str(e)}") raise Exception(f"Failed to get preview: {str(e)}")
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Insufficient Auditing: New critical actions (model preview fetch and model download/import with normalization) lack structured audit logs including actor identity and outcome details beyond generic info/error messages. Referred Code try: blender = get_blender_connection() logger.info(f"Getting Sketchfab model preview for UID: {uid}") result = blender.send_command("get_sketchfab_model_preview", {"uid": uid}) if result is None: raise Exception("Received no response from Blender") if "error" in result: raise Exception(result["error"]) # Decode base64 image data image_data = base64.b64decode(result["image_data"]) img_format = result.get("format", "jpeg") # Log model info model_name = result.get("model_name", "Unknown") author = result.get("author", "Unknown") logger.info(f"Preview retrieved for '{model_name}' by {author}") ... (clipped 72 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Edge Cases Missing: The preview and download flows return generic error strings and do not validate inputs like empty uid or handle network/content-type anomalies comprehensively, potentially limiting actionable context. Referred Code def get_sketchfab_model_preview(self, uid): """Get thumbnail preview image of a Sketchfab model by its UID""" try: import base64 api_key = bpy.context.scene.blendermcp_sketchfab_api_key if not api_key: return {"error": "Sketchfab API key is not configured"} headers = {"Authorization": f"Token {api_key}"} # Get model info which includes thumbnails response = requests.get( f"https://api.sketchfab.com/v3/models/{uid}", headers=headers, timeout=30 ) if response.status_code == 401: return {"error": "Authentication failed (401). Check your API key."} ... (clipped 74 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Sensitive Errors: Exceptions are converted to user-visible error strings (e.g., returning traceback via print and str(e)) which may expose internal details rather than logging them securely. Referred Code except requests.exceptions.Timeout: return {"error": "Request timed out. Check your internet connection."} except Exception as e: import traceback traceback.print_exc() return {"error": f"Failed to get model preview: {str(e)}"} Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Input Validation: The new tools accept uid and target_size without explicit validation (e.g., ensuring uid format and target_size > 0), which could lead to unexpected behavior or downstream errors. Referred Code @mcp.tool() def download_sketchfab_model( ctx: Context, uid: str, target_size: float ) -> str: """ Download and import a Sketchfab model by its UID. The model will be scaled so its largest dimension equals target_size. Parameters: - uid: The unique identifier of the Sketchfab model - target_size: REQUIRED. The target size in Blender units/meters for the largest dimension. You must specify the desired size for the model. Examples: - Chair: target_size=1.0 (1 meter tall) - Table: target_size=0.75 (75cm tall) - Car: target_size=4.5 (4.5 meters long) - Person: target_size=1.7 (1.7 meters tall) - Small object (cup, phone): target_size=0.1 to 0.3 ... (clipped 12 lines) Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

qodo-code-review · 2025-12-02T09:55:02Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Validate normalization parameters Add validation for `target_size` to ensure it is positive and for `max_dimension` to prevent silent normalization failures or division by zero errors. addon.py [1803-1815] # Apply normalization if requested scale_applied = 1.0 -if normalize_size and max_dimension > 0: +if normalize_size: + if target_size <= 0: + return {"error": f"Invalid target_size: {target_size}. Must be positive."} + + if max_dimension <= 0: + return {"error": f"Cannot normalize model: invalid dimensions (max={max_dimension})"} + scale_factor = target_size / max_dimension scale_applied = scale_factor # Apply scale to all imported objects (not just meshes) for obj in imported_objects: obj.scale = ( obj.scale.x * scale_factor, obj.scale.y * scale_factor, obj.scale.z * scale_factor ) Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that an invalid `target_size` (e.g., zero or negative) is not handled, which could lead to errors. It also correctly notes that a `max_dimension` of zero would cause normalization to be skipped silently. Adding validation for these parameters makes the function more robust and prevents unexpected behavior.	Medium
	Validate target_size parameter Before calling the Blender command, validate that the `target_size` parameter is a positive number to prevent downstream errors and provide a clear error message to the user. src/blender_mcp/server.py [729-733] +if target_size is None or target_size <= 0: + return f"Error: target_size must be a positive number (got: {target_size})" + result = blender.send_command("download_sketchfab_model", { "uid": uid, "normalize_size": True, # Always normalize "target_size": target_size }) Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that an invalid `target_size` (e.g., non-positive) would be passed to the Blender addon, causing an error there. Adding this validation on the server side provides a faster and clearer error message to the end-user, which is a significant improvement in usability and robustness.	Medium
	✅ ~~Validate successful AABB calculations~~ Suggestion Impact: The commit did not add the suggested explicit "valid_bbox_count==0" check, but it changed the implementation to compute the combined world bounding box directly from each mesh's bound_box corners (without per-object AABB calls/try-except). This effectively reduces/eliminates the prior failure mode where all AABB calculations could fail and leave all_min/all_max at infinities. code diff: - # Calculate combined bounding box for all mesh objects - mesh_objects = [obj for obj in imported_objects if obj.type == 'MESH'] + # Find root objects (objects without parents in the imported set) + root_objects = [obj for obj in imported_objects if obj.parent is None] + + # Helper function to recursively get all mesh children + def get_all_mesh_children(obj): + """Recursively collect all mesh objects in the hierarchy""" + meshes = [] + if obj.type == 'MESH': + meshes.append(obj) + for child in obj.children: + meshes.extend(get_all_mesh_children(child)) + return meshes + + # Collect ALL meshes from the entire hierarchy (starting from roots) + all_meshes = [] + for obj in root_objects: + all_meshes.extend(get_all_mesh_children(obj)) - if mesh_objects: - # Calculate combined AABB for all meshes + if all_meshes: + # Calculate combined world bounding box for all meshes all_min = mathutils.Vector((float('inf'), float('inf'), float('inf'))) all_max = mathutils.Vector((float('-inf'), float('-inf'), float('-inf'))) - for obj in mesh_objects: - try: - bbox = self._get_aabb(obj) - obj_min = mathutils.Vector(bbox[0]) - obj_max = mathutils.Vector(bbox[1]) - - all_min.x = min(all_min.x, obj_min.x) - all_min.y = min(all_min.y, obj_min.y) - all_min.z = min(all_min.z, obj_min.z) - - all_max.x = max(all_max.x, obj_max.x) - all_max.y = max(all_max.y, obj_max.y) - all_max.z = max(all_max.z, obj_max.z) - except Exception as e: - print(f"Warning: Could not get AABB for {obj.name}: {e}") - continue + for mesh_obj in all_meshes: + # Get world-space bounding box corners + for corner in mesh_obj.bound_box: + world_corner = mesh_obj.matrix_world @ mathutils.Vector(corner) + all_min.x = min(all_min.x, world_corner.x) + all_min.y = min(all_min.y, world_corner.y) + all_min.z = min(all_min.z, world_corner.z) + all_max.x = max(all_max.x, world_corner.x) + all_max.y = max(all_max.y, world_corner.y) + all_max.z = max(all_max.z, world_corner.z) # Calculate dimensions dimensions = [ @@ -1806,36 +1815,31 @@ scale_factor = target_size / max_dimension scale_applied = scale_factor - # Apply scale to all imported objects (not just meshes) - for obj in imported_objects: - obj.scale = ( - obj.scale.x * scale_factor, - obj.scale.y * scale_factor, - obj.scale.z * scale_factor + # ✅ Only apply scale to ROOT objects (not children!) + # Child objects inherit parent's scale through matrix_world + for root in root_objects: + root.scale = ( + root.scale.x * scale_factor, + root.scale.y * scale_factor, + root.scale.z * scale_factor ) - # Update the scene to recalculate bounding boxes + # Update the scene to recalculate matrix_world for all objects bpy.context.view_layer.update() # Recalculate bounding box after scaling all_min = mathutils.Vector((float('inf'), float('inf'), float('inf'))) all_max = mathutils.Vector((float('-inf'), float('-inf'), float('-inf'))) - for obj in mesh_objects: - try: - bbox = self._get_aabb(obj) - obj_min = mathutils.Vector(bbox[0]) - obj_max = mathutils.Vector(bbox[1]) - - all_min.x = min(all_min.x, obj_min.x) - all_min.y = min(all_min.y, obj_min.y) - all_min.z = min(all_min.z, obj_min.z) - - all_max.x = max(all_max.x, obj_max.x) - all_max.y = max(all_max.y, obj_max.y) - all_max.z = max(all_max.z, obj_max.z) - except Exception: - continue + for mesh_obj in all_meshes: + for corner in mesh_obj.bound_box: + world_corner = mesh_obj.matrix_world @ mathutils.Vector(corner) + all_min.x = min(all_min.x, world_corner.x) + all_min.y = min(all_min.y, world_corner.y) + all_min.z = min(all_min.z, world_corner.z) + all_max.x = max(all_max.x, world_corner.x) + all_max.y = max(all_max.y, world_corner.y) + all_max.z = max(all_max.z, world_corner.z) Add a check to ensure at least one bounding box was successfully calculated before proceeding with dimension and scaling calculations. If none were successful, handle it as a case with no valid mesh objects. addon.py [1770-1793] # Calculate combined bounding box for all mesh objects mesh_objects = [obj for obj in imported_objects if obj.type == 'MESH'] if mesh_objects: # Calculate combined AABB for all meshes all_min = mathutils.Vector((float('inf'), float('inf'), float('inf'))) all_max = mathutils.Vector((float('-inf'), float('-inf'), float('-inf'))) + valid_bbox_count = 0 for obj in mesh_objects: try: bbox = self._get_aabb(obj) - ... + obj_min = mathutils.Vector(bbox[0]) + obj_max = mathutils.Vector(bbox[1]) + + all_min.x = min(all_min.x, obj_min.x) + all_min.y = min(all_min.y, obj_min.y) + all_min.z = min(all_min.z, obj_min.z) + + all_max.x = max(all_max.x, obj_max.x) + all_max.y = max(all_max.y, obj_max.y) + all_max.z = max(all_max.z, obj_max.z) + valid_bbox_count += 1 except Exception as e: print(f"Warning: Could not get AABB for {obj.name}: {e}") continue + + # Only proceed if at least one valid bounding box was computed + if valid_bbox_count == 0: + world_bounding_box = None + dimensions = None + scale_applied = 1.0 + else: + # Calculate dimensions... `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies an edge case where if all AABB calculations fail, the function would return invalid data (`-inf` for dimensions). This is a valid bug fix that improves the robustness and correctness of the function's output.	Medium
High-level	Reconsider always-on normalization approach The `download_sketchfab_model` function in the MCP server hardcodes `normalize_size=True`, creating an inconsistency with the addon which accepts it as a parameter. This should be resolved by either removing the parameter from the addon or exposing it in the MCP server for consistency and flexibility. Examples: src/blender_mcp/server.py [729-733] result = blender.send_command("download_sketchfab_model", { "uid": uid, "normalize_size": True, # Always normalize "target_size": target_size }) addon.py [1658-1665] def download_sketchfab_model(self, uid, normalize_size=False, target_size=1.0): """Download a model from Sketchfab by its UID Parameters: - uid: The unique identifier of the Sketchfab model - normalize_size: If True, scale the model so its largest dimension equals target_size - target_size: The target size in Blender units (meters) for the largest dimension """ Solution Walkthrough: Before: # src/blender_mcp/server.py @mcp.tool() def download_sketchfab_model(ctx: Context, uid: str, target_size: float) -> str: # ... result = blender.send_command("download_sketchfab_model", { "uid": uid, "normalize_size": True, # Always normalize "target_size": target_size }) # ... # addon.py class BlenderMCPServer: def download_sketchfab_model(self, uid, normalize_size=False, target_size=1.0): # ... if normalize_size and max_dimension > 0: # ... apply scaling # ... After: # src/blender_mcp/server.py @mcp.tool() def download_sketchfab_model( ctx: Context, uid: str, target_size: float, normalize_size: bool = True # Make it optional ) -> str: # ... result = blender.send_command("download_sketchfab_model", { "uid": uid, "normalize_size": normalize_size, # Pass the parameter "target_size": target_size }) # ... # addon.py class BlenderMCPServer: # No change needed, as it already supports the parameter def download_sketchfab_model(self, uid, normalize_size=False, target_size=1.0): # ... Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a design inconsistency between the MCP server and the addon, where the server hardcodes `normalize_size=True` while the addon retains it as a flexible parameter, making the API confusing.	Medium
Update

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (3)

src/blender_mcp/server.py (2)

696-698: Consider using exception chaining for better traceability.

When re-raising exceptions, use raise ... from e to preserve the original traceback chain.
     except Exception as e:
         logger.error(f"Error getting Sketchfab preview: {str(e)}")
-        raise Exception(f"Failed to get preview: {str(e)}")
+        raise Exception(f"Failed to get preview: {str(e)}") from e
747-747: Remove unnecessary f-string prefix.

This string has no placeholders, so the f prefix is unnecessary.
-            output = f"Successfully imported model.\n"
+            output = "Successfully imported model.\n"

addon.py (1)

1770-1845: Consider extracting duplicate bounding box calculation logic.

The bounding box calculation code (lines 1774-1793) is duplicated after scaling (lines 1820-1838). Consider extracting this into a helper method to reduce code duplication and improve maintainability.

Example helper:

def _calculate_combined_aabb(self, mesh_objects):
    """Calculate combined AABB for multiple mesh objects."""
    all_min = mathutils.Vector((float('inf'), float('inf'), float('inf')))
    all_max = mathutils.Vector((float('-inf'), float('-inf'), float('-inf')))
    
    for obj in mesh_objects:
        try:
            bbox = self._get_aabb(obj)
            obj_min = mathutils.Vector(bbox[0])
            obj_max = mathutils.Vector(bbox[1])
            
            all_min.x = min(all_min.x, obj_min.x)
            all_min.y = min(all_min.y, obj_min.y)
            all_min.z = min(all_min.z, obj_min.z)
            
            all_max.x = max(all_max.x, obj_max.x)
            all_max.y = max(all_max.y, obj_max.y)
            all_max.z = max(all_max.z, obj_max.z)
        except Exception as e:
            print(f"Warning: Could not get AABB for {obj.name}: {e}")
            continue
    
    return all_min, all_max

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 668be7f and fcefedf.

📒 Files selected for processing (2)

addon.py (3 hunks)
src/blender_mcp/server.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

src/blender_mcp/server.py (1)

addon.py (2)

get_sketchfab_model_preview (1572-1656)

download_sketchfab_model (1658-1875)

addon.py (1)

src/blender_mcp/server.py (2)

get_sketchfab_model_preview (660-698)

download_sketchfab_model (702-772)

🪛 Ruff (0.14.7)

src/blender_mcp/server.py

661-661: Unused function argument: ctx

(ARG001)

680-680: Abstract raise to an inner function

(TRY301)

680-680: Create your own exception

(TRY002)

680-680: Avoid specifying long messages outside the exception class

(TRY003)

683-683: Abstract raise to an inner function

(TRY301)

683-683: Create your own exception

(TRY002)

696-696: Do not catch blind exception: Exception

(BLE001)

697-697: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

697-697: Use explicit conversion flag

Replace with conversion flag

(RUF010)

698-698: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

698-698: Create your own exception

(TRY002)

698-698: Avoid specifying long messages outside the exception class

(TRY003)

698-698: Use explicit conversion flag

Replace with conversion flag

(RUF010)

703-703: Unused function argument: ctx

(ARG001)

747-747: f-string without any placeholders

Remove extraneous f prefix

(F541)

addon.py

1653-1653: Do not catch blind exception: Exception

(BLE001)

1656-1656: Use explicit conversion flag

Replace with conversion flag

(RUF010)

1791-1791: Do not catch blind exception: Exception

(BLE001)

1837-1838: try-except-continue detected, consider logging the exception

(S112)

1837-1837: Do not catch blind exception: Exception

(BLE001)

1866-1866: Consider moving this statement to an else block

(TRY300)

🔇 Additional comments (6)

src/blender_mcp/server.py (2)

701-724: Good design: requiring explicit size specification.

Making target_size a required parameter forces the caller to think about proper model sizing, which helps maintain consistent scale relationships between objects in the scene. The comprehensive docstring with examples is helpful.

660-694: LGTM!

The preview functionality is well-implemented with proper error handling and logging. This enables visual confirmation of models before downloading, which is a nice UX improvement.

addon.py (4)

233-238: LGTM!

Handler registration follows the established pattern and is properly guarded by the Sketchfab enable flag.

1572-1656: LGTM!

The implementation is solid with:

Proper API authentication

Smart thumbnail selection preferring medium sizes (400-800px)

Comprehensive error handling for various HTTP status codes

Timeout protection

Rich metadata in the response

1808-1818: LGTM!

Correctly applies scaling to all imported objects (not just meshes) to maintain hierarchy and relative positioning, then updates the view layer to refresh transforms.

1852-1866: LGTM!

Result construction is clean with appropriate conditional inclusion of metadata and sensible precision for float values.

coderabbitai · 2025-12-02T09:56:05Z

+            if mesh_objects:
+                # Calculate combined AABB for all meshes
+                all_min = mathutils.Vector((float('inf'), float('inf'), float('inf')))
+                all_max = mathutils.Vector((float('-inf'), float('-inf'), float('-inf')))
+
+                for obj in mesh_objects:
+                    try:
+                        bbox = self._get_aabb(obj)
+                        obj_min = mathutils.Vector(bbox[0])
+                        obj_max = mathutils.Vector(bbox[1])
+
+                        all_min.x = min(all_min.x, obj_min.x)
+                        all_min.y = min(all_min.y, obj_min.y)
+                        all_min.z = min(all_min.z, obj_min.z)
+
+                        all_max.x = max(all_max.x, obj_max.x)
+                        all_max.y = max(all_max.y, obj_max.y)
+                        all_max.z = max(all_max.z, obj_max.z)
+                    except Exception as e:
+                        print(f"Warning: Could not get AABB for {obj.name}: {e}")
+                        continue
+
+                # Calculate dimensions
+                dimensions = [
+                    all_max.x - all_min.x,
+                    all_max.y - all_min.y,
+                    all_max.z - all_min.z
+                ]
+                max_dimension = max(dimensions)


⚠️ Potential issue | 🟡 Minor

Potential edge case: invalid bounding box when all mesh AABBs fail.

If all mesh objects fail the _get_aabb call (e.g., due to exceptions), all_min and all_max will remain at their initial inf/-inf values, resulting in invalid bounding box and dimension calculations.

Consider adding a check after the loop:

+ # Check if any valid bounding boxes were found + if all_min.x == float('inf'): + # No valid bounding boxes found + world_bounding_box = None + dimensions = None + max_dimension = 0 + else: # Calculate dimensions dimensions = [ all_max.x - all_min.x, ...

🧰 Tools

🪛 Ruff (0.14.7)

1791-1791: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents

In addon.py around lines 1773 to 1801, the loop that collects per-mesh AABBs doesn't handle the case where every _get_aabb call fails, leaving all_min/ all_max as inf/-inf and producing invalid dimensions; after the loop check whether all_min.x is still inf (or any component is inf/-inf) and if so handle the error path (e.g., log/raise a clear error or set dimensions to zeros or skip further processing) before computing dimensions and max_dimension so you never compute with infinite values.

…rchical models - Add recursive get_all_mesh_children() to collect meshes from entire hierarchy - Fix bounding box calculation to include all nested mesh objects - Use transform_apply() to properly apply scale to entire object hierarchy - Prevents parts from becoming scattered when scaling complex models This fixes issues with Sketchfab models that have deep hierarchies like: Sketchfab_model (EMPTY) └─ RootNode (EMPTY) └─ Chassis (MESH) ← now correctly included

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

addon.py (2)

1572-1656: Preview flow is solid; consider narrowing the catch-all exception

The overall logic (API key check, model fetch, thumbnail selection, base64 encoding, and contextual metadata) looks good and matches how get_sketchfab_model_preview is consumed in server.py. To avoid masking programming/runtime errors and to address BLE001, you could narrow the final except Exception as e to something like requests.exceptions.RequestException plus json.JSONDecodeError, keeping only genuinely unexpected failures out of the happy path while still returning a clean error dict for network/JSON issues.

1658-1883: Download + normalization pipeline looks correct; tighten normalization API around target_size

The download, zip path‑traversal checks, glTF import, hierarchy mesh aggregation, and AABB/dimension recomputation after scaling all look coherent and line up with how download_sketchfab_model is called from server.py (including the expected result keys). Given that the MCP server always passes normalize_size=True and requires target_size, you might (a) simplify this API by defaulting normalize_size to True or removing it to reflect that normalization is now mandatory from the tool’s perspective, and (b) add an early guard for invalid target_size (e.g., <= 0) when normalization is requested so models are never accidentally scaled to zero or a negative size.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fcefedf and 44979b4.

📒 Files selected for processing (1)

addon.py (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

addon.py (1)

src/blender_mcp/server.py (2)

get_sketchfab_model_preview (660-698)

download_sketchfab_model (702-772)

🪛 Ruff (0.14.7)

addon.py

1653-1653: Do not catch blind exception: Exception

(BLE001)

1656-1656: Use explicit conversion flag

Replace with conversion flag

(RUF010)

1882-1882: Consider moving this statement to an else block

(TRY300)

🔇 Additional comments (1)

addon.py (1)

231-239: Sketchfab handler registration correctly exposes the new preview command

Adding "get_sketchfab_model_preview": self.get_sketchfab_model_preview to the Sketchfab handler map is consistent with the existing pattern and keeps the Blender side aligned with the MCP server tools. No changes needed.

- Find root objects (objects without parents) instead of all selected objects - Apply scale factor ONLY to root objects, not children - Remove transform_apply (parent scale automatically affects children via matrix_world) - Child meshes inherit scaling through Blender's hierarchy system Fixes issue where scale was applied to every object in hierarchy, causing cumulative scaling (0.007^5 ≈ 0) that made models invisible.

* feat: Add Sketchfab model preview functionality - Add get_sketchfab_model_preview handler in addon.py - Fetches thumbnail from Sketchfab API - Returns base64-encoded image with model info - Add get_sketchfab_model_preview MCP tool in server.py - Returns Image type for visual confirmation - Allows users to preview models before downloading * feat: Add size normalization for Sketchfab model imports - Add normalize_size and target_size parameters to download_sketchfab_model - Calculate combined bounding box for all imported mesh objects - Apply uniform scale to normalize largest dimension to target_size - Return detailed import info: dimensions, bounding box, scale factor - Default: normalize_size=True, target_size=1.0 (1 meter) This prevents extremely large or small models from being imported, making it easier for LLMs to work with consistent object sizes. * feat: Make target_size required parameter for Sketchfab downloads - Remove normalize_size parameter (always normalize now) - Make target_size a required parameter with no default value - Add helpful examples in docstring for common object sizes - Forces LLM to explicitly consider and specify model size * chore: Remove temporary test files from development * fix: Correct bounding box calculation and scale application for hierarchical models - Add recursive get_all_mesh_children() to collect meshes from entire hierarchy - Fix bounding box calculation to include all nested mesh objects - Use transform_apply() to properly apply scale to entire object hierarchy - Prevents parts from becoming scattered when scaling complex models This fixes issues with Sketchfab models that have deep hierarchies like: Sketchfab_model (EMPTY) └─ RootNode (EMPTY) └─ Chassis (MESH) ← now correctly included * fix: Apply scale only to root objects to prevent cumulative scaling - Find root objects (objects without parents) instead of all selected objects - Apply scale factor ONLY to root objects, not children - Remove transform_apply (parent scale automatically affects children via matrix_world) - Child meshes inherit scaling through Blender's hierarchy system Fixes issue where scale was applied to every object in hierarchy, causing cumulative scaling (0.007^5 ≈ 0) that made models invisible.

JinTanba added 4 commits December 2, 2025 17:29

chore: Remove temporary test files from development

fcefedf

qodo-code-review Bot added the Review effort 3/5 label Dec 2, 2025

coderabbitai Bot reviewed Dec 2, 2025

View reviewed changes

JinTanba changed the title ~~Feature/sketchfab preview~~ sketchfab preview/model size normalisation Dec 2, 2025

ahujasid merged commit 4794edc into ahujasid:main Jan 5, 2026
1 check passed

coderabbitai Bot mentioned this pull request Jan 6, 2026

feat: Add tool annotations for improved LLM tool understanding #166

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sketchfab preview/model size normalisation#163

sketchfab preview/model size normalisation#163
ahujasid merged 6 commits intoahujasid:mainfrom
JinTanba:feature/sketchfab-preview

JinTanba commented Dec 2, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Dec 2, 2025 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Dec 2, 2025 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Dec 2, 2025 •

edited

Loading

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Dec 2, 2025

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

JinTanba commented Dec 2, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

PR Type

Description

Diagram Walkthrough

File Walkthrough

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

Pre-merge checks and finishing touches

Uh oh!

qodo-code-review Bot commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Compliance Guide 🔍

Uh oh!

qodo-code-review Bot commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JinTanba commented Dec 2, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Dec 2, 2025 •

edited

Loading

qodo-code-review Bot commented Dec 2, 2025 •

edited

Loading

qodo-code-review Bot commented Dec 2, 2025 •

edited

Loading