feat: Add OBJ import and export functionality to Blender MCP server #29
Conversation
- Implemented `import_obj` and `export_obj` methods in the Blender addon for importing and exporting OBJ files directly. - Updated the Blender MCP server to register new tools for handling OBJ file operations. - Enhanced error handling and logging for better debugging during import/export processes. - Validated OBJ content format before processing to ensure successful imports.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce OBJ file import and export capabilities to the Blender MCP server and its client interface. New command handlers and async tool methods are added for transferring OBJ data. An example script demonstrates the workflow, and comprehensive unit tests verify correct functionality and error handling for OBJ transfer operations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPServer
participant Blender
Client->>MCPServer: import_obj_file(obj_content, object_name)
MCPServer->>Blender: import_obj(obj_content, object_name)
Blender-->>MCPServer: {import result JSON}
MCPServer-->>Client: {import result JSON}
Client->>MCPServer: export_obj_file(object_name)
MCPServer->>Blender: export_obj(object_name)
Blender-->>MCPServer: {export result JSON with OBJ content}
MCPServer-->>Client: {export result JSON with OBJ content}
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
src/lightfast_mcp/servers/blender/server.py (2)
157-175: 🛠️ Refactor suggestionOverwriting
self.info.toolsloses tools registered in super-classes
self.info.tools = [...]replaces any tools a parent class might already have appended.
Safer alternative:-self.info.tools = [ - "get_state", - "execute_command", - "import_obj_file", - "export_obj_file", -] +self.info.tools.extend( + [ + "get_state", + "execute_command", + "import_obj_file", + "export_obj_file", + ] +)This preserves inherited registrations and prevents accidental loss when multiple mix-ins add tools.
64-88:⚠️ Potential issue
send_commandis not thread-safe – concurrent requests may interleave on the same socket
import_obj_fileandexport_obj_filedelegate torun_in_executor, spawning threads that callsend_command()simultaneously.
Becausesend_commandwrites to a shared socket without a lock, two threads can interleavesock.sendall()calls, resulting in corrupted JSON and mismatched responses.Minimal fix:
class BlenderConnection: def __init__(...): ... + self._write_lock = threading.Lock() def send_command(...): ... - self.sock.sendall(json.dumps(command).encode("utf-8")) + with self._write_lock: + self.sock.sendall(json.dumps(command).encode("utf-8"))You may also need a read-lock around
_receive_response()if Blender ever sends unsolicited messages.
🧹 Nitpick comments (7)
examples/obj_transfer_example.py (1)
48-66: Uselogginginstead ofThe example relies exclusively on
print()statements for status updates. This:
- makes it hard to integrate with existing log pipelines,
- cannot be filtered/structured by log-level, and
- is inconsistent with the rest of the project where
logging_utils.get_logger()is used.Consider switching to the project’s logger:
-import asyncio -import json +import asyncio +import json +from lightfast_mcp.utils.logging_utils import get_logger -# … -print("🔌 Connecting to Blender MCP server…") +logger = get_logger(__name__) + +logger.info("🔌 Connecting to Blender MCP server…")You can still emit the emojis if you want (
logger.info("✅ Connected …")).tests/unit/test_blender_obj_transfer.py (1)
78-83: Avoid repetitiveimport jsonstatements inside each test
jsonis imported eight times across individual test scopes. Import it once at module level to keep tests concise and speed up import time:-import json +import json # top-of-file, below other importsWhile harmless, it eliminates duplication and follows DRY.
src/lightfast_mcp/servers/blender/server.py (3)
293-304: Useasyncio.get_running_loop()inside coroutines
asyncio.get_event_loop()is deprecated in >3.10 for code running inside a coroutine.
Switching preventsDeprecationWarningand future-proofs the server:-loop = asyncio.get_event_loop() +loop = asyncio.get_running_loop()Apply to
execute_command,import_obj_file, andexport_obj_file.
337-363: OBJ validation misses lines with leading whitespace
line.startswith(("v ", "f ", "vn ", "vt "))fails when OBJ lines are indented or start with tabs (valid but uncommon). A minimal fix is to strip leading whitespace:-if not any( - line.startswith(("v ", "f ", "vn ", "vt ")) - for line in obj_content.split("\n") -): +if not any( + line.lstrip().startswith(("v ", "f ", "vn ", "vt ")) + for line in obj_content.splitlines() +):Alternatively consider using a small regex, but the above keeps it simple.
404-414: Guard against invalidobject_nametypes
export_obj_fileblindly serialisesobject_nameinto JSON. If a caller mistakenly passes a non-serialisable type (e.g.,dict),json.dumpswill raise deep insidesend_command, returning a generic error.Add an early check:
if object_name is not None and not isinstance(object_name, str): return json.dumps( {"error": "object_name must be str or None", "server_name": self.config.name}, indent=2, )This provides a clear, deterministic error for the client.
addons/blender/lightfast_blender_addon.py (2)
283-285: Registering new handlers looks good – consider documenting them.
import_objandexport_objare correctly wired into the dispatcher.
For completeness, please remember to update any public docs / README that enumerate supported MCP commands so client developers can discover the new capabilities.
465-483: Export modifies the user’s current selection and never restores it.Calling
object.select_all/select_setinside a headless server task silently trashes the artist’s current selection, which can be disruptive when the addon runs in an interactive session.A lightweight fix is to snapshot & restore selection:
- if object_name: + # Preserve current selection to restore later + prev_selection = {obj.name for obj in bpy.context.selected_objects} + + if object_name: ... - else: + else: ... - export_objects = selected_objects + export_objects = selected_objects + + # … perform export … + + # Restore original selection + bpy.ops.object.select_all(action="DESELECT") + for name in prev_selection: + obj = bpy.data.objects.get(name) + if obj: + obj.select_set(True)This keeps the operator side-effect-free from a user-perspective.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
addons/blender/lightfast_blender_addon.py(2 hunks)examples/obj_transfer_example.py(1 hunks)src/lightfast_mcp/servers/blender/server.py(3 hunks)tests/unit/test_blender_obj_transfer.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/lightfast_mcp/servers/blender/server.py (3)
src/tools/common/logging.py (2)
info(104-106)error(112-126)addons/blender/lightfast_blender_addon.py (1)
execute_command(266-273)src/lightfast_mcp/exceptions.py (1)
BlenderMCPError(22-25)
🪛 Ruff (0.11.9)
addons/blender/lightfast_blender_addon.py
452-452: Do not use bare except
(E722)
519-519: Do not use bare except
(E722)
🪛 GitHub Actions: CI/CD Pipeline
addons/blender/lightfast_blender_addon.py
[error] 452-452: Ruff: Do not use bare except (E722)
[error] 519-519: Ruff: Do not use bare except (E722)
🔇 Additional comments (1)
tests/unit/test_blender_obj_transfer.py (1)
120-138:patch.objecton an instance method can leak between async testsBecause
blender_serveris a module-scoped fixture, the same server instance is shared across tests.
patch.object(blender_server.blender_connection, "send_command")therefore mutates a shared object and can accidentally bleed side-effects into subsequent tests (e.g., call-count assertions).Recommendation:
@pytest.mark.asyncio async def test_export_obj_specific_object(sample_obj_content): server = make_isolated_server() # new helper that returns a *fresh* instance # … patch server.blender_connection.send_command as beforeor change the fixture to
@pytest.fixture()(function scope) instead of re-using the same object.
| if import_result: | ||
| result_data = json.loads(import_result[0].text) | ||
| print(f"✅ Import result: {result_data}") | ||
| else: | ||
| print("❌ Import failed - no result returned") | ||
| return |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Result handling is brittle – don’t assume call_tool always returns a [Response] list
import_result[0].text assumes:
call_tool()returns a non-empty list- The first element has a
.textattribute
If call_tool() is updated to return a plain string / dict (or an empty list on error), json.loads() will raise.
A safer pattern:
if not import_result:
logger.error("No import result returned")
return
if isinstance(import_result, list):
payload = import_result[0].text if hasattr(import_result[0], "text") else import_result[0]
else:
payload = import_result
try:
result_data = json.loads(payload)
except (TypeError, json.JSONDecodeError) as exc:
logger.error("Malformed response from server", exc_info=exc)
return🤖 Prompt for AI Agents
In examples/obj_transfer_example.py around lines 78 to 83, the code assumes
call_tool() returns a non-empty list with elements having a .text attribute,
which is brittle and can cause json.loads() to fail. To fix this, first check if
import_result is empty and return early if so. Then, determine if import_result
is a list; if yes, extract the payload from the first element's .text attribute
if it exists, otherwise use the element directly. If import_result is not a
list, use it as the payload. Finally, wrap json.loads(payload) in a try-except
block to catch TypeError and json.JSONDecodeError, logging an error and
returning if parsing fails.
| try: | ||
| os.unlink(temp_file_path) | ||
| except: | ||
| pass # Ignore cleanup errors |
There was a problem hiding this comment.
Second bare except: – mirror the scoped fix above.
Same rationale as the earlier cleanup block; please specialise the exception or at least log the error.
- except:
- pass # Ignore cleanup errors
+ except FileNotFoundError:
+ pass
+ except OSError as cleanup_err:
+ print(f"[export_obj] Temp-file cleanup failed: {cleanup_err}")📝 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: | |
| os.unlink(temp_file_path) | |
| except: | |
| pass # Ignore cleanup errors | |
| try: | |
| os.unlink(temp_file_path) | |
| except FileNotFoundError: | |
| pass | |
| except OSError as cleanup_err: | |
| print(f"[export_obj] Temp-file cleanup failed: {cleanup_err}") |
🧰 Tools
🪛 Ruff (0.11.9)
519-519: Do not use bare except
(E722)
🪛 GitHub Actions: CI/CD Pipeline
[error] 519-519: Ruff: Do not use bare except (E722)
🤖 Prompt for AI Agents
In addons/blender/lightfast_blender_addon.py around lines 517 to 520, the bare
except clause used to ignore errors during file cleanup should be replaced with
a more specific exception handler, such as catching OSError. Additionally, log
the exception details instead of silently passing to aid in debugging potential
issues during cleanup.
| # Clean up temporary file | ||
| try: | ||
| os.unlink(temp_file_path) | ||
| except: | ||
| pass # Ignore cleanup errors | ||
|
|
||
| except Exception as e: |
There was a problem hiding this comment.
Bare except: hides real errors and is raising CI warnings.
Catching everything (including KeyboardInterrupt, SystemExit) makes debugging harder and violates PEP 8 / Ruff rule E722.
- except:
- pass # Ignore cleanup errors
+ except FileNotFoundError:
+ # File already deleted – fine.
+ pass
+ except OSError as cleanup_err:
+ print(f"[import_obj] Temp-file cleanup failed: {cleanup_err}")Repeat this pattern in other cleanup blocks to appease static analysis and keep error logs meaningful.
📝 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.
| finally: | |
| # Clean up temporary file | |
| try: | |
| os.unlink(temp_file_path) | |
| except: | |
| pass # Ignore cleanup errors | |
| finally: | |
| # Clean up temporary file | |
| try: | |
| os.unlink(temp_file_path) | |
| except FileNotFoundError: | |
| # File already deleted – fine. | |
| pass | |
| except OSError as cleanup_err: | |
| print(f"[import_obj] Temp-file cleanup failed: {cleanup_err}") |
🧰 Tools
🪛 Ruff (0.11.9)
452-452: Do not use bare except
(E722)
🪛 GitHub Actions: CI/CD Pipeline
[error] 452-452: Ruff: Do not use bare except (E722)
🤖 Prompt for AI Agents
In addons/blender/lightfast_blender_addon.py around lines 448 to 454, replace
the bare except clause in the cleanup block with a more specific exception
handler, such as catching only OSError or Exception, to avoid hiding critical
errors like KeyboardInterrupt or SystemExit. Apply this change consistently to
other similar cleanup blocks to comply with PEP 8 and reduce CI warnings.
| raise Exception("No objects were imported from OBJ content") | ||
|
|
||
| # Get the newly imported objects | ||
| imported_objects = [] | ||
| for obj in bpy.context.selected_objects: | ||
| if obj.name not in [ | ||
| o.name for o in bpy.data.objects[:initial_object_count] | ||
| ]: | ||
| imported_objects.append(obj.name) | ||
|
|
||
| # If a specific name was requested and only one object was imported, rename it | ||
| if len(imported_objects) == 1 and object_name != "ImportedObject": |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Imported-object detection may miss objects that the importer leaves un-selected.
bpy.ops.wm.obj_import does not guarantee that every newly-created object is left selected.
Relying on bpy.context.selected_objects can therefore under-report (or mis-report after a prior user selection) and make the object_names list inconsistent with object_count.
A robust way is to diff the object list before/after the import:
- imported_objects = []
- for obj in bpy.context.selected_objects:
- if obj.name not in [
- o.name for o in bpy.data.objects[:initial_object_count]
- ]:
- imported_objects.append(obj.name)
+ # Identify newly created objects independently of selection state
+ imported_objects = [
+ obj.name
+ for obj in bpy.data.objects[initial_object_count:]
+ ]This guarantees len(imported_objects) == imported_count and avoids selection-state edge cases.
📝 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.
| if imported_count == 0: | |
| raise Exception("No objects were imported from OBJ content") | |
| # Get the newly imported objects | |
| imported_objects = [] | |
| for obj in bpy.context.selected_objects: | |
| if obj.name not in [ | |
| o.name for o in bpy.data.objects[:initial_object_count] | |
| ]: | |
| imported_objects.append(obj.name) | |
| # If a specific name was requested and only one object was imported, rename it | |
| if imported_count == 0: | |
| raise Exception("No objects were imported from OBJ content") | |
| # Get the newly imported objects | |
| + # Identify newly created objects independently of selection state | |
| + imported_objects = [ | |
| + obj.name | |
| + for obj in bpy.data.objects[initial_object_count:] | |
| + ] | |
| # If a specific name was requested and only one object was imported, rename it |
🤖 Prompt for AI Agents
In addons/blender/lightfast_blender_addon.py around lines 424 to 435, the code
uses bpy.context.selected_objects to detect newly imported objects, which can
miss objects if they are not selected by the importer. To fix this, capture the
list of object names before the import, then after the import, compute the
difference between the new list and the old list to identify all newly imported
objects. Replace the selection-based detection with this diff approach to ensure
imported_objects matches imported_count accurately.
Summary by CodeRabbit
New Features
Documentation
Tests