feat: Add Geometry Node Module#92
Conversation
📝 WalkthroughWalkthroughThis PR extends the Blender MCP server with geometry nodes support, adds integrations for multiple asset libraries (Polyhaven, Sketchfab, Hyper3D, Hunyuan3D), introduces viewport screenshot and code execution tools, and implements telemetry instrumentation across the platform. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
README.md (1)
171-179: Helpful example commands
These commands illustrate the geometry node module usage well and will benefit users experimenting with procedural modeling.For consistency with the rest of the document, consider aligning the bullet points' style (e.g., capitalization, punctuation).
src/blender_mcp/server.py (5)
9-9: Remove the unusedUnionimport
Static analysis indicatesUnionis never used. Removing it avoids clutter and adheres to best practices.-from typing import AsyncIterator, Dict, Any, List, Union +from typing import AsyncIterator, Dict, Any, List🧰 Tools
🪛 Ruff (0.8.2)
9-9:
typing.Unionimported but unusedRemove unused import:
typing.Union(F401)
208-221: Use of globals for geometry node status
Using_geometry_nodes_enabledacross the code is convenient for quick checks. However, consider wrapping these flags into a dedicated class or configuration object for better maintainability if this project grows.
951-1044: Nested condition simplification
This function is generally solid. Consider combining nested if statements (lines 1009-1011) into a single condition to reduce complexity.Example diff:
-if isinstance(node_type_name, str): - if ',' in node_type_name: - node_type_name = [name.strip() for name in node_type_name.split(',') if name.strip()] +if isinstance(node_type_name, str) and ',' in node_type_name: + node_type_name = [name.strip() for name in node_type_name.split(',') if name.strip()]🧰 Tools
🪛 Ruff (0.8.2)
1009-1011: Use a single
ifstatement instead of nestedifstatements(SIM102)
1045-1145: Large single method
complete_geometry_nodeis quite extensive and handles many steps in a single function. Splitting it into smaller helper methods (e.g., node creation, link creation, and modifier parameter setup) enhances readability and maintainability.
1146-1162: Remove unused local variable
The variableenabledis assigned but never utilized, based on static analysis. You can omit it.- enabled = result.get("enabled", False) message = result.get("message", "")🧰 Tools
🪛 Ruff (0.8.2)
1155-1155: Local variable
enabledis assigned to but never usedRemove assignment to unused variable
enabled(F841)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)README.md(2 hunks)src/blender_mcp/server.py(5 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/blender_mcp/server.py (2)
addon.py (3)
get_node_info(2114-2204)complete_geometry_node(1700-1987)get_geometry_nodes_status(2595-2622)main.py (1)
main(3-5)
🪛 Ruff (0.8.2)
src/blender_mcp/server.py
9-9: typing.Union imported but unused
Remove unused import: typing.Union
(F401)
1009-1011: Use a single if statement instead of nested if statements
(SIM102)
1155-1155: Local variable enabled is assigned to but never used
Remove assignment to unused variable enabled
(F841)
🔇 Additional comments (5)
.gitignore (1)
11-12: Ignore JetBrains.ideafolder
No concerns with ignoring JetBrains IDE metadata. This is a standard entry for better version control hygiene.README.md (2)
154-154: New geometry node module heading
This heading clearly communicates the new geometry node functionality. Good addition.
185-185: No issues with spacing
This line appears to be a blank or spacing line. No further concerns.src/blender_mcp/server.py (2)
204-204: Global_geometry_nodes_enabledvariable declaration
Initialization looks fine. No immediate concerns regarding usage.
1170-1170: Entry point inclusion
Callingmain()on script execution is standard. No concerns here.
|
@duqich curious what this would add over the existing "execute_code" function, since it can run code for anything in Blender. Is this not possible? |
|
@ahujasid While execute_code can indeed perform any operation through the Blender Python API, including creating geometry node modifiers, LLMs face significant challenges in automatically constructing geometry node networks for two main reasons:
My implementation addresses these limitations by:
Beyond the core functionality, I've also:
This enables LLMs to construct geometry node networks more reliably. Additionally, if errors occur, the agent can:
This structured approach significantly improves the reliability and usability of procedural modeling through LLM-driven geometry nodes. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
src/blender_mcp/server.py (3)
9-9: Remove unused imports.The
UnionandListimports from thetypingmodule are never used in this file.-from typing import AsyncIterator, Dict, Any, List, Union +from typing import AsyncIterator, Dict, Any🧰 Tools
🪛 Ruff (0.8.2)
9-9:
typing.Listimported but unusedRemove unused import
(F401)
9-9:
typing.Unionimported but unusedRemove unused import
(F401)
766-859: Simplify nested conditional logic.The function implementation looks great overall, but the nested
ifstatements for processingnode_type_nameparameter could be simplified.- # Process node_type_name parameter, support comma-separated string - if node_type_name is not None: - # If it's a string, check if it's a comma-separated list - if isinstance(node_type_name, str): - # Check if it's a multi-node name with commas - if ',' in node_type_name: - # Split by comma and remove whitespace - node_type_name = [name.strip() for name in node_type_name.split(',') if name.strip()] + # Process node_type_name parameter, support comma-separated string + if node_type_name is not None and isinstance(node_type_name, str) and ',' in node_type_name: + # Split by comma and remove whitespace + node_type_name = [name.strip() for name in node_type_name.split(',') if name.strip()]🧰 Tools
🪛 Ruff (0.8.2)
824-826: Use a single
ifstatement instead of nestedifstatements(SIM102)
961-976: Remove unused variable.The
enabledvariable is assigned but never used in this function.- enabled = result.get("enabled", False) message = result.get("message", "")🧰 Tools
🪛 Ruff (0.8.2)
970-970: Local variable
enabledis assigned to but never usedRemove assignment to unused variable
enabled(F841)
addon.py (8)
15-16: Remove unusedOptionalimport.According to static analysis,
typing.Optionalis not used anywhere in the file. Removing it helps keep the imports concise and avoid confusion.Apply this diff to remove the unused import:
-from typing import List, Dict, Union, Any, Optional, Tuple +from typing import List, Dict, Union, Any, Tuple🧰 Tools
🪛 Ruff (0.8.2)
16-16:
typing.Optionalimported but unusedRemove unused import:
typing.Optional(F401)
30-31: Add clarifying docstring aroundIS_BLENDER_4.Although the code is self-explanatory, adding a short comment or docstring clarifies the purpose of checking the Blender major version. This helps new contributors or maintainers understand why certain conditionals exist for Blender 4.0+ compatibility.
105-105: Consider removing or clarifying commented placeholders if any.It looks like line 105 is marked changed with no visible content in the snippet. If this was merely a placeholder or leftover commentary, please ensure it remains purposeful or remove it to avoid confusion.
135-138: Adoptcontextlib.suppress(Exception)for silent exception handling.Multiple lines rely on a try-except-pass pattern (lines 135-138, 560-563, 618-621, 1726-1728). Using
with contextlib.suppress(Exception):is more pythonic and avoids swallowing system-level exceptions inadvertently. It reads clearer and prevents repeated pass blocks.Example refactor for lines 135-138:
-try: - self.socket.close() -except: - pass +import contextlib +with contextlib.suppress(Exception): + self.socket.close()Also applies to: 560-563, 618-621, 1726-1728
🧰 Tools
🪛 Ruff (0.8.2)
135-138: Use
contextlib.suppress(Exception)instead oftry-except-passReplace with
contextlib.suppress(Exception)(SIM105)
137-137: Do not use bare
except(E722)
573-573: Remove extraneous f-prefix from strings without placeholders.Lines 573, 624, 1901, 2150, 2188, and 2264 contain f-strings with no placeholders. Removing the
fprefix avoids confusion and minor overhead.-f"Requested resolution or format not available for this HDRI" +"Requested resolution or format not available for this HDRI"Also applies to: 624-624, 1901-1901, 2150-2150, 2188-2188, 2264-2264
🧰 Tools
🪛 Ruff (0.8.2)
573-573: f-string without any placeholders
Remove extraneous
fprefix(F541)
583-584: Consider merging nestedifconditions into a single statement.At lines 583-584, the nested checks can be simplified to one condition, as suggested by static analysis (SIM102), improving readability.
🧰 Tools
🪛 Ruff (0.8.2)
583-584: Use a single
ifstatement instead of nestedifstatements(SIM102)
1648-1648: Avoid assigning tolinkif unused.Line 1648 sets
link = node_group.links.new(from_socket, to_socket)but never referenceslink. This variable assignment is unnecessary; you can omit the variable altogether.-link = node_group.links.new(from_socket, to_socket) +node_group.links.new(from_socket, to_socket)🧰 Tools
🪛 Ruff (0.8.2)
1648-1648: Local variable
linkis assigned to but never usedRemove assignment to unused variable
link(F841)
1706-1706: Usekey in dictinstead ofkey in dict.keys().At lines 1706 and 1719, removing
.keys()is clearer, asif key in my_dict:is pythonic and avoids extra overhead.-if socket_name in socket_dict.keys(): +if socket_name in socket_dict:Also applies to: 1719-1719
🧰 Tools
🪛 Ruff (0.8.2)
1706-1706: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
addon.py(45 hunks)src/blender_mcp/server.py(5 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
addon.py (1)
src/blender_mcp/server.py (3)
get_geometry_nodes_status(962-976)get_node_info(767-858)complete_geometry_node(861-959)
🪛 Ruff (0.8.2)
src/blender_mcp/server.py
9-9: typing.List imported but unused
Remove unused import
(F401)
9-9: typing.Union imported but unused
Remove unused import
(F401)
824-826: Use a single if statement instead of nested if statements
(SIM102)
970-970: Local variable enabled is assigned to but never used
Remove assignment to unused variable enabled
(F841)
addon.py
16-16: typing.Optional imported but unused
Remove unused import: typing.Optional
(F401)
135-138: Use contextlib.suppress(Exception) instead of try-except-pass
Replace with contextlib.suppress(Exception)
(SIM105)
137-137: Do not use bare except
(E722)
560-563: Use contextlib.suppress(Exception) instead of try-except-pass
(SIM105)
562-562: Do not use bare except
(E722)
573-573: f-string without any placeholders
Remove extraneous f prefix
(F541)
583-584: Use a single if statement instead of nested if statements
(SIM102)
618-621: Use contextlib.suppress(Exception) instead of try-except-pass
Replace with contextlib.suppress(Exception)
(SIM105)
620-620: Do not use bare except
(E722)
624-624: f-string without any placeholders
Remove extraneous f prefix
(F541)
1648-1648: Local variable link is assigned to but never used
Remove assignment to unused variable link
(F841)
1706-1706: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
1719-1719: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
1726-1726: Do not use bare except
(E722)
1728-1728: Do not use bare except
(E722)
1901-1901: f-string without any placeholders
Remove extraneous f prefix
(F541)
1981-1981: Import socket from line 7 shadowed by loop variable
(F402)
1993-1993: Do not use bare except
(E722)
2092-2092: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
2150-2150: f-string without any placeholders
Remove extraneous f prefix
(F541)
2188-2188: f-string without any placeholders
Remove extraneous f prefix
(F541)
2264-2264: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (15)
src/blender_mcp/server.py (4)
204-204: LGTM: Properly tracked geometry nodes feature status.The global variable is a good approach to keep track of whether the geometry nodes feature is enabled.
208-221: Feature status tracking implementation looks good.The code now properly checks and updates the geometry nodes status when establishing a Blender connection.
739-752: Well-structured documentation for geometry nodes workflow.The asset creation strategy documentation provides clear guidance on using the geometry nodes feature with a logical step-by-step approach.
860-959: Well-implemented geometry node network creation function.The
complete_geometry_nodefunction is very well documented with clear parameter specifications and usage examples. The implementation checks for feature availability before proceeding and properly handles the node creation process.addon.py (11)
34-42: Data classNodeDefinitionappears correct.The usage of
@dataclassforNodeDefinitionis clean and clear, storing node parameters in a structured way. The class docstring is concise, and the typed fields help ensure clarity.
44-51: Data classNodeLinkis well-defined.Defining connections with explicit fields for node/socket references is a good approach. This fosters clear modeling of links between node sockets. No issues found here.
53-61: Data classGeometryNodeNetworkis coherent.This class straightforwardly groups nodes, links, and socket definitions. The approach is consistent with the other data classes, making the code maintainable and modular.
63-73: Data classSocketInfois well-structured.Using typed fields and capturing
default_valueis helpful for robust node creation. The docstring also clearly explains each field.
75-84: Data classPropertyInfois properly implemented.Enum options and property details are captured in a structured format, which is useful for introspection and dynamic UI building.
86-94: Data classNodeInfoaligns well with the existing data classes.All basic node metadata (description, inputs, outputs, properties) is included. This design fosters a consistent approach to geometry node information retrieval.
263-279: Check for redundant entries in handlers.New geometry node handlers at lines 268-269 partially duplicate the dictionary entry in lines 278-279. Ensure the logic in
_execute_command_internalremains free of collisions or repeated key usage (e.g.,'get_geometry_nodes_status'is assigned multiple times).
291-299: Hyper3D conditionally added handlers block.This conditional registration approach is consistent with PolyHaven’s logic above. Looks good. Just be sure you handle potential collisions in the
handlersdictionary (no issues seem present right now).
300-306: Conditional geometry node handlers registration.Registering geometry node commands only when
blendermcp_use_geometry_nodesis enabled is a clean approach. This ensures you don't load or expose geometry node functionalities unnecessarily. Implementation looks correct.
1452-1741: Review concurrency with main-thread scheduling for geometry node creation.This method,
complete_geometry_node, delegates node creation to the main thread viabpy.app.timers.register(...). Ensure that any shared state (e.g., node group references) is carefully guarded or remains static to prevent race conditions if other timers or threads also manipulate these objects. The logic for building node networks and linking them looks correct otherwise.Would you like to run additional multi-thread concurrency tests to confirm that no data races occur when multiple geometry node operations arrive in quick succession?
🧰 Tools
🪛 Ruff (0.8.2)
1648-1648: Local variable
linkis assigned to but never usedRemove assignment to unused variable
link(F841)
1706-1706: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
1719-1719: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
1726-1726: Do not use bare
except(E722)
1728-1728: Do not use bare
except(E722)
1867-2350:get_node_infoand supporting private methods are well-structured.The multi-step approach to gather node details, property data, and caching is thorough. It fosters quick lookups without frequently hitting Blender’s heavy node system. Overall, the logic is correct, though watch out for performance overhead if the node list becomes very large.
🧰 Tools
🪛 Ruff (0.8.2)
1901-1901: f-string without any placeholders
Remove extraneous
fprefix(F541)
1981-1981: Import
socketfrom line 7 shadowed by loop variable(F402)
1993-1993: Do not use bare
except(E722)
2092-2092: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
2150-2150: f-string without any placeholders
Remove extraneous
fprefix(F541)
2188-2188: f-string without any placeholders
Remove extraneous
fprefix(F541)
2264-2264: f-string without any placeholders
Remove extraneous
fprefix(F541)
|
@ahujasid |
|
Keeping this in the backlog for now, it's a good feature, but probably a bit too specific at this stage |
Hello @ahujasid, Geometry Nodes are one of the really important features of Blender. Their procedural nature can help the LLMs talk to and understand Blender. This Pull Request code from @duqich seems to have an UI option in the Blender AddOn to turn off Geometry Nodes if they and not needed. Can we revisit this Pull Request and get it merged into the main branch? |
|
@duqich if you can simplify the code and solve the conflicts i can merge |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/blender_mcp/server.py (1)
882-884: Bug: URL validation uses wrong variable.Line 883 validates
input_image_pathsinstead ofinput_image_urls, causing incorrect validation when URLs are provided.Proposed fix
elif input_image_urls is not None: - if not all(urlparse(i) for i in input_image_paths): + if not all(urlparse(i).scheme for i in input_image_urls): return "Error: not all image URLs are valid!" images = input_image_urls.copy()Note: Also added
.schemecheck sinceurlparse()always returns aParseResult(truthy), making the original validation ineffective.
🤖 Fix all issues with AI agents
In @src/blender_mcp/server.py:
- Around line 686-691: The telemetry decorator on get_sketchfab_model_preview is
using the wrong tool name; update the @telemetry_tool(...) argument to match the
function name (e.g., change "download_sketchfab_model" to
"get_sketchfab_model_preview") so the telemetry identifier aligns with the
get_sketchfab_model_preview tool and decorator.
- Around line 729-734: The download_sketchfab_model tool is missing the
@telemetry_tool decorator, causing inconsistent telemetry; add the
@telemetry_tool decorator directly above the existing @mcp.tool() on the
download_sketchfab_model function and ensure telemetry_tool is
imported/available in the module (matching how other tools are decorated) so
telemetry events are recorded consistently for download_sketchfab_model.
- Around line 304-328: The temp file created for the Blender screenshot
(temp_path) is not guaranteed to be removed if reading the file or any later
step raises; wrap the screenshot creation/reading/delete sequence in a
try/finally (or use a context manager surrounding the lifecycle) so that after
calling get_blender_connection() and
blender.send_command("get_viewport_screenshot", ...) you always attempt to
os.remove(temp_path) in the finally block if the file exists; ensure existing
error checks (result["error"] and existence of temp_path) remain and that
image_bytes is set from the with open(...) read inside the try before returning
or re-raising the exception.
🧹 Nitpick comments (14)
src/blender_mcp/telemetry.py (7)
38-49: Silent exception swallowing loses diagnostic information.The broad
except Exceptionwithpasscompletely silences errors when reading the version. Consider logging at debug level to aid troubleshooting.Proposed improvement
def get_package_version() -> str: """Get version from pyproject.toml""" try: pyproject_path = Path(__file__).parent.parent.parent.parent / "pyproject.toml" if pyproject_path.exists(): if tomli: with open(pyproject_path, "rb") as f: data = tomli.load(f) return data["project"]["version"] - except Exception: - pass + except Exception as e: + logger.debug(f"Could not read version from pyproject.toml: {e}") return "unknown"
113-113: Usinglogger.warningfor routine telemetry initialization is excessive.This will clutter logs with warnings on every server start. Use
logger.infoorlogger.debugfor non-error operational information.Proposed fix
- logger.warning(f"Telemetry initialized (enabled={self.config.enabled}, has_supabase={HAS_SUPABASE}, customer_uuid={self._customer_uuid})") + logger.info(f"Telemetry initialized (enabled={self.config.enabled}, has_supabase={HAS_SUPABASE})")Note: Also consider not logging the full
customer_uuideven at info level for privacy reasons.
141-163: Move successful return toelseblock for clearer control flow.The successful
return customer_uuidon line 160 should be in anelseblock after thetry, making it clearer that the fallback UUID on line 163 only applies when persistence fails.Proposed fix
def _get_or_create_uuid(self) -> str: """Get or create anonymous customer UUID""" try: data_dir = self._get_data_directory() uuid_file = data_dir / "customer_uuid.txt" if uuid_file.exists(): customer_uuid = uuid_file.read_text(encoding="utf-8").strip() if customer_uuid: return customer_uuid # Create new UUID customer_uuid = str(uuid.uuid4()) uuid_file.write_text(customer_uuid, encoding="utf-8") # Set restrictive permissions on Unix if sys.platform != "win32": os.chmod(uuid_file, 0o600) - - return customer_uuid except Exception as e: logger.debug(f"Failed to persist UUID: {e}") return str(uuid.uuid4()) + else: + return customer_uuid
176-184: Usinglogger.warningfor normal telemetry events is incorrect.Lines 178, 181, and 184 use
logger.warningfor routine flow control and event recording. Warnings should be reserved for actual problems. Uselogger.debugfor skipped events andlogger.info(ordebug) for recording events.Proposed fix
def record_event( ... ): """Record a telemetry event (non-blocking)""" if not self.config.enabled: - logger.warning(f"Telemetry disabled, skipping event: {event_type}") + logger.debug(f"Telemetry disabled, skipping event: {event_type}") return if not HAS_SUPABASE: - logger.warning(f"Supabase not available, skipping event: {event_type}") + logger.debug(f"Supabase not available, skipping event: {event_type}") return - logger.warning(f"Recording telemetry event: {event_type}, tool={tool_name}") + logger.debug(f"Recording telemetry event: {event_type}, tool={tool_name}")
230-248: Creating a new Supabase client for every event is inefficient.
_send_eventinstantiates a newClientper event, which adds connection overhead and may exhaust resources under load. Consider caching or reusing the client.Proposed approach
Initialize the Supabase client once in
__init__(or lazily on first send) and reuse it:# In __init__: self._supabase_client: Client | None = None # In _send_event: def _get_supabase_client(self) -> Client: if self._supabase_client is None: from supabase import ClientOptions options = ClientOptions(auto_refresh_token=False, persist_session=False) self._supabase_client = create_client( self.config.supabase_url, self.config.supabase_anon_key, options=options ) return self._supabase_client
267-267: Unused variableresponse.The response from the Supabase insert is assigned but never used. Remove the assignment or use it for error checking.
Proposed fix
- response = supabase.table("telemetry_events").insert(data, returning="minimal").execute() + supabase.table("telemetry_events").insert(data, returning="minimal").execute() logger.debug(f"Telemetry sent: {event_type}")
218-228: Worker thread runs forever with no graceful shutdown mechanism.The
_worker_loopruns an infinitewhile Trueloop with a blockingget(). When the process terminates, this daemon thread is abruptly killed, potentially losing queued events. Consider adding a sentinel value or timeout-based shutdown.Proposed approach
# Add a shutdown sentinel _SHUTDOWN = object() def shutdown(self): """Signal the worker to stop""" self._queue.put(_SHUTDOWN) self._worker.join(timeout=5.0) def _worker_loop(self): while True: event = self._queue.get() if event is _SHUTDOWN: break try: self._send_event(event) except Exception as e: logger.debug(f"Telemetry send failed: {e}") finally: with contextlib.suppress(Exception): self._queue.task_done()src/blender_mcp/server.py (7)
775-775: f-string without placeholders.The f-string prefix is unnecessary here since there are no interpolations.
Proposed fix
- output = f"Successfully imported model.\n" + output = "Successfully imported model.\n"
983-998: Missing telemetry decorator on Hunyuan3D tools.
get_hunyuan3d_status,generate_hunyuan3d_model,poll_hunyuan_job_status, andimport_generated_asset_hunyuanare missing@telemetry_tooldecorators, unlike other similar tools.Proposed fix for get_hunyuan3d_status
+@telemetry_tool("get_hunyuan3d_status") @mcp.tool() def get_hunyuan3d_status(ctx: Context) -> str:Apply similar decorators to:
generate_hunyuan3d_model→@telemetry_tool("generate_hunyuan3d_model")poll_hunyuan_job_status→@telemetry_tool("poll_hunyuan_job_status")import_generated_asset_hunyuan→@telemetry_tool("import_generated_asset_hunyuan")
1095-1101: Missing telemetry decorators on geometry nodes tools.
get_node_info,complete_geometry_node, andget_geometry_nodes_statuslack@telemetry_tooldecorators for consistency with other tools.Proposed fix
+@telemetry_tool("get_node_info") @mcp.tool() def get_node_info( ... +@telemetry_tool("complete_geometry_node") @mcp.tool() def complete_geometry_node( ... +@telemetry_tool("get_geometry_nodes_status") @mcp.tool() def get_geometry_nodes_status(ctx: Context) -> str:Also applies to: 1191-1198, 1293-1294
1300-1305: Unused variableenabled.The
enabledvariable is assigned but never used. Either use it or remove the assignment.Proposed fix
try: blender = get_blender_connection() result = blender.send_command("get_geometry_nodes_status") - enabled = result.get("enabled", False) message = result.get("message", "") return message
240-243: Bareexceptclause silently swallows all exceptions.The bare
except:on line 242 catches and ignores all exceptions includingKeyboardInterruptandSystemExit. Useexcept Exception:at minimum.Proposed fix
try: _blender_connection.disconnect() - except: + except Exception: pass _blender_connection = None
1003-1004: Use explicitOptionaltype hints.PEP 484 prohibits implicit
Optional. Usestr | NoneorOptional[str]for clarity.Proposed fix
def generate_hunyuan3d_model( ctx: Context, - text_prompt: str = None, - input_image_url: str = None + text_prompt: str | None = None, + input_image_url: str | None = None ) -> str:Similar fixes needed for:
- Line 614:
categories: str = None→categories: str | None = None- Line 1040:
job_id: str = None→job_id: str | None = None- Line 1197:
input_sockets: list = None→input_sockets: list | None = None
231-235: Geometry nodes and PolyHaven status checks add latency to every connection validation.Every call to
get_blender_connection()sends two additional round-trip commands (get_polyhaven_statusandget_geometry_nodes_status), regardless of whether the connection was already validated. With 27 calls to this function across various tools, this cumulative latency could be significant. The status values are stored in globals but are unconditionally refreshed on every call rather than cached with a TTL or checked only on initial connection.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.gitignoreREADME.mdTERMS_AND_CONDITIONS.mdaddon.pypyproject.tomlsrc/blender_mcp/server.pysrc/blender_mcp/telemetry.pysrc/blender_mcp/telemetry_decorator.py
🧰 Additional context used
🧬 Code graph analysis (2)
src/blender_mcp/telemetry_decorator.py (1)
src/blender_mcp/telemetry.py (1)
record_tool_usage(286-299)
src/blender_mcp/server.py (3)
src/blender_mcp/telemetry.py (2)
record_startup(302-307)get_telemetry(278-283)src/blender_mcp/telemetry_decorator.py (1)
telemetry_tool(16-65)addon.py (8)
get_sketchfab_status(1585-1645)search_sketchfab_models(1647-1707)get_sketchfab_model_preview(1709-1793)download_sketchfab_model(1795-2016)get_hunyuan3d_status(2020-2064)poll_hunyuan_job_status(2306-2307)import_generated_asset_hunyuan(2353-2354)get_node_info(2847-2937)
🪛 LanguageTool
README.md
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...Blender. This integration enables prompt assisted 3D modeling, scene creation, an...
(QB_NEW_EN_HYPHEN)
[grammar] ~125-~125: Use a hyphen to join words.
Context: ..." button and paste - To use as a project specific server, create `.cursor/mcp.jso...
(QB_NEW_EN_HYPHEN)
TERMS_AND_CONDITIONS.md
[uncategorized] ~19-~19: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ts and text inputs** you provide to the AI - Generated code produced in response to your pro...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~171-~171: Consider replacing this word to strengthen your wording.
Context: ... *Blender MCP is an independent project and is not affiliated with the Blender Foun...
(AND_THAT)
🪛 markdownlint-cli2 (0.18.1)
README.md
3-3: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.14.10)
src/blender_mcp/telemetry_decorator.py
28-28: Consider moving this statement to an else block
(TRY300)
36-36: Do not catch blind exception: Exception
(BLE001)
48-48: Consider moving this statement to an else block
(TRY300)
56-56: Do not catch blind exception: Exception
(BLE001)
src/blender_mcp/telemetry.py
47-48: try-except-pass detected, consider logging the exception
(S110)
47-47: Do not catch blind exception: Exception
(BLE001)
160-160: Consider moving this statement to an else block
(TRY300)
161-161: Do not catch blind exception: Exception
(BLE001)
224-224: Do not catch blind exception: Exception
(BLE001)
267-267: Local variable response is assigned to but never used
Remove assignment to unused variable response
(F841)
270-270: Do not catch blind exception: Exception
(BLE001)
314-314: Do not catch blind exception: Exception
(BLE001)
src/blender_mcp/server.py
184-184: Do not catch blind exception: Exception
(BLE001)
262-262: Unused function argument: ctx
(ARG001)
270-270: Do not catch blind exception: Exception
(BLE001)
271-271: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
271-271: Use explicit conversion flag
Replace with conversion flag
(RUF010)
272-272: Use explicit conversion flag
Replace with conversion flag
(RUF010)
295-295: Unused function argument: ctx
(ARG001)
318-318: Abstract raise to an inner function
(TRY301)
318-318: Create your own exception
(TRY002)
321-321: Abstract raise to an inner function
(TRY301)
321-321: Create your own exception
(TRY002)
321-321: Avoid specifying long messages outside the exception class
(TRY003)
332-332: Do not catch blind exception: Exception
(BLE001)
333-333: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
333-333: Use explicit conversion flag
Replace with conversion flag
(RUF010)
334-334: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
334-334: Create your own exception
(TRY002)
334-334: Avoid specifying long messages outside the exception class
(TRY003)
334-334: Use explicit conversion flag
Replace with conversion flag
(RUF010)
339-339: Unused function argument: ctx
(ARG001)
351-351: Do not catch blind exception: Exception
(BLE001)
352-352: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
352-352: Use explicit conversion flag
Replace with conversion flag
(RUF010)
353-353: Use explicit conversion flag
Replace with conversion flag
(RUF010)
564-564: Consider moving this statement to an else block
(TRY300)
565-565: Do not catch blind exception: Exception
(BLE001)
566-566: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
566-566: Use explicit conversion flag
Replace with conversion flag
(RUF010)
567-567: Use explicit conversion flag
Replace with conversion flag
(RUF010)
592-592: Unused function argument: ctx
(ARG001)
604-604: Consider moving this statement to an else block
(TRY300)
605-605: Do not catch blind exception: Exception
(BLE001)
606-606: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
606-606: Use explicit conversion flag
Replace with conversion flag
(RUF010)
607-607: Use explicit conversion flag
Replace with conversion flag
(RUF010)
612-612: Unused function argument: ctx
(ARG001)
614-614: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
679-679: Consider moving this statement to an else block
(TRY300)
680-680: Do not catch blind exception: Exception
(BLE001)
681-681: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
681-681: Use explicit conversion flag
Replace with conversion flag
(RUF010)
683-683: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
684-684: Use explicit conversion flag
Replace with conversion flag
(RUF010)
689-689: Unused function argument: ctx
(ARG001)
708-708: Abstract raise to an inner function
(TRY301)
708-708: Create your own exception
(TRY002)
708-708: Avoid specifying long messages outside the exception class
(TRY003)
711-711: Abstract raise to an inner function
(TRY301)
711-711: Create your own exception
(TRY002)
724-724: Do not catch blind exception: Exception
(BLE001)
725-725: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
725-725: Use explicit conversion flag
Replace with conversion flag
(RUF010)
726-726: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
726-726: Create your own exception
(TRY002)
726-726: Avoid specifying long messages outside the exception class
(TRY003)
726-726: Use explicit conversion flag
Replace with conversion flag
(RUF010)
731-731: Unused function argument: ctx
(ARG001)
775-775: f-string without any placeholders
Remove extraneous f prefix
(F541)
796-796: Do not catch blind exception: Exception
(BLE001)
797-797: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
797-797: Use explicit conversion flag
Replace with conversion flag
(RUF010)
799-799: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
800-800: Use explicit conversion flag
Replace with conversion flag
(RUF010)
984-984: Unused function argument: ctx
(ARG001)
995-995: Consider moving this statement to an else block
(TRY300)
996-996: Do not catch blind exception: Exception
(BLE001)
997-997: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
997-997: Use explicit conversion flag
Replace with conversion flag
(RUF010)
998-998: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1002-1002: Unused function argument: ctx
(ARG001)
1003-1003: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
1004-1004: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
1033-1033: Do not catch blind exception: Exception
(BLE001)
1034-1034: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1034-1034: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1035-1035: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1039-1039: Unused function argument: ctx
(ARG001)
1040-1040: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
1061-1061: Consider moving this statement to an else block
(TRY300)
1062-1062: Do not catch blind exception: Exception
(BLE001)
1063-1063: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1063-1063: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1064-1064: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1068-1068: Unused function argument: ctx
(ARG001)
1089-1089: Consider moving this statement to an else block
(TRY300)
1090-1090: Do not catch blind exception: Exception
(BLE001)
1091-1091: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1091-1091: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1092-1092: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1097-1097: Unused function argument: ctx
(ARG001)
1183-1183: Consider moving this statement to an else block
(TRY300)
1185-1185: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1186-1186: Do not catch blind exception: Exception
(BLE001)
1187-1187: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1187-1187: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1188-1188: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1193-1193: Unused function argument: ctx
(ARG001)
1197-1197: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
1288-1288: Do not catch blind exception: Exception
(BLE001)
1289-1289: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1289-1289: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1290-1290: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1294-1294: Unused function argument: ctx
(ARG001)
1302-1302: Local variable enabled is assigned to but never used
Remove assignment to unused variable enabled
(F841)
1305-1305: Consider moving this statement to an else block
(TRY300)
1306-1306: Do not catch blind exception: Exception
(BLE001)
1307-1307: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1307-1307: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1308-1308: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (14)
pyproject.toml (2)
3-3: LGTM: Version bump aligns with feature scope.The version bump from 1.1.3 to 1.4.0 appropriately reflects the significant feature additions (geometry nodes, asset integrations, telemetry system).
18-19: Versions exist; verify supabase transitive dependencies for security.Confirmed: supabase 2.0.0 (Oct 2023) and tomli 2.0.0 (Dec 2021) are both available on PyPI. tomli has no known CVEs. However, supabase has transitive dependency vulnerabilities (e.g., CVE-2025-27516 affecting Jinja2 via supabase-pydantic). Before merging, scan supabase's transitive dependencies (httpx, Jinja2, etc.) for known vulnerabilities to ensure the telemetry backend integration doesn't introduce security gaps.
.gitignore (1)
11-16: LGTM: Appropriate ignore patterns added.Adding
.DS_Store(macOS metadata) andsrc/blender_mcp/config.py(local secrets/config) to the ignore list follows best practices for keeping the repository clean and preventing accidental commits of sensitive information.TERMS_AND_CONDITIONS.md (2)
15-59: LGTM: Clear and transparent data collection disclosure.The terms clearly specify what data is collected (prompts, code, scene metadata) and what isn't (screenshots, models, personal files). The disclosure that data may be used for AI training and released as public datasets is appropriately transparent.
103-122: Users grant a perpetual, irrevocable license.Users should be aware that by using Blender MCP, they grant a "worldwide, royalty-free, perpetual license" to their prompts, generated code, and scene metadata. This is reasonable for an open-source project collecting training data, but it's a significant commitment that users should understand before agreeing.
Consider adding a more prominent consent checkbox or acceptance flow in the UI to ensure users actively acknowledge these terms rather than passively agreeing by using the software.
README.md (4)
7-7: LGTM: Important safety warning added.The warning about unofficial websites helps protect users from potential phishing or malicious sites impersonating this project.
87-97: LGTM: Environment variables properly documented.The addition of
BLENDER_HOSTandBLENDER_PORTenvironment variables with a Docker example helps users configure remote Blender connections.
118-156: LGTM: Comprehensive Cursor integration instructions.The instructions clearly differentiate between Mac and Windows setups, and explain the difference between global and project-specific MCP server configurations.
162-166: LGTM: VS Code integration added.The addition of Visual Studio Code integration with prerequisites and an install badge expands IDE support and improves accessibility.
src/blender_mcp/telemetry_decorator.py (3)
16-38: LGTM: Robust telemetry implementation for sync functions.The sync_wrapper correctly:
- Tracks execution time and success status
- Re-raises exceptions after recording error messages
- Ensures telemetry recording doesn't disrupt the wrapped function (debug log on failure)
The broad exception catching (Ruff BLE001) is intentional to ensure telemetry failures never impact normal operation.
39-58: LGTM: Async wrapper mirrors sync implementation.The async_wrapper correctly handles asynchronous functions with the same robust error handling and telemetry recording patterns as the sync version.
59-65: LGTM: Proper sync/async detection.The decorator correctly uses
inspect.iscoroutinefunctionto automatically select the appropriate wrapper type, supporting both synchronous and asynchronous tool functions.src/blender_mcp/server.py (2)
16-18: LGTM: Telemetry integration imports.The telemetry imports are well-structured, importing only the necessary functions from the telemetry module.
25-27: LGTM: Centralized default configuration.Good practice to define default host/port as named constants for maintainability.
| try: | ||
| blender = get_blender_connection() | ||
|
|
||
| # Create temp file path | ||
| temp_dir = tempfile.gettempdir() | ||
| temp_path = os.path.join(temp_dir, f"blender_screenshot_{os.getpid()}.png") | ||
|
|
||
| result = blender.send_command("get_viewport_screenshot", { | ||
| "max_size": max_size, | ||
| "filepath": temp_path, | ||
| "format": "png" | ||
| }) | ||
|
|
||
| if "error" in result: | ||
| raise Exception(result["error"]) | ||
|
|
||
| if not os.path.exists(temp_path): | ||
| raise Exception("Screenshot file was not created") | ||
|
|
||
| # Read the file | ||
| with open(temp_path, 'rb') as f: | ||
| image_bytes = f.read() | ||
|
|
||
| # Delete the temp file | ||
| os.remove(temp_path) |
There was a problem hiding this comment.
Temp file not cleaned up on error path.
If reading the file fails (line 324-325), the temp file remains on disk. Use a try/finally or context manager to ensure cleanup.
Proposed fix
if not os.path.exists(temp_path):
raise Exception("Screenshot file was not created")
- # Read the file
- with open(temp_path, 'rb') as f:
- image_bytes = f.read()
-
- # Delete the temp file
- os.remove(temp_path)
+ try:
+ # Read the file
+ with open(temp_path, 'rb') as f:
+ image_bytes = f.read()
+ finally:
+ # Delete the temp file
+ if os.path.exists(temp_path):
+ os.remove(temp_path)
return Image(data=image_bytes, format="png")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try: | |
| blender = get_blender_connection() | |
| # Create temp file path | |
| temp_dir = tempfile.gettempdir() | |
| temp_path = os.path.join(temp_dir, f"blender_screenshot_{os.getpid()}.png") | |
| result = blender.send_command("get_viewport_screenshot", { | |
| "max_size": max_size, | |
| "filepath": temp_path, | |
| "format": "png" | |
| }) | |
| if "error" in result: | |
| raise Exception(result["error"]) | |
| if not os.path.exists(temp_path): | |
| raise Exception("Screenshot file was not created") | |
| # Read the file | |
| with open(temp_path, 'rb') as f: | |
| image_bytes = f.read() | |
| # Delete the temp file | |
| os.remove(temp_path) | |
| try: | |
| blender = get_blender_connection() | |
| # Create temp file path | |
| temp_dir = tempfile.gettempdir() | |
| temp_path = os.path.join(temp_dir, f"blender_screenshot_{os.getpid()}.png") | |
| result = blender.send_command("get_viewport_screenshot", { | |
| "max_size": max_size, | |
| "filepath": temp_path, | |
| "format": "png" | |
| }) | |
| if "error" in result: | |
| raise Exception(result["error"]) | |
| if not os.path.exists(temp_path): | |
| raise Exception("Screenshot file was not created") | |
| try: | |
| # Read the file | |
| with open(temp_path, 'rb') as f: | |
| image_bytes = f.read() | |
| finally: | |
| # Delete the temp file | |
| if os.path.exists(temp_path): | |
| os.remove(temp_path) | |
| return Image(data=image_bytes, format="png") |
🧰 Tools
🪛 Ruff (0.14.10)
318-318: Abstract raise to an inner function
(TRY301)
318-318: Create your own exception
(TRY002)
321-321: Abstract raise to an inner function
(TRY301)
321-321: Create your own exception
(TRY002)
321-321: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In @src/blender_mcp/server.py around lines 304 - 328, The temp file created for
the Blender screenshot (temp_path) is not guaranteed to be removed if reading
the file or any later step raises; wrap the screenshot creation/reading/delete
sequence in a try/finally (or use a context manager surrounding the lifecycle)
so that after calling get_blender_connection() and
blender.send_command("get_viewport_screenshot", ...) you always attempt to
os.remove(temp_path) in the finally block if the file exists; ensure existing
error checks (result["error"] and existence of temp_path) remain and that
image_bytes is set from the with open(...) read inside the try before returning
or re-raising the exception.
| @telemetry_tool("download_sketchfab_model") | ||
| @mcp.tool() | ||
| def get_sketchfab_model_preview( | ||
| ctx: Context, | ||
| uid: str | ||
| ) -> Image: |
There was a problem hiding this comment.
Incorrect telemetry decorator name.
The @telemetry_tool("download_sketchfab_model") decorator is applied to get_sketchfab_model_preview, which is incorrect. The tool name should match the function.
Proposed fix
-@telemetry_tool("download_sketchfab_model")
+@telemetry_tool("get_sketchfab_model_preview")
@mcp.tool()
def get_sketchfab_model_preview(
ctx: Context,
uid: str
) -> Image:🧰 Tools
🪛 Ruff (0.14.10)
689-689: Unused function argument: ctx
(ARG001)
🤖 Prompt for AI Agents
In @src/blender_mcp/server.py around lines 686 - 691, The telemetry decorator on
get_sketchfab_model_preview is using the wrong tool name; update the
@telemetry_tool(...) argument to match the function name (e.g., change
"download_sketchfab_model" to "get_sketchfab_model_preview") so the telemetry
identifier aligns with the get_sketchfab_model_preview tool and decorator.
| @mcp.tool() | ||
| def download_sketchfab_model( | ||
| ctx: Context, | ||
| uid: str, | ||
| target_size: float | ||
| ) -> str: |
There was a problem hiding this comment.
Missing telemetry decorator on download_sketchfab_model.
This tool lacks the @telemetry_tool decorator, unlike other similar tools, causing inconsistent telemetry tracking.
Proposed fix
+@telemetry_tool("download_sketchfab_model")
@mcp.tool()
def download_sketchfab_model(
ctx: Context,
uid: str,
target_size: float
) -> str:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @mcp.tool() | |
| def download_sketchfab_model( | |
| ctx: Context, | |
| uid: str, | |
| target_size: float | |
| ) -> str: | |
| @telemetry_tool("download_sketchfab_model") | |
| @mcp.tool() | |
| def download_sketchfab_model( | |
| ctx: Context, | |
| uid: str, | |
| target_size: float | |
| ) -> str: |
🧰 Tools
🪛 Ruff (0.14.10)
731-731: Unused function argument: ctx
(ARG001)
🤖 Prompt for AI Agents
In @src/blender_mcp/server.py around lines 729 - 734, The
download_sketchfab_model tool is missing the @telemetry_tool decorator, causing
inconsistent telemetry; add the @telemetry_tool decorator directly above the
existing @mcp.tool() on the download_sketchfab_model function and ensure
telemetry_tool is imported/available in the module (matching how other tools are
decorated) so telemetry events are recorded consistently for
download_sketchfab_model.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @addon.py:
- Around line 2849-2859: In _get_socket_default_value, replace the bare
"except:" with a specific exception handler (use "except Exception:") so only
regular exceptions from accessing or converting socket.default_value are caught;
update the except clause in the try/except block that handles
hasattr(socket.default_value, '__len__') and list(...) conversion to use "except
Exception:" and keep returning None on error.
- Around line 2894-2927: Replace the bare except with "except Exception as e:"
in _verify_node_identifier and log the exception (using
self.logger.exception(...) if a logger attribute exists, or
logging.getLogger(__name__).exception(...)) including context like
node_type_name and the exception/traceback (traceback.format_exc()) so the
caught exception variable e is used for debugging; keep the finally block that
removes temp_tree to ensure cleanup.
🧹 Nitpick comments (4)
addon.py (4)
1218-1246: Consider returning consistent structure and improving error handling.The method returns different dictionary structures for success vs. error cases. For consistency with other status methods in the codebase, consider standardizing the response format.
Additionally, the broad exception catch on line 1239 could mask unexpected errors. Consider catching more specific exceptions or at least logging the full traceback.
♻️ Proposed improvements
def get_geometry_nodes_status(self): """Get the status of the geometry nodes feature in Blender. Returns: dict: A dictionary containing information about the geometry nodes feature status. """ try: # Get the geometry nodes setting from Blender preferences enabled = bpy.context.scene.blendermcp_use_geometry_nodes if enabled: return { - "status": "success", "enabled": True, "message": "Geometry nodes feature is enabled." } else: return { - "status": "warning", "enabled": False, "message": "Geometry nodes feature is disabled. Enable it in the BlenderMCP panel to use geometry node commands." } - except Exception as e: + except Exception as e: + import traceback + traceback.print_exc() # Handle errors when checking status return { - "status": "error", "enabled": False, "message": f"Error checking geometry nodes status: {str(e)}" }
2433-2459: Well-structured delegation pattern with room for improved error handling.The method appropriately delegates to helper functions, making it maintainable. However, consider:
- The broad
Exceptioncatch on line 2458 could hide unexpected errors- Add more context to error messages for debugging
💡 Optional enhancement
if input_sockets: self._set_modifier_parameters(geometry_modifier, node_group, input_sockets) return { "status": "success", "object_name": object_name, "modifier_name": geometry_modifier.name, "node_group_name": node_group.name, "nodes": created_nodes, "links": created_links } except Exception as e: + import traceback + traceback.print_exc() return {"error": f"Error completing geometry node network: {str(e)}"}
2615-2635: Silent error handling may hide issues with parameter setting.The method silently catches and prints exceptions when setting modifier parameters (lines 2632, 2634). This could make debugging difficult if parameters fail to set correctly. Consider at least collecting these errors and returning them in the response, or logging them more prominently.
💡 Enhanced error reporting
def _set_modifier_parameters(self, geometry_modifier, node_group, input_sockets): """Set modifier parameters from input socket values""" + errors = [] try: group_input_node = next((node for node in node_group.nodes if node.type == 'GROUP_INPUT'), None) if not group_input_node: group_input_node = node_group.nodes.new('NodeGroupInput') socket_dict = {output.name: i for i, output in enumerate(group_input_node.outputs)} for socket in input_sockets: if "value" in socket and socket.get("type") != "NodeSocketGeometry": socket_name = socket.get("name") if socket_name in socket_dict: socket_key = f"Socket_{socket_dict[socket_name]}" try: if socket_key in geometry_modifier: geometry_modifier[socket_key] = socket["value"] except Exception as e: - print(f"Error setting modifier parameter {socket_key}: {e}") + error_msg = f"Error setting modifier parameter {socket_key}: {e}" + print(error_msg) + errors.append(error_msg) except Exception as e: - print(f"Error setting modifier parameters: {e}") + error_msg = f"Error setting modifier parameters: {e}" + print(error_msg) + errors.append(error_msg) + return errors if errors else NoneThen in
complete_geometry_node, you could include parameter errors in the response:param_errors = self._set_modifier_parameters(geometry_modifier, node_group, input_sockets) result = { "status": "success", ... } if param_errors: result["parameter_warnings"] = param_errors return result
2831-2846: Variable shadowing: rename loop variable to avoid confusion.Line 2836 uses
socketas a loop variable, which shadows thesocketmodule imported at line 8. While this works because the import is in a different scope, it could cause confusion. Consider using a more specific name likesocket_itemorsock.def _collect_socket_info(self, node, is_input: bool = True) -> List[SocketInfo]: """Collect node socket information""" socket_list = node.inputs if is_input else node.outputs sockets = [] - for socket in socket_list: + for sock in socket_list: - default_value = self._get_socket_default_value(socket) + default_value = self._get_socket_default_value(sock) sockets.append(SocketInfo( - name=socket.name, + name=sock.name, - type=socket.type, + type=sock.type, - description=socket.description, + description=sock.description, - enabled=socket.enabled, + enabled=sock.enabled, - hide=socket.hide, + hide=sock.hide, default_value=default_value )) return sockets
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
addon.py
🧰 Additional context used
🧬 Code graph analysis (1)
addon.py (1)
src/blender_mcp/server.py (3)
get_geometry_nodes_status(1294-1308)get_node_info(1096-1188)complete_geometry_node(1192-1290)
🪛 Ruff (0.14.10)
addon.py
1239-1239: Do not catch blind exception: Exception
(BLE001)
1244-1244: Use explicit conversion flag
Replace with conversion flag
(RUF010)
2450-2457: Consider moving this statement to an else block
(TRY300)
2458-2458: Do not catch blind exception: Exception
(BLE001)
2459-2459: Use explicit conversion flag
Replace with conversion flag
(RUF010)
2531-2531: Do not catch blind exception: Exception
(BLE001)
2532-2532: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2532-2532: Create your own exception
(TRY002)
2532-2532: Avoid specifying long messages outside the exception class
(TRY003)
2532-2532: Use explicit conversion flag
Replace with conversion flag
(RUF010)
2542-2542: Do not catch blind exception: Exception
(BLE001)
2557-2557: Do not catch blind exception: Exception
(BLE001)
2590-2590: Abstract raise to an inner function
(TRY301)
2590-2590: Create your own exception
(TRY002)
2590-2590: Avoid specifying long messages outside the exception class
(TRY003)
2592-2592: Abstract raise to an inner function
(TRY301)
2592-2592: Create your own exception
(TRY002)
2592-2592: Avoid specifying long messages outside the exception class
(TRY003)
2598-2598: Abstract raise to an inner function
(TRY301)
2598-2598: Create your own exception
(TRY002)
2598-2598: Avoid specifying long messages outside the exception class
(TRY003)
2600-2600: Abstract raise to an inner function
(TRY301)
2600-2600: Create your own exception
(TRY002)
2600-2600: Avoid specifying long messages outside the exception class
(TRY003)
2610-2610: Do not catch blind exception: Exception
(BLE001)
2611-2611: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
2611-2611: Create your own exception
(TRY002)
2611-2611: Avoid specifying long messages outside the exception class
(TRY003)
2611-2611: Use explicit conversion flag
Replace with conversion flag
(RUF010)
2632-2632: Do not catch blind exception: Exception
(BLE001)
2634-2634: Do not catch blind exception: Exception
(BLE001)
2690-2695: Consider moving this statement to an else block
(TRY300)
2696-2696: Do not catch blind exception: Exception
(BLE001)
2697-2697: Use explicit conversion flag
Replace with conversion flag
(RUF010)
2769-2769: Abstract raise to an inner function
(TRY301)
2769-2769: Avoid specifying long messages outside the exception class
(TRY003)
2783-2783: Do not catch blind exception: Exception
(BLE001)
2784-2784: Use explicit conversion flag
Replace with conversion flag
(RUF010)
2787-2787: Use explicit conversion flag
Replace with conversion flag
(RUF010)
2804-2804: Avoid specifying long messages outside the exception class
(TRY003)
2807-2807: Prefer TypeError exception for invalid type
(TRY004)
2807-2807: Avoid specifying long messages outside the exception class
(TRY003)
2836-2836: Import socket from line 8 shadowed by loop variable
(F402)
2858-2858: Do not use bare except
(E722)
2922-2922: Consider moving this statement to an else block
(TRY300)
2923-2923: Do not catch blind exception: Exception
(BLE001)
2923-2923: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
2981-2981: f-string without any placeholders
Remove extraneous f prefix
(F541)
3014-3014: Consider moving this statement to an else block
(TRY300)
3015-3015: Do not catch blind exception: Exception
(BLE001)
3019-3019: f-string without any placeholders
Remove extraneous f prefix
(F541)
3140-3140: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (2)
addon.py (2)
2510-2534: LGTM: Clear node creation logic with proper error context.The method properly wraps each node creation in a try-except block, providing specific error messages that include the node type. This is good defensive programming that will help with debugging.
3411-3415: LGTM: UI integration follows established patterns.The geometry nodes toggle is properly integrated into the scene properties and follows the same pattern as other feature flags (Polyhaven, Sketchfab, etc.). The property is correctly registered in
register()and cleaned up inunregister().
| def _register_node_info_cache(self): | ||
| """Register custom properties""" | ||
| if not hasattr(bpy.types.WindowManager, "node_info_cache"): | ||
| bpy.types.WindowManager.node_info_cache = bpy.props.StringProperty( | ||
| name="Node Info Cache", | ||
| description="Cache for node information", | ||
| default="" | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Cache mechanism lacks invalidation strategy.
The cache is stored in WindowManager.node_info_cache and persists across sessions, but there's no mechanism to invalidate it when:
- Blender version changes
- Addons are loaded/unloaded (which may add new node types)
- The geometry node system is updated
Consider adding version checking or a cache key that includes relevant version information.
🔧 Add cache versioning
def _register_node_info_cache(self):
"""Register custom properties"""
if not hasattr(bpy.types.WindowManager, "node_info_cache"):
bpy.types.WindowManager.node_info_cache = bpy.props.StringProperty(
name="Node Info Cache",
description="Cache for node information",
default=""
)
+ bpy.types.WindowManager.node_info_cache_version = bpy.props.StringProperty(
+ name="Node Info Cache Version",
+ description="Blender version for cached node info",
+ default=""
+ )Then in _get_nodes_from_cache_or_collect:
def _get_nodes_from_cache_or_collect(self) -> List[NodeInfo]:
"""Get node information from cache or collect new ones"""
wm = bpy.context.window_manager
+ current_version = f"{bpy.app.version[0]}.{bpy.app.version[1]}.{bpy.app.version[2]}"
# Try to get data from cache
- if wm.node_info_cache:
+ if wm.node_info_cache and wm.node_info_cache_version == current_version:
print(f"Using cache..")
...
# After collecting:
self._update_cache(node_infos)
+ wm.node_info_cache_version = current_version| def _get_socket_default_value(self, socket): | ||
| """Get socket default value safely""" | ||
| if not hasattr(socket, 'default_value'): | ||
| return None | ||
| try: | ||
| if hasattr(socket.default_value, '__len__'): | ||
| return list(socket.default_value) | ||
| else: | ||
| return socket.default_value | ||
| except: | ||
| return None |
There was a problem hiding this comment.
Replace bare except with specific exception type.
Line 2858 uses a bare except: clause which catches all exceptions including SystemExit and KeyboardInterrupt. This is generally considered bad practice. Use except Exception: instead to catch only regular exceptions.
def _get_socket_default_value(self, socket):
"""Get socket default value safely"""
if not hasattr(socket, 'default_value'):
return None
try:
if hasattr(socket.default_value, '__len__'):
return list(socket.default_value)
else:
return socket.default_value
- except:
+ except Exception:
return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _get_socket_default_value(self, socket): | |
| """Get socket default value safely""" | |
| if not hasattr(socket, 'default_value'): | |
| return None | |
| try: | |
| if hasattr(socket.default_value, '__len__'): | |
| return list(socket.default_value) | |
| else: | |
| return socket.default_value | |
| except: | |
| return None | |
| def _get_socket_default_value(self, socket): | |
| """Get socket default value safely""" | |
| if not hasattr(socket, 'default_value'): | |
| return None | |
| try: | |
| if hasattr(socket.default_value, '__len__'): | |
| return list(socket.default_value) | |
| else: | |
| return socket.default_value | |
| except Exception: | |
| return None |
🧰 Tools
🪛 Ruff (0.14.10)
2858-2858: Do not use bare except
(E722)
🤖 Prompt for AI Agents
In @addon.py around lines 2849 - 2859, In _get_socket_default_value, replace the
bare "except:" with a specific exception handler (use "except Exception:") so
only regular exceptions from accessing or converting socket.default_value are
caught; update the except clause in the try/except block that handles
hasattr(socket.default_value, '__len__') and list(...) conversion to use "except
Exception:" and keep returning None on error.
| def _verify_node_identifier(self, node_type_name: str) -> Tuple[ | ||
| bool, List[SocketInfo], List[SocketInfo], List[PropertyInfo]]: | ||
| """Verify node identifier is valid for creating a node and get node information | ||
|
|
||
| Args: | ||
| node_type_name: Node type name | ||
|
|
||
| Returns: | ||
| Tuple: (is_usable, input_sockets, output_sockets, property_info) | ||
| """ | ||
| # Create a temporary geometry node tree for testing | ||
| temp_tree = bpy.data.node_groups.new('TempNodeTree', 'GeometryNodeTree') | ||
| inputs = [] | ||
| outputs = [] | ||
| properties = [] | ||
|
|
||
| try: | ||
| # Try to create a node | ||
| node = temp_tree.nodes.new(node_type_name) | ||
| node_type = getattr(bpy.types, node_type_name) | ||
|
|
||
| # Get socket information | ||
| inputs = self._collect_socket_info(node, is_input=True) | ||
| outputs = self._collect_socket_info(node, is_input=False) | ||
|
|
||
| # Get property information | ||
| properties = self._collect_property_info(node_type) | ||
|
|
||
| return True, inputs, outputs, properties | ||
| except Exception as e: | ||
| return False, inputs, outputs, properties | ||
| finally: | ||
| # Clean up | ||
| bpy.data.node_groups.remove(temp_tree) |
There was a problem hiding this comment.
Replace bare except with specific exception type and improve error handling.
Lines 2923-2924 use a bare except: clause and assign to an unused variable. Replace with except Exception as e: and consider logging the error for debugging purposes.
# Get property information
properties = self._collect_property_info(node_type)
return True, inputs, outputs, properties
- except Exception as e:
+ except Exception:
+ # Optionally log for debugging
+ # print(f"Cannot create node {node_type_name}: {e}")
return False, inputs, outputs, properties
finally:
# Clean up
bpy.data.node_groups.remove(temp_tree)🧰 Tools
🪛 Ruff (0.14.10)
2922-2922: Consider moving this statement to an else block
(TRY300)
2923-2923: Do not catch blind exception: Exception
(BLE001)
2923-2923: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
🤖 Prompt for AI Agents
In @addon.py around lines 2894 - 2927, Replace the bare except with "except
Exception as e:" in _verify_node_identifier and log the exception (using
self.logger.exception(...) if a logger attribute exists, or
logging.getLogger(__name__).exception(...)) including context like
node_type_name and the exception/traceback (traceback.format_exc()) so the
caught exception variable e is used for debugging; keep the finally block that
removes temp_tree to ensure cleanup.
|
@ahujasid Here is a test example |
|
@duqich I think using a tool makes it quite prescriptive, what do you feel about repackaging it as a skill and simplifying it? https://platform.claude.com/docs/en/agents-and-tools/agent-skills/overview |
@ahujasid If we make it use Claude Skills won't that only allow us to use Anthropic's Claude? I have not tried it but I think some people may be experimenting and using other LLMs? Maybe make using Claude Skills optional? 🤔 |
|
Most agent harnesses now support skills. A skill that works with Claude Code should also work with OpenCode, Hermes. The frustrating part is the location(s) of installed skills. What a mess. Also, are you aware of https://www.blender.org/lab/mcp-server/ ? (I just learned about it today so I'm not sure how it compares.) |
User description
Hello,
Over the past few weeks, I’ve been very excited about this project but also concerned that 3D models generated by LLMs through code are not easily controllable or adjustable.
To address this, I’m adding a module that enables procedural 3D model creation using Blender’s Geometry Nodes. With this module, models can expose adjustable parameters, making it much easier for users to fine-tune and accelerate the creation of their own 3D asset libraries.
After some basic tests described in the README, I believe it’s now ready for submission. (By the way, I used Cursor + Claude to test my examples.)
PR Type
Enhancement, Documentation
Description
Introduced a Geometry Node Module for procedural 3D modeling.
Added support for creating and managing geometry node networks.
Enhanced Blender MCP server with geometry node-related commands.
Updated documentation with examples for the new Geometry Node Module.
Changes walkthrough 📝
addon.py
Introduced Geometry Node Module and UI integration.addon.py
server.py
Added geometry node commands to MCP server.src/blender_mcp/server.py
README.md
Updated documentation for Geometry Node Module.README.md
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.