Merged
Conversation
Major refactor of server architecture: - Extract ServerState, ViewerProtocol, qt_helpers, and output modules - Replace module-level globals with ServerState class - Add STANDALONE/AUTO_DETECT startup modes - Simplify CLI to single `install --target <name>` command - Update bridge_server to use new state management Test suite reorganization: - Delete duplicate test files (test_coverage.py, test_napari_server_coverage.py) - Rename test_edge_cases.py to test_qt_helpers.py (reflects actual content) - Create test_server_tools.py as canonical home for tool unit tests - Add tests for AUTO_DETECT proxy fallbacks, invalid inputs, concurrent access - Add multi-step workflow integration tests - Consolidate widget tests into test_widget.py - Fix SIM117 lint warnings in test_bridge_server.py - Remove raw napari API tests that don't test our code - Net: 286 tests, 84.15% coverage (0 duplicates, 13 new tests) CI and docs: - Bump --cov-fail-under from 70% to 80% - Update CONTRIBUTING.md test file listing - Update documentation references Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
API consolidation: - Merge add_image/add_labels/add_points into unified add_layer (all 7 types) - Merge set_camera/set_ndisplay/set_dims_current_step/set_grid/reset_view into configure_viewer - Absorb set_active_layer into set_layer_properties (active param) - Absorb timelapse_screenshot into screenshot (axis/slice_range params) - New tools: get_layer (metadata+data+slicing), apply_to_layers (batch ops), save_layer_data (npy/tiff/csv export) Validation and error handling: - Input validation for opacity (0-1), gamma (>0), zoom (>0), ndisplay (2-3) - contrast_limits must be exactly [min, max] (reject wrong lengths) - Invalid colormap/blending return error instead of crashing - configure_viewer validates before mutating (no partial state on error) - Dims values clamped to valid range with warning - add_layer rejects conflicting data sources (data + data_var + path) New shared helpers module (_helpers.py): - parse_bool, resolve_layer_type, build_layer_detail, create_layer_on_viewer - run_code (exec with stdout/stderr capture), build_truncated_response - Eliminates duplication between server.py and bridge_server.py Bridge server: - Exclude lifecycle tools (init_viewer, close_viewer) in bridge mode - Updated for add_layer API Tests (454 passing, 80.71% coverage): - test_server_tools.py rewritten: class-per-tool, ~100 tests - test_helpers.py: unit tests for shared helpers - test_widget.py: port change UI, server failure handling - test_bridge_server.py: lifecycle exclusion, add_layer via bridge - test_tools.py: tool completeness checks, README sync test Other: - Delete tox.ini (uv-based workflow) - Add _helpers.py to ruff per-file ignores - Update docs, CONTRIBUTING.md, codecov.yml Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Normalize line_limit from string to int with fallback to 30 in execute_code, install_packages, and build_truncated_response - Remove _parse_bool/_truncate_output backward-compat aliases from server.py; tests now import directly from _helpers/output modules - Add tests: numpy slicing edge cases, max_elements cap, line_limit normalization (string/invalid/None), save_layer_data format coverage (png, jpg, csv tracks, labels tiff), security package name validation (pip flag injection, URL specifiers, newline injection) 474 tests, 81.05% coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Server shutdown:
- Add ServerState.request_shutdown() — schedules delayed loop.stop(),
safe to call from any thread (Qt main thread on viewer destroy)
- Wire _on_destroyed signal and close_viewer to request_shutdown
- Capture event loop via FastMCP lifespan context manager
- Clean up shutdown state in conftest reset_server_state fixture
Bridge server fixes:
- Reject 'path' param for non-image/labels layer types
- Require 'data_var' for surface layers (no inline data support)
- Inject real napari module in exec namespace (was None)
Other:
- Fix cline_cursor install command hint (was missing 'install')
- Fix codecov threshold comment (said 10%, was actually 2%)
- Remove CHANGELOG.md from MANIFEST.in (doesn't exist)
- Rename CI step labels ("Test with tox" -> proper names)
- Make test_expected_tools_matches_server properly async
- Add benchmark marker to pyproject.toml
Tests (487 passing, 81.20% coverage):
- TestRequestShutdown: lifecycle, idempotency, closed loop, delayed stop
- TestBridgeAddLayerValidation: path rejection, surface, conflicts, unknown type
- TestBridgeExecNamespace: napari module injection, numpy availability
- test_close_viewer_requests_shutdown, test_on_destroyed_requests_shutdown
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMajor refactor introducing ServerState and StartupMode, consolidating bridge tools (add_layer, execute_code), new helper modules for layer management and code execution, Qt event-loop helpers, unified CLI installer with an explicit "install" subcommand and napari_backend handling, extensive docs/CI updates, and large test suite reorganization. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI Installer"
participant Factory as "Installer Factory / BaseInstaller"
participant Build as "build_server_config"
participant Server as "MCP Server (create_server)"
participant State as "ServerState"
participant GUI as "Qt Main Thread"
participant Tool as "Tool (add_layer / execute_code)"
CLI->>Factory: install(target, napari_backend)
Factory->>Build: build_server_config(..., napari_requirement)
Build->>Server: create_server(state)
Server->>State: initialize (mode, output storage, gui executor)
Server->>Tool: register tools (add_layer, execute_code)
Note over Tool,GUI: Tool execution runs on GUI main thread
Tool->>State: request gui_execute(operation)
State->>GUI: schedule operation on Qt main thread (with timeout)
GUI-->>Tool: operation result / timeout
Tool->>State: store_output(tool_name, stdout, stderr, result)
State-->>Tool: return output_id
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
|
Contributor
|
✅ Documentation builds successfully! The site will be updated when this PR is merged. |
Contributor
|
✅ Documentation builds successfully! The site will be updated when this PR is merged. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #36 +/- ##
==========================================
+ Coverage 78.23% 78.87% +0.63%
==========================================
Files 16 21 +5
Lines 1792 2248 +456
Branches 230 387 +157
==========================================
+ Hits 1402 1773 +371
- Misses 282 355 +73
- Partials 108 120 +12
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (18)
.github/workflows/release.yml (3)
67-68: Same condition improvement applies here.For consistency and precision, consider using
startsWith(github.ref, 'refs/tags/')instead ofcontains(github.ref, 'tags').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 67 - 68, Update the workflow conditional that currently uses contains(github.ref, 'tags') && !inputs.dry_run to use startsWith(github.ref, 'refs/tags/') && !inputs.dry_run for precise tag detection; locate the step using the softprops/action-gh-release@v2 entry and replace the condition so it only triggers on refs that actually start with refs/tags/.
85-89: Redundant directory listings in dry run notification.Lines 87 and 89 both list the
dist/directory with slightly different formats (-lavs-lh). The "Wheel contents" label on line 88 is also misleading sincels -lh dist/shows file metadata, not the contents of the wheel archive.Suggested fix
- name: Dry run notification if: inputs.dry_run run: | echo "🔍 Dry run complete - artifacts built but not published" echo "Built artifacts:" ls -la dist/ - echo "Wheel contents:" - ls -lh dist/Or if you want to actually show wheel contents:
- echo "Wheel contents:" - ls -lh dist/ + echo "Wheel contents:" + unzip -l dist/*.whl | head -50🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 85 - 89, Remove the redundant directory listing by keeping a single informative listing and correct the misleading "Wheel contents:" line: replace the duplicate 'ls -lh dist/' (or the "Wheel contents:" echo followed by 'ls -lh dist/') with either a single 'ls -la dist/' after the "Built artifacts:" echo, or if you intend to show actual wheel archive contents, replace that block with a command that inspects wheel archives (e.g., 'unzip -l dist/*.whl') and update the label accordingly; look for the exact strings "🔍 Dry run complete - artifacts built but not published", "Built artifacts:", "Wheel contents:", 'ls -la dist/' and 'ls -lh dist/' to make this change.
48-59: Version extraction regex may miss pre-release versions.The sed pattern
's/.*-\([0-9][0-9.]*\)-.*/\1/p'only matches numeric versions with dots. It won't correctly extract versions like1.0.0rc1,1.0.0a1, or1.0.0.dev1which are valid PEP 440 versions.Additionally, consider using
startsWith(github.ref, 'refs/tags/')instead ofcontains(github.ref, 'tags')for more precise tag detection—containscould theoretically match a branch namedmy-tags-branch.Suggested improvements
- name: Validate version matches tag - if: contains(github.ref, 'tags') + if: startsWith(github.ref, 'refs/tags/') run: | WHEEL=$(ls dist/*.whl | head -1) - WHEEL_VERSION=$(echo "$WHEEL" | sed -n 's/.*-\([0-9][0-9.]*\)-.*/\1/p') + WHEEL_VERSION=$(echo "$WHEEL" | sed -n 's/.*-\([0-9][0-9.a-zA-Z]*\)-.*/\1/p') TAG_VERSION="${GITHUB_REF_NAME#v}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 48 - 59, The workflow step "Validate version matches tag" uses a narrow sed pattern to set WHEEL_VERSION and a loose contains(github.ref, 'tags') check; update the tag detection to use startsWith(github.ref, 'refs/tags/') and broaden the wheel-version extraction so pre-release PEP 440 identifiers (e.g., a, b, rc, dev, plus signs, letters) are captured into WHEEL_VERSION (replace the sed pattern that produces WHEEL_VERSION with one that allows letters and punctuation used in PEP 440), then compare that WHEEL_VERSION to TAG_VERSION (still derived from GITHUB_REF_NAME#v) as before. Ensure you modify the "Validate version matches tag" step and the variables WHEEL_VERSION and TAG_VERSION in the diff..github/workflows/test.yml (2)
177-199: Well-structured non-blocking security job.The security job design is sound:
- Job-level
continue-on-error: trueensures pipeline completion- Step-level
continue-on-error: trueallows both scans to run even if one fails- Bandit exclusions (B110, B101) are reasonable for test/development code
One minor improvement: consider saving the security scan outputs as artifacts for later review.
Optional: Save security scan artifacts
- name: Bandit scan continue-on-error: true - run: bandit -r src/ --skip B110,B101 -f json + run: bandit -r src/ --skip B110,B101 -f json -o bandit-report.json || true - name: Safety check continue-on-error: true - run: safety check + run: safety check --output json > safety-report.json || true + + - name: Upload security reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: security-reports + path: | + bandit-report.json + safety-report.json + retention-days: 30🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 177 - 199, Add artifact upload steps to the existing security job so scan outputs are preserved: capture Bandit JSON by writing the bandit command output to a file (in the "Bandit scan" step) and capture Safety output to a file (in the "Safety check" step), then add subsequent steps that use actions/upload-artifact@v4 to upload those files; reference the "security" job and the "Bandit scan" and "Safety check" steps when implementing so the outputs (e.g., bandit-report.json and safety-report.txt) are saved as artifacts for later review.
191-191: Safety version pin explanation would be helpful.The
"safety<4.0"pin suggests a breaking change or compatibility issue in Safety v4. Consider adding a comment explaining the reason for the pin, which helps future maintainers understand when it might be safe to upgrade.Add explanatory comment
- name: Install security tools - run: pip install bandit "safety<4.0" + # Safety 4.0+ requires authentication; pin to 3.x for open-source CI + run: pip install bandit "safety<4.0"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml at line 191, Add a brief inline comment next to the pip install step that pins Safety ("safety<4.0") explaining why v4 is avoided (e.g., known breaking change, API/CLI differences, or compatibility issue with our test tooling), reference the run: pip install bandit "safety<4.0" line so reviewers can find it, and include guidance on what conditions or release notes to look for before removing the pin (e.g., "remove when Safety v4 fixes X or our tooling supports new CLI").CONTRIBUTING.md (1)
203-207: "Bad" example uses same function nameadd_layeras the "good" example above.Both examples define
add_layer, which may confuse readers. Consider using a different function name in the bad example (e.g.,add_dataorprocess_layer) to make the contrast clearer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` around lines 203 - 207, The "Bad" example reuses the function name add_layer which conflicts with the "Good" example; change the bad example's function name (e.g., add_data or process_layer) so it doesn't reuse add_layer and update any references within that snippet to the new name, ensuring the examples clearly contrast without name collision.docs/scripts/gen_ref_pages.py (2)
198-199: Hardcoded tool count "16" will drift out of sync with actual tools.The tool count is hardcoded as "16" in multiple places while
ALL_TOOLSis computed dynamically. This creates a maintenance burden where adding/removing tools requires updating these strings manually.Consider deriving the count from
ALL_TOOLS:# Line 198-199 f"Auto-generated documentation for all {len(ALL_TOOLS)} napari MCP server tools, "# Line 264 f"**Total: {len(ALL_TOOLS)} MCP tools available**"Also applies to: 259-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/scripts/gen_ref_pages.py` around lines 198 - 199, Replace the hardcoded "16" tool counts with dynamic length calls using the ALL_TOOLS collection: update the f-strings that currently say "all 16 napari MCP server tools" and "**Total: 16 MCP tools available**" to use f"...{len(ALL_TOOLS)}..." so the displayed counts (around the blocks that build the page header and the summary/footer) reflect the actual number of tools; ensure you reference the same ALL_TOOLS variable used to generate the tool list so the count stays in sync with functions that iterate over ALL_TOOLS.
82-84: Comment mentions@_registerdecorator but code doesn't actually check for it.The docstring states it looks for functions "decorated with
@_register", but the implementation only checks ifchild.name in ALL_TOOLS. This is fine if the intention is to match by name (which works), but the comment is misleading.Consider either:
- Updating the docstring to reflect the actual behavior (matching by name)
- Or adding decorator verification if that's truly desired
Also applies to: 93-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/scripts/gen_ref_pages.py` around lines 82 - 84, The docstring for create_server() incorrectly claims it looks for functions decorated with `@_register` while the implementation actually matches by name using ALL_TOOLS (child.name in ALL_TOOLS); update the docstring to state that the function searches for async def <name>(...) inside create_server() that match names in ALL_TOOLS (or clearly document the name-based matching), or alternatively implement actual decorator verification by inspecting child.decorator_list for a decorator named '_register' and requiring it in addition to name-matching (referencing create_server(), ALL_TOOLS, and the `@_register` decorator).src/napari_mcp/_helpers.py (1)
93-97: Silent exception swallowing may hide bugs in contrast_limits extraction.The bare
except Exception: passsilently ignores all errors. While this prevents crashes, it could hide legitimate bugs (e.g., unexpected data types). Consider logging at debug level or being more specific about expected exceptions.♻️ Suggested improvement
try: cl = layer.contrast_limits detail["contrast_limits"] = [float(cl[0]), float(cl[1])] - except Exception: - pass + except (TypeError, IndexError, AttributeError): + # contrast_limits may be None or have unexpected format + pass🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/napari_mcp/_helpers.py` around lines 93 - 97, The current try/except around reading layer.contrast_limits silently swallows all exceptions; change it to catch only expected exceptions (e.g., AttributeError, TypeError, IndexError) when converting cl to floats and populate detail["contrast_limits"] from contrast_limits, and add a debug-level log message including the offending value (contrast_limits or cl) when an exception occurs so issues are visible during debugging; update the block around contrast_limits in _helpers.py (the variables/members: layer.contrast_limits, cl, detail["contrast_limits"]) to implement specific exception handling and a debug log instead of a bare except/pass.src/napari_mcp/__init__.py (1)
21-26: Decouple guarded Qt imports to avoid coupled fallback behavior.If one import fails, both symbols are set to
Nonetoday. Splitting thetryblocks keeps partial availability and makes failures easier to diagnose.♻️ Suggested split
-try: - from .bridge_server import NapariBridgeServer - from .widget import MCPControlWidget -except ImportError: # pragma: no cover - NapariBridgeServer = None # type: ignore[assignment,misc] - MCPControlWidget = None # type: ignore[assignment,misc] +try: + from .bridge_server import NapariBridgeServer +except ImportError: # pragma: no cover + NapariBridgeServer = None # type: ignore[assignment,misc] + +try: + from .widget import MCPControlWidget +except ImportError: # pragma: no cover + MCPControlWidget = None # type: ignore[assignment,misc]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/napari_mcp/__init__.py` around lines 21 - 26, Split the combined try/except into two independent try blocks so a failure importing one symbol doesn't set both to None: wrap "from .bridge_server import NapariBridgeServer" in its own try/except and set NapariBridgeServer = None with the same type ignore and pragma as fallback, and do the same separately for "from .widget import MCPControlWidget" so MCPControlWidget remains available when only the other import fails; preserve the ImportError catch, comments (pragma: no cover), and the type: ignore annotations for each fallback assignment.tests/test_performance.py (1)
120-128: Reset global server state at the end of the exec-globals persistence test.Line 121 sets shared viewer state but the test doesn’t restore it, so later tests may inherit mutated state.
♻️ Minimal teardown pattern
- viewer = make_napari_viewer() - napari_mcp_server._state.viewer = viewer - - res = await napari_mcp_server.execute_code("my_var = 42") - assert res["status"] == "ok" - - res = await napari_mcp_server.execute_code("my_var * 2") - assert res["status"] == "ok" - assert res["result_repr"] == "84" + viewer = make_napari_viewer() + napari_mcp_server._state.viewer = viewer + try: + res = await napari_mcp_server.execute_code("my_var = 42") + assert res["status"] == "ok" + + res = await napari_mcp_server.execute_code("my_var * 2") + assert res["status"] == "ok" + assert res["result_repr"] == "84" + finally: + napari_mcp_server._state.viewer = None + napari_mcp_server._state.exec_globals.clear()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_performance.py` around lines 120 - 128, The test sets shared viewer state (viewer via make_napari_viewer) on napari_mcp_server._state.viewer but never restores it; update the test to reset that global state at the end (teardown) by clearing napari_mcp_server._state.viewer (e.g., set to None) so subsequent tests do not inherit mutated viewer state; locate the assignments to viewer and napari_mcp_server._state.viewer in the test and add a final cleanup step to restore the original value.tests/test_bridge_server.py (1)
78-78: Accessing private FastMCP internals is fragile.
server._tool_manager._toolsrelies on FastMCP's internal implementation details. If FastMCP refactors its internals, this test will break silently.Consider using the public
get_tools()method (already used elsewhere in this file) for consistency and resilience:tools = await server.server.get_tools() tool_names = set(tools.keys())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_bridge_server.py` at line 78, The test is accessing FastMCP internals via server.server._tool_manager._tools which is fragile; replace that direct access with the public API by calling await server.server.get_tools() and derive tool_names from the returned mapping (e.g., tool_names = set(tools.keys())) so the test uses server.server.get_tools() instead of server.server._tool_manager._tools.src/napari_mcp/qt_helpers.py (1)
41-56: Consider logging exceptions instead of silent pass.The broad
except: passat line 55-56 may hide legitimate issues (e.g.,viewer.windowexists but_qt_windowdoesn't due to napari version changes).🔧 Suggested improvement
except Exception: - pass + import logging + logging.getLogger(__name__).debug( + "Could not connect window destroyed signal", exc_info=True + )This preserves the defensive behavior while providing debuggability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/napari_mcp/qt_helpers.py` around lines 41 - 56, The function connect_window_destroyed_signal currently swallows all errors with a bare except; replace the silent pass with logging of the caught exception so failures retrieving viewer.window._qt_window or connecting qt_win.destroyed.connect(...) are visible; catch Exception as e and call a module logger (e.g., logging.getLogger(__name__)) to log the exception (logger.exception or logger.error with the exception) while preserving the existing behavior of not raising and keeping state.window_close_connected unchanged on failure.src/napari_mcp/cli/install/utils.py (1)
206-214: Consider preserving the original args structure when napari_requirement is provided.The current implementation replaces the entire
argslist whennapari_requirementis set. This works but duplicates the base structure. A minor improvement would be to insert the extra--witharguments.♻️ Optional: Insert instead of replace
if napari_requirement: - config["args"] = [ - "run", - "--with", - "napari-mcp", - "--with", - napari_requirement, - "napari-mcp", - ] + # Insert additional --with before the final "napari-mcp" + config["args"] = [ + "run", + "--with", + "napari-mcp", + "--with", + napari_requirement, + "napari-mcp", + ]The current approach is fine and explicit—this is purely a stylistic observation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/napari_mcp/cli/install/utils.py` around lines 206 - 214, The code replaces config["args"] when napari_requirement is set, duplicating the base args; instead update the existing config["args"] list by inserting the extra "--with", napari_requirement pair into the current args (e.g., locate config["args"] and call an insert/extend operation) so the original structure (including existing "run" and "napari-mcp" entries) is preserved; modify the block that checks napari_requirement to append or insert the two tokens rather than reassigning config["args"].src/napari_mcp/state.py (3)
120-129: Eviction logic has potential inefficiency with large storage.The eviction sorts all keys on every insert that exceeds capacity. For typical usage this is fine, but with very large
max_output_itemsvalues, this could become noticeable.For the current use case (default 1000 items), this is acceptable. If performance becomes a concern, consider using
collections.OrderedDictand popping from the front, or maintaining insertion order differently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/napari_mcp/state.py` around lines 120 - 129, The eviction logic in the method that trims output_storage when len(self.output_storage) > self.max_output_items currently sorts all keys each time (see self.output_storage, self.max_output_items), which is inefficient for large stores; to fix, switch to a data structure that preserves insertion order such as collections.OrderedDict (or Python 3.7+ dict guarantees insertion order) and update the code that inserts/evicts so you can popitem(last=False) or pop the oldest key without sorting; update any places that assign into self.output_storage to use OrderedDict (or maintain a separate deque of keys) and replace the sorted(...)[ :overflow] removal with repeated popitem(last=False) (or deque.popleft) to remove exactly overflow oldest entries.
170-171: Broad exception handling swallows all errors silently.Catching
Exceptionand returningNoneis appropriate for the fallback behavior, but consider logging at DEBUG level for troubleshooting.🔧 Add debug logging for troubleshooting
except Exception: + logger.debug("proxy_to_external failed", exc_info=True) return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/napari_mcp/state.py` around lines 170 - 171, The except Exception: return None block in src/napari_mcp/state.py is swallowing errors silently; change it to capture the exception (except Exception as e) and emit a debug-level log before returning None—use the module logger (e.g., logger.debug) or logging.getLogger(__name__).debug with a concise message and the exception info (include the exception object or exc_info) so troubleshooting is possible while preserving the fallback behavior in that function.
143-171: Add connection timeout to prevent hanging on unreachable services.The
fastmcp.Clientsupports atimeoutparameter to prevent indefinite hangs when the external service is unreachable.🔧 Add timeout parameter
try: from fastmcp import Client - client = Client(f"http://localhost:{self.bridge_port}/mcp") + client = Client( + f"http://localhost:{self.bridge_port}/mcp", + timeout=5.0, + ) async with client:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/napari_mcp/state.py` around lines 143 - 171, The external MCP client creation using fastmcp.Client currently can hang if the external service is unreachable; update the client instantiation (where fastmcp.Client is constructed with f"http://localhost:{self.bridge_port}/mcp") to pass a timeout argument (e.g., timeout=<reasonable_seconds>) so connections fail fast, and ensure any timeout-related exceptions are handled like other exceptions in the surrounding try/except block; locate the code around the Client construction and the async with client block and add the timeout param to the Client(...) call (and adjust error handling if you want to surface timeout-specific info).src/napari_mcp/bridge_server.py (1)
272-272: Consider documenting the 600s timeout choice.The execute_code timeout is hardcoded to 600s (10 minutes), which differs from the general 300s (5 minutes) default in
run_in_main_thread. A brief comment explaining this choice would help maintainability.📝 Add clarifying comment
- self.qt_bridge.run_in_main_thread(_run_on_qt, timeout=600.0) + # 10 minutes: longer than default 5min to allow heavier computations + self.qt_bridge.run_in_main_thread(_run_on_qt, timeout=600.0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/napari_mcp/bridge_server.py` at line 272, The execute_code coroutine sets a hardcoded 600s timeout which differs from the 300s default used by run_in_main_thread; add a concise comment above async def execute_code (and/or next to the timeout usage) explaining why a longer 10-minute timeout is required (e.g., expected long-running user code, blocking image processing, or historical reasons) and note that it intentionally diverges from run_in_main_thread’s 300s default so future maintainers understand the rationale.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/getting-started/quickstart.md`:
- Line 36: The fenced code block starting with "```bash" should be converted to
an indented code block to satisfy markdownlint MD046; locate the fenced block
marker (```bash) in quickstart.md and replace the fenced block by indenting each
line of the snippet with four spaces (or alternatively update the repo's
markdownlint config to allow fenced blocks), ensuring the code is formatted as
an indented code block rather than using ``` markers.
In `@docs/integrations/index.md`:
- Around line 24-25: The table rows use bare tokens ("cline-cursor" and "codex")
that are easy to mis-copy; replace those tokens with the full CLI commands to
match the first alternative. Specifically, in the row containing "Cline" change
the second column from `cline-cursor` to `napari-mcp-install install
cline-cursor`, and in the "Gemini / Codex" row change `codex` to
`napari-mcp-install install codex` (or `napari-mcp-install install gemini` if
you prefer that alternative), so both options are full, copy-pasteable commands.
In `@docs/integrations/python.md`:
- Line 171: Replace the invalid Anthropic model identifier used in the example
(the model="claude-sonnet-4-6" entry) with a valid model ID; update the string
in the docs/integrations/python.md snippet to a supported name such as
"claude-sonnet-4-20250514" or "claude-3-7-sonnet-20250219" so the sample will
work against the Anthropic API.
In `@mkdocs.yml`:
- Around line 114-118: The navigation entries in mkdocs.yml reference missing
docs: api/session.md, api/viewer-controls.md, and api/modules.md; fix by either
creating the three files under docs/api (session.md, viewer-controls.md,
modules.md) with appropriate content or update the mkdocs.yml nav entries (the
lines listing "Session & Viewer: api/session.md", "Viewer Controls:
api/viewer-controls.md", "Supporting Modules: api/modules.md") to point to
existing documentation paths or remove them; ensure the referenced filenames
exactly match the created files' names and that the docs/api directory contains
those files so MkDocs validation passes.
In `@pyproject.toml`:
- Around line 186-190: Remove the file-wide S102/S307 suppressions from
server.py and bridge_server.py and remove the S603/S607 suppression from
codex_cli.py; keep the A002 suppression in server.py. Instead, add inline noqa
comments on the specific risky callsites: in _helpers.py add "# noqa: S102,
S307" directly on the lines with the exec/eval calls inside the run_code/execute
helpers (the three exec/eval occurrences), and in cli/install/utils.py add "#
noqa: S603, S607" on the single subprocess call used for Python detection; this
preserves intent while avoiding broad file-level silences.
- Line 55: The `all` extra under [project.optional-dependencies] currently only
lists "napari-mcp[dev]" which omits the `[dependency-groups]` testing
dependencies, causing pip installs of napari-mcp[all] to miss test tools; update
the `all` extra to include the testing group packages (or merge the testing
group into `all`) so that `all` contains the same test-related entries defined
under `testing` in `[dependency-groups]` (or alternatively add a short comment
documenting the intentional divergence between `[project.optional-dependencies]
all` and `[dependency-groups] testing` if you prefer to keep them separate).
In `@src/napari_mcp/_helpers.py`:
- Around line 223-232: Validate that resolved_data is a sequence with at least
two elements and that resolved_data[0] and resolved_data[1] are array-like
before indexing; in the branch that calls viewer.add_surface and assigns verts =
np.asarray(resolved_data[0]) and faces = np.asarray(resolved_data[1]), add a
guard that checks isinstance(resolved_data, (list, tuple)) and
len(resolved_data) >= 2 (and optionally verify verts.ndim == 2 and faces.ndim ==
2 with appropriate shapes), and if the check fails return or raise a clear
ValueError/error dict indicating an invalid surface tuple instead of allowing an
IndexError from verts/faces access.
- Around line 214-221: In the elif lt == "tracks" branch validate the numpy
array before indexing: ensure resolved_data -> arr is a 2D array (arr.ndim == 2)
and has at least one column (arr.shape[1] >= 1); if validation fails return a
clear error status/message instead of proceeding to viewer.add_tracks and
computing n_tracks via arr[:,0]; update the block around the arr creation and
the n_tracks calculation to perform these checks and handle invalid shapes
gracefully (refer to variables arr, viewer.add_tracks, and the n_tracks
calculation using arr[:, 0]).
In `@src/napari_mcp/cli/main.py`:
- Around line 341-344: In list_installations, reading each config with
read_json_config can raise for corrupted files; wrap the call to
read_json_config(config_path) in a try/except that catches the propagated
exceptions, log or warn about the corrupted config (including config_path and
error) and continue to the next item, then only call
check_existing_server(config, "napari-mcp") when config was successfully loaded
to avoid aborting the entire listing.
In `@src/napari_mcp/qt_helpers.py`:
- Around line 14-27: The current check "if QtWidgets is None" never runs because
qtpy raises on failed imports; change the top import of QtWidgets to a
try/except that catches ImportError (and optionally QtBindingsNotFoundError) and
sets QtWidgets = None on failure, then keep the existing fallback that defines
_StubApp and assigns state.qt_app = _StubApp() when state.qt_app is None; ensure
references to QtWidgets, _StubApp and state.qt_app are used exactly as in the
diff so the stub is used in headless environments.
In `@src/napari_mcp/state.py`:
- Around line 208-240: The external_session_information method lacks exception
handling around the external Client usage (unlike detect_external_viewer and
proxy_to_external); wrap the Client creation and async with /
test_client.call_tool block in a try/except, catch broad exceptions, log or
record the exception (including the exception message and optionally traceback)
and return the existing error dict with a helpful message (including the
exception text) instead of letting the exception propagate; reference
external_session_information, Client, test_client.call_tool and self.bridge_port
when locating and updating the code.
In `@tests/test_helpers.py`:
- Around line 449-460: Update the type hint for the line_limit parameter to
include None so it matches the function's runtime behavior and the test; locate
the function signature (e.g., build_truncated_response or the helper in
_helpers.py where line_limit is declared as line_limit: int | str) and change
the annotation to line_limit: int | str | None (or Optional[int | str]) to
reflect that None is accepted and will fallback to 30.
In `@tests/test_property_based.py`:
- Around line 66-70: The test's line-counting logic for variable result doesn't
match truncate_output's behavior; change the computation of actual_line_count to
mirror truncate_output by using result.splitlines(keepends=True) (and handle
result being falsy as before) so actual_line_count =
len(result.splitlines(keepends=True)) — update the assertion accordingly;
reference variables: actual_line_count, result and function truncate_output to
locate the code to change.
In `@tests/test_qt_helpers.py`:
- Around line 138-139: The test currently only asserts
mock_shutdown.assert_called_once() inside an if mock_shutdown.called guard so it
can silently pass if the Qt destroyed signal never fires; change the test to be
deterministic by explicitly emitting or invoking the cleanup path (e.g., trigger
the widget's destroyed signal or call the method that should run shutdown) and
then assert mock_shutdown.assert_called_once() unconditionally, or if you cannot
reliably trigger the signal make the test skip (pytest.skip) instead of silently
passing; locate the assertion around mock_shutdown and the code that
connects/emits the Qt destroyed signal to implement this change.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 67-68: Update the workflow conditional that currently uses
contains(github.ref, 'tags') && !inputs.dry_run to use startsWith(github.ref,
'refs/tags/') && !inputs.dry_run for precise tag detection; locate the step
using the softprops/action-gh-release@v2 entry and replace the condition so it
only triggers on refs that actually start with refs/tags/.
- Around line 85-89: Remove the redundant directory listing by keeping a single
informative listing and correct the misleading "Wheel contents:" line: replace
the duplicate 'ls -lh dist/' (or the "Wheel contents:" echo followed by 'ls -lh
dist/') with either a single 'ls -la dist/' after the "Built artifacts:" echo,
or if you intend to show actual wheel archive contents, replace that block with
a command that inspects wheel archives (e.g., 'unzip -l dist/*.whl') and update
the label accordingly; look for the exact strings "🔍 Dry run complete -
artifacts built but not published", "Built artifacts:", "Wheel contents:", 'ls
-la dist/' and 'ls -lh dist/' to make this change.
- Around line 48-59: The workflow step "Validate version matches tag" uses a
narrow sed pattern to set WHEEL_VERSION and a loose contains(github.ref, 'tags')
check; update the tag detection to use startsWith(github.ref, 'refs/tags/') and
broaden the wheel-version extraction so pre-release PEP 440 identifiers (e.g.,
a, b, rc, dev, plus signs, letters) are captured into WHEEL_VERSION (replace the
sed pattern that produces WHEEL_VERSION with one that allows letters and
punctuation used in PEP 440), then compare that WHEEL_VERSION to TAG_VERSION
(still derived from GITHUB_REF_NAME#v) as before. Ensure you modify the
"Validate version matches tag" step and the variables WHEEL_VERSION and
TAG_VERSION in the diff.
In @.github/workflows/test.yml:
- Around line 177-199: Add artifact upload steps to the existing security job so
scan outputs are preserved: capture Bandit JSON by writing the bandit command
output to a file (in the "Bandit scan" step) and capture Safety output to a file
(in the "Safety check" step), then add subsequent steps that use
actions/upload-artifact@v4 to upload those files; reference the "security" job
and the "Bandit scan" and "Safety check" steps when implementing so the outputs
(e.g., bandit-report.json and safety-report.txt) are saved as artifacts for
later review.
- Line 191: Add a brief inline comment next to the pip install step that pins
Safety ("safety<4.0") explaining why v4 is avoided (e.g., known breaking change,
API/CLI differences, or compatibility issue with our test tooling), reference
the run: pip install bandit "safety<4.0" line so reviewers can find it, and
include guidance on what conditions or release notes to look for before removing
the pin (e.g., "remove when Safety v4 fixes X or our tooling supports new CLI").
In `@CONTRIBUTING.md`:
- Around line 203-207: The "Bad" example reuses the function name add_layer
which conflicts with the "Good" example; change the bad example's function name
(e.g., add_data or process_layer) so it doesn't reuse add_layer and update any
references within that snippet to the new name, ensuring the examples clearly
contrast without name collision.
In `@docs/scripts/gen_ref_pages.py`:
- Around line 198-199: Replace the hardcoded "16" tool counts with dynamic
length calls using the ALL_TOOLS collection: update the f-strings that currently
say "all 16 napari MCP server tools" and "**Total: 16 MCP tools available**" to
use f"...{len(ALL_TOOLS)}..." so the displayed counts (around the blocks that
build the page header and the summary/footer) reflect the actual number of
tools; ensure you reference the same ALL_TOOLS variable used to generate the
tool list so the count stays in sync with functions that iterate over ALL_TOOLS.
- Around line 82-84: The docstring for create_server() incorrectly claims it
looks for functions decorated with `@_register` while the implementation actually
matches by name using ALL_TOOLS (child.name in ALL_TOOLS); update the docstring
to state that the function searches for async def <name>(...) inside
create_server() that match names in ALL_TOOLS (or clearly document the
name-based matching), or alternatively implement actual decorator verification
by inspecting child.decorator_list for a decorator named '_register' and
requiring it in addition to name-matching (referencing create_server(),
ALL_TOOLS, and the `@_register` decorator).
In `@src/napari_mcp/__init__.py`:
- Around line 21-26: Split the combined try/except into two independent try
blocks so a failure importing one symbol doesn't set both to None: wrap "from
.bridge_server import NapariBridgeServer" in its own try/except and set
NapariBridgeServer = None with the same type ignore and pragma as fallback, and
do the same separately for "from .widget import MCPControlWidget" so
MCPControlWidget remains available when only the other import fails; preserve
the ImportError catch, comments (pragma: no cover), and the type: ignore
annotations for each fallback assignment.
In `@src/napari_mcp/_helpers.py`:
- Around line 93-97: The current try/except around reading layer.contrast_limits
silently swallows all exceptions; change it to catch only expected exceptions
(e.g., AttributeError, TypeError, IndexError) when converting cl to floats and
populate detail["contrast_limits"] from contrast_limits, and add a debug-level
log message including the offending value (contrast_limits or cl) when an
exception occurs so issues are visible during debugging; update the block around
contrast_limits in _helpers.py (the variables/members: layer.contrast_limits,
cl, detail["contrast_limits"]) to implement specific exception handling and a
debug log instead of a bare except/pass.
In `@src/napari_mcp/bridge_server.py`:
- Line 272: The execute_code coroutine sets a hardcoded 600s timeout which
differs from the 300s default used by run_in_main_thread; add a concise comment
above async def execute_code (and/or next to the timeout usage) explaining why a
longer 10-minute timeout is required (e.g., expected long-running user code,
blocking image processing, or historical reasons) and note that it intentionally
diverges from run_in_main_thread’s 300s default so future maintainers understand
the rationale.
In `@src/napari_mcp/cli/install/utils.py`:
- Around line 206-214: The code replaces config["args"] when napari_requirement
is set, duplicating the base args; instead update the existing config["args"]
list by inserting the extra "--with", napari_requirement pair into the current
args (e.g., locate config["args"] and call an insert/extend operation) so the
original structure (including existing "run" and "napari-mcp" entries) is
preserved; modify the block that checks napari_requirement to append or insert
the two tokens rather than reassigning config["args"].
In `@src/napari_mcp/qt_helpers.py`:
- Around line 41-56: The function connect_window_destroyed_signal currently
swallows all errors with a bare except; replace the silent pass with logging of
the caught exception so failures retrieving viewer.window._qt_window or
connecting qt_win.destroyed.connect(...) are visible; catch Exception as e and
call a module logger (e.g., logging.getLogger(__name__)) to log the exception
(logger.exception or logger.error with the exception) while preserving the
existing behavior of not raising and keeping state.window_close_connected
unchanged on failure.
In `@src/napari_mcp/state.py`:
- Around line 120-129: The eviction logic in the method that trims
output_storage when len(self.output_storage) > self.max_output_items currently
sorts all keys each time (see self.output_storage, self.max_output_items), which
is inefficient for large stores; to fix, switch to a data structure that
preserves insertion order such as collections.OrderedDict (or Python 3.7+ dict
guarantees insertion order) and update the code that inserts/evicts so you can
popitem(last=False) or pop the oldest key without sorting; update any places
that assign into self.output_storage to use OrderedDict (or maintain a separate
deque of keys) and replace the sorted(...)[ :overflow] removal with repeated
popitem(last=False) (or deque.popleft) to remove exactly overflow oldest
entries.
- Around line 170-171: The except Exception: return None block in
src/napari_mcp/state.py is swallowing errors silently; change it to capture the
exception (except Exception as e) and emit a debug-level log before returning
None—use the module logger (e.g., logger.debug) or
logging.getLogger(__name__).debug with a concise message and the exception info
(include the exception object or exc_info) so troubleshooting is possible while
preserving the fallback behavior in that function.
- Around line 143-171: The external MCP client creation using fastmcp.Client
currently can hang if the external service is unreachable; update the client
instantiation (where fastmcp.Client is constructed with
f"http://localhost:{self.bridge_port}/mcp") to pass a timeout argument (e.g.,
timeout=<reasonable_seconds>) so connections fail fast, and ensure any
timeout-related exceptions are handled like other exceptions in the surrounding
try/except block; locate the code around the Client construction and the async
with client block and add the timeout param to the Client(...) call (and adjust
error handling if you want to surface timeout-specific info).
In `@tests/test_bridge_server.py`:
- Line 78: The test is accessing FastMCP internals via
server.server._tool_manager._tools which is fragile; replace that direct access
with the public API by calling await server.server.get_tools() and derive
tool_names from the returned mapping (e.g., tool_names = set(tools.keys())) so
the test uses server.server.get_tools() instead of
server.server._tool_manager._tools.
In `@tests/test_performance.py`:
- Around line 120-128: The test sets shared viewer state (viewer via
make_napari_viewer) on napari_mcp_server._state.viewer but never restores it;
update the test to reset that global state at the end (teardown) by clearing
napari_mcp_server._state.viewer (e.g., set to None) so subsequent tests do not
inherit mutated viewer state; locate the assignments to viewer and
napari_mcp_server._state.viewer in the test and add a final cleanup step to
restore the original value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74e57ab7-9162-45fa-b7d5-821963503c56
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (69)
.github/workflows/claude-code-review.yml.github/workflows/docs.yml.github/workflows/release.yml.github/workflows/test.ymlCONTRIBUTING.mdMANIFEST.inREADME.mdcodecov.ymldocs/examples/README.mddocs/examples/anthropic_integration.pydocs/getting-started/index.mddocs/getting-started/installation.mddocs/getting-started/quickstart.mddocs/getting-started/zero-install.mddocs/guides/napari-plugin.mddocs/guides/troubleshooting.mddocs/index.mddocs/integrations/chatgpt.mddocs/integrations/claude-code.mddocs/integrations/claude-desktop.mddocs/integrations/cline.mddocs/integrations/cursor.mddocs/integrations/index.mddocs/integrations/other-llms.mddocs/integrations/python.mddocs/scripts/gen_ref_pages.pymkdocs.ymlpyproject.tomlsrc/napari_mcp/__init__.pysrc/napari_mcp/_helpers.pysrc/napari_mcp/bridge_server.pysrc/napari_mcp/cli/install/base.pysrc/napari_mcp/cli/install/cline_cursor.pysrc/napari_mcp/cli/install/codex_cli.pysrc/napari_mcp/cli/install/cursor.pysrc/napari_mcp/cli/install/gemini_cli.pysrc/napari_mcp/cli/install/utils.pysrc/napari_mcp/cli/main.pysrc/napari_mcp/output.pysrc/napari_mcp/qt_helpers.pysrc/napari_mcp/server.pysrc/napari_mcp/state.pysrc/napari_mcp/viewer_protocol.pysrc/napari_mcp/widget.pytests/conftest.pytests/fixtures/__init__.pytests/fixtures/async_utils.pytests/test_base_installer.pytests/test_bridge_server.pytests/test_bridge_simplified.pytests/test_cli_installer.pytests/test_cli_installers/test_platform_installers.pytests/test_cli_utils.pytests/test_coverage.pytests/test_edge_cases.pytests/test_external_viewer.pytests/test_helpers.pytests/test_integration.pytests/test_napari_server_coverage.pytests/test_output_storage.pytests/test_performance.pytests/test_property_based.pytests/test_qt_helpers.pytests/test_server_tools.pytests/test_state.pytests/test_timelapse.pytests/test_tools.pytests/test_widget.pytox.ini
💤 Files with no reviewable changes (8)
- MANIFEST.in
- tox.ini
- tests/test_bridge_simplified.py
- tests/fixtures/init.py
- tests/test_napari_server_coverage.py
- tests/fixtures/async_utils.py
- tests/test_edge_cases.py
- tests/test_coverage.py
|
|
||
| ### For Other Applications | ||
|
|
||
| ```bash |
There was a problem hiding this comment.
Fix markdownlint MD046 in this section.
Line 36 uses fenced code-block style where markdownlint expects indented style, so this will keep doc lint noisy/failing.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 36-36: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/getting-started/quickstart.md` at line 36, The fenced code block
starting with "```bash" should be converted to an indented code block to satisfy
markdownlint MD046; locate the fenced block marker (```bash) in quickstart.md
and replace the fenced block by indenting each line of the snippet with four
spaces (or alternatively update the repo's markdownlint config to allow fenced
blocks), ensuring the code is formatted as an indented code block rather than
using ``` markers.
Comment on lines
+24
to
+25
| | **Cline** | `napari-mcp-install install cline-vscode` or `cline-cursor` | ✅ Full Support | [Setup →](cline.md) | | ||
| | **Gemini / Codex** | `napari-mcp-install install gemini` or `codex` | ✅ Full Support | [Setup →](other-llms.md) | |
There was a problem hiding this comment.
Use full CLI command for both alternatives in the table.
On Lines 24-25, the second option in each row is a bare token (cline-cursor, codex) instead of a full command, which is easy to misread/copy incorrectly.
Suggested doc fix
-| **Cline** | `napari-mcp-install install cline-vscode` or `cline-cursor` | ✅ Full Support | [Setup →](cline.md) |
-| **Gemini / Codex** | `napari-mcp-install install gemini` or `codex` | ✅ Full Support | [Setup →](other-llms.md) |
+| **Cline** | `napari-mcp-install install cline-vscode` or `napari-mcp-install install cline-cursor` | ✅ Full Support | [Setup →](cline.md) |
+| **Gemini / Codex** | `napari-mcp-install install gemini` or `napari-mcp-install install codex` | ✅ Full Support | [Setup →](other-llms.md) |📝 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.
Suggested change
| | **Cline** | `napari-mcp-install install cline-vscode` or `cline-cursor` | ✅ Full Support | [Setup →](cline.md) | | |
| | **Gemini / Codex** | `napari-mcp-install install gemini` or `codex` | ✅ Full Support | [Setup →](other-llms.md) | | |
| | **Cline** | `napari-mcp-install install cline-vscode` or `napari-mcp-install install cline-cursor` | ✅ Full Support | [Setup →](cline.md) | | |
| | **Gemini / Codex** | `napari-mcp-install install gemini` or `napari-mcp-install install codex` | ✅ Full Support | [Setup →](other-llms.md) | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/integrations/index.md` around lines 24 - 25, The table rows use bare
tokens ("cline-cursor" and "codex") that are easy to mis-copy; replace those
tokens with the full CLI commands to match the first alternative. Specifically,
in the row containing "Cline" change the second column from `cline-cursor` to
`napari-mcp-install install cline-cursor`, and in the "Gemini / Codex" row
change `codex` to `napari-mcp-install install codex` (or `napari-mcp-install
install gemini` if you prefer that alternative), so both options are full,
copy-pasteable commands.
| # Use Claude to interact with napari | ||
| message = client.messages.create( | ||
| model="claude-3-5-sonnet-20241022", | ||
| model="claude-sonnet-4-6", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Anthropic Claude model names 2025
💡 Result:
Anthropic Claude model names you’d commonly see in 2025 (product name → Anthropic API model ID):
- Claude 3.7 Sonnet →
claude-3-7-sonnet-20250219(released Feb 24, 2025) [1][2] - Claude Sonnet 4 →
claude-sonnet-4-20250514[2] - Claude Opus 4 →
claude-opus-4-20250514[2]
Models also widely used during 2025 (though their snapshot dates are 2024):
- Claude Sonnet 3.5 →
claude-3-5-sonnet-20240620,claude-3-5-sonnet-20241022[3] - Claude Haiku 3.5 →
claude-3-5-haiku-20241022[2][3]
Sources: [1] Anthropic announcement for Claude 3.7 Sonnet, [2] Anthropic “Model deprecations” list (includes official API model IDs), [3] Anthropic API release notes (lists 3.5 Sonnet model IDs and deprecation info.
Use a valid Anthropic model name instead of claude-sonnet-4-6.
The model name claude-sonnet-4-6 is invalid. Anthropic model IDs follow a specific format with a date suffix (e.g., claude-sonnet-4-20250514). Use claude-sonnet-4-20250514 (Claude Sonnet 4) or claude-3-7-sonnet-20250219 (Claude 3.7 Sonnet) instead. The current invalid name will cause API errors when users run this example.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/integrations/python.md` at line 171, Replace the invalid Anthropic model
identifier used in the example (the model="claude-sonnet-4-6" entry) with a
valid model ID; update the string in the docs/integrations/python.md snippet to
a supported name such as "claude-sonnet-4-20250514" or
"claude-3-7-sonnet-20250219" so the sample will work against the Anthropic API.
Comment on lines
+114
to
+118
| - Session & Viewer: api/session.md | ||
| - Layer Management: api/layer-management.md | ||
| - Viewer Controls: api/viewer-controls.md | ||
| - Utilities: api/utilities.md | ||
| - Supporting Modules: api/modules.md |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking MkDocs nav targets added/updated in mkdocs.yml..."
for rel in docs/api/session.md docs/api/viewer-controls.md docs/api/modules.md; do
if [ -f "$rel" ]; then
echo "FOUND: $rel"
else
echo "MISSING: $rel"
fi
done
echo
echo "Nav entries in mkdocs.yml:"
rg -n 'Session & Viewer|Viewer Controls|Supporting Modules' mkdocs.ymlRepository: royerlab/napari-mcp
Length of output: 388
All three referenced API documentation files are missing and will break the docs build.
Lines 114, 116, and 118 reference files that do not exist:
docs/api/session.md(referenced asapi/session.md)docs/api/viewer-controls.md(referenced asapi/viewer-controls.md)docs/api/modules.md(referenced asapi/modules.md)
MkDocs will fail validation and the build will fail when it tries to resolve these navigation targets. Create these files or update the navigation entries in mkdocs.yml to point to existing documentation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mkdocs.yml` around lines 114 - 118, The navigation entries in mkdocs.yml
reference missing docs: api/session.md, api/viewer-controls.md, and
api/modules.md; fix by either creating the three files under docs/api
(session.md, viewer-controls.md, modules.md) with appropriate content or update
the mkdocs.yml nav entries (the lines listing "Session & Viewer:
api/session.md", "Viewer Controls: api/viewer-controls.md", "Supporting Modules:
api/modules.md") to point to existing documentation paths or remove them; ensure
the referenced filenames exactly match the created files' names and that the
docs/api directory contains those files so MkDocs validation passes.
| "black>=24.0.0", | ||
| ] | ||
| all = ["napari-mcp[test,dev]"] | ||
| all = ["napari-mcp[dev]"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether repo docs/workflows expect `.[all]` to include test tooling.
rg -n -C2 '\.\[all\]|napari-mcp\[all\]|napari-mcp\[dev\]|optional-dependencies|dependency-groups'Repository: royerlab/napari-mcp
Length of output: 1940
🏁 Script executed:
# Check if there are any references to other extras like 'test', 'docs', 'full' in the repo
rg -i 'extras.*=|dependencies.*\[test\]|\[docs\]|\[full\]|dev.*test|test.*dev' --type toml --type yaml --type mdRepository: royerlab/napari-mcp
Length of output: 672
🏁 Script executed:
# Check README or documentation for guidance on extras
fd -e md -e rst -e txt | head -20 | xargs grep -l "extra\|optional\|install" 2>/dev/null || echo "No match found"Repository: royerlab/napari-mcp
Length of output: 586
🏁 Script executed:
# Read the full optional-dependencies and dependency-groups sections
sed -n '46,60p' pyproject.toml
echo "---"
sed -n '195,215p' pyproject.tomlRepository: royerlab/napari-mcp
Length of output: 839
🏁 Script executed:
# Get a wider view of the optional-dependencies section to see all extras
sed -n '46,75p' pyproject.tomlRepository: royerlab/napari-mcp
Length of output: 743
🏁 Script executed:
# Also get the full dependency-groups section to see what all includes
sed -n '198,230p' pyproject.tomlRepository: royerlab/napari-mcp
Length of output: 875
[project.optional-dependencies] all extra is incomplete and inconsistent with other package managers.
Line 55 defines all = ["napari-mcp[dev]"], but a separate testing group with pytest and related tools is defined in [dependency-groups]. This causes pip install napari-mcp[all] to exclude testing dependencies, while uv add napari-mcp[all] and pixi include them via [dependency-groups] and [tool.pixi.environments].
Either add testing dependencies to the pip all extra for consistency, or document the intentional split across package managers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pyproject.toml` at line 55, The `all` extra under
[project.optional-dependencies] currently only lists "napari-mcp[dev]" which
omits the `[dependency-groups]` testing dependencies, causing pip installs of
napari-mcp[all] to miss test tools; update the `all` extra to include the
testing group packages (or merge the testing group into `all`) so that `all`
contains the same test-related entries defined under `testing` in
`[dependency-groups]` (or alternatively add a short comment documenting the
intentional divergence between `[project.optional-dependencies] all` and
`[dependency-groups] testing` if you prefer to keep them separate).
Comment on lines
+14
to
+27
| from qtpy import QtWidgets | ||
|
|
||
| if QtWidgets is None: | ||
|
|
||
| class _StubApp: | ||
| def processEvents(self, *_: Any) -> None: # noqa: N802 | ||
| pass | ||
|
|
||
| def setQuitOnLastWindowClosed(self, *_: Any) -> None: # noqa: N802 | ||
| pass | ||
|
|
||
| if state.qt_app is None: | ||
| state.qt_app = _StubApp() | ||
| return state.qt_app |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does qtpy QtWidgets import return None when Qt is not available?
💡 Result:
No. If no Qt Python bindings (PyQt5/PySide2/PyQt6/PySide6) are available, importing qtpy (and thus from qtpy import QtWidgets) raises qtpy.QtBindingsNotFoundError (an ImportError), with the message “No Qt bindings could be found”—it does not return None. [1]
If a binding is selected but a particular submodule isn’t available, QtPy raises a (different) module-not-found exception (QtModuleNotFoundError / ModuleNotFoundError), rather than returning None. [1]
Sources:
[1] QtPy source (qtpy/__init__.py) showing QtBindingsNotFoundError and raise QtBindingsNotFoundError when bindings can’t be imported. (github.com)
Citations:
Fix the QtWidgets is None check—it will never be true.
When qtpy.QtWidgets cannot be imported, qtpy raises ImportError (specifically QtBindingsNotFoundError), not returns None. Without catching this exception, the code will crash instead of falling back to the stub implementation.
Wrap the import in a try/except block to properly handle headless scenarios:
Suggested fix
def ensure_qt_app(state: ServerState) -> Any:
"""Return the Qt application, creating one if necessary, or a no-op stub."""
- from qtpy import QtWidgets
-
- if QtWidgets is None:
+ try:
+ from qtpy import QtWidgets
+ except ImportError:
+ QtWidgets = None
+ if QtWidgets is None:
class _StubApp:
def processEvents(self, *_: Any) -> None: # noqa: N802
pass
def setQuitOnLastWindowClosed(self, *_: Any) -> None: # noqa: N802
pass
if state.qt_app is None:
state.qt_app = _StubApp()
return state.qt_app📝 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.
Suggested change
| from qtpy import QtWidgets | |
| if QtWidgets is None: | |
| class _StubApp: | |
| def processEvents(self, *_: Any) -> None: # noqa: N802 | |
| pass | |
| def setQuitOnLastWindowClosed(self, *_: Any) -> None: # noqa: N802 | |
| pass | |
| if state.qt_app is None: | |
| state.qt_app = _StubApp() | |
| return state.qt_app | |
| try: | |
| from qtpy import QtWidgets | |
| except ImportError: | |
| QtWidgets = None | |
| if QtWidgets is None: | |
| class _StubApp: | |
| def processEvents(self, *_: Any) -> None: # noqa: N802 | |
| pass | |
| def setQuitOnLastWindowClosed(self, *_: Any) -> None: # noqa: N802 | |
| pass | |
| if state.qt_app is None: | |
| state.qt_app = _StubApp() | |
| return state.qt_app |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/napari_mcp/qt_helpers.py` around lines 14 - 27, The current check "if
QtWidgets is None" never runs because qtpy raises on failed imports; change the
top import of QtWidgets to a try/except that catches ImportError (and optionally
QtBindingsNotFoundError) and sets QtWidgets = None on failure, then keep the
existing fallback that defines _StubApp and assigns state.qt_app = _StubApp()
when state.qt_app is None; ensure references to QtWidgets, _StubApp and
state.qt_app are used exactly as in the diff so the stub is used in headless
environments.
Comment on lines
+208
to
+240
| async def external_session_information(self) -> dict[str, Any]: | ||
| """Get session information from the external viewer.""" | ||
| from fastmcp import Client | ||
|
|
||
| test_client = Client(f"http://localhost:{self.bridge_port}/mcp") | ||
| async with test_client: | ||
| result = await test_client.call_tool("session_information") | ||
| if hasattr(result, "content"): | ||
| content = result.content | ||
| if isinstance(content, list) and len(content) > 0: | ||
| info = ( | ||
| content[0].text | ||
| if hasattr(content[0], "text") | ||
| else str(content[0]) | ||
| ) | ||
| info_dict = json.loads(info) if isinstance(info, str) else info | ||
| if info_dict.get("session_type") == "napari_bridge_session": | ||
| return { | ||
| "status": "ok", | ||
| "viewer_type": "external", | ||
| "title": info_dict.get("viewer", {}).get( | ||
| "title", "External Viewer" | ||
| ), | ||
| "layers": info_dict.get("viewer", {}).get( | ||
| "layer_names", [] | ||
| ), | ||
| "port": info_dict.get("bridge_port", self.bridge_port), | ||
| } | ||
|
|
||
| return { | ||
| "status": "error", | ||
| "message": "Failed to get session information from external viewer", | ||
| } |
There was a problem hiding this comment.
Missing error handling for external_session_information.
Unlike detect_external_viewer and proxy_to_external, this method doesn't have a try/except block. If the client connection fails or call_tool raises, the exception will propagate uncaught.
🛡️ Proposed fix: Add try/except for consistency
async def external_session_information(self) -> dict[str, Any]:
"""Get session information from the external viewer."""
- from fastmcp import Client
+ try:
+ from fastmcp import Client
- test_client = Client(f"http://localhost:{self.bridge_port}/mcp")
- async with test_client:
- result = await test_client.call_tool("session_information")
- if hasattr(result, "content"):
- content = result.content
- if isinstance(content, list) and len(content) > 0:
- info = (
- content[0].text
- if hasattr(content[0], "text")
- else str(content[0])
- )
- info_dict = json.loads(info) if isinstance(info, str) else info
- if info_dict.get("session_type") == "napari_bridge_session":
- return {
- "status": "ok",
- "viewer_type": "external",
- "title": info_dict.get("viewer", {}).get(
- "title", "External Viewer"
- ),
- "layers": info_dict.get("viewer", {}).get(
- "layer_names", []
- ),
- "port": info_dict.get("bridge_port", self.bridge_port),
- }
+ test_client = Client(f"http://localhost:{self.bridge_port}/mcp")
+ async with test_client:
+ result = await test_client.call_tool("session_information")
+ if hasattr(result, "content"):
+ content = result.content
+ if isinstance(content, list) and len(content) > 0:
+ info = (
+ content[0].text
+ if hasattr(content[0], "text")
+ else str(content[0])
+ )
+ info_dict = json.loads(info) if isinstance(info, str) else info
+ if info_dict.get("session_type") == "napari_bridge_session":
+ return {
+ "status": "ok",
+ "viewer_type": "external",
+ "title": info_dict.get("viewer", {}).get(
+ "title", "External Viewer"
+ ),
+ "layers": info_dict.get("viewer", {}).get(
+ "layer_names", []
+ ),
+ "port": info_dict.get("bridge_port", self.bridge_port),
+ }
+ except Exception:
+ pass
return {
"status": "error",
"message": "Failed to get session information from external viewer",
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/napari_mcp/state.py` around lines 208 - 240, The
external_session_information method lacks exception handling around the external
Client usage (unlike detect_external_viewer and proxy_to_external); wrap the
Client creation and async with / test_client.call_tool block in a try/except,
catch broad exceptions, log or record the exception (including the exception
message and optionally traceback) and return the existing error dict with a
helpful message (including the exception text) instead of letting the exception
propagate; reference external_session_information, Client, test_client.call_tool
and self.bridge_port when locating and updating the code.
Comment on lines
+449
to
+460
| def test_none_line_limit_falls_back(self): | ||
| """None line_limit should fall back to 30, not crash.""" | ||
| resp = build_truncated_response( | ||
| status="ok", | ||
| output_id="id1", | ||
| stdout_full="hello\n", | ||
| stderr_full="", | ||
| result_repr=None, | ||
| line_limit=None, | ||
| ) | ||
| assert resp["status"] == "ok" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the type hint for line_limit in build_truncated_response
rg -n "def build_truncated_response" --type py -A 15Repository: royerlab/napari-mcp
Length of output: 1002
🏁 Script executed:
#!/bin/bash
# Get the full implementation of build_truncated_response
sed -n '325,380p' src/napari_mcp/_helpers.pyRepository: royerlab/napari-mcp
Length of output: 1672
🏁 Script executed:
#!/bin/bash
# Verify the test content at lines 449-460
sed -n '449,460p' tests/test_helpers.pyRepository: royerlab/napari-mcp
Length of output: 440
Type hint for line_limit should include None.
The test at line 457 passes line_limit=None, which the function handles gracefully via a try/except block that catches TypeError and defaults to 30. However, the type hint at line 332 of _helpers.py specifies line_limit: int | str, omitting None. Update the type hint to line_limit: int | str | None to match the intentional behavior documented in the test.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_helpers.py` around lines 449 - 460, Update the type hint for the
line_limit parameter to include None so it matches the function's runtime
behavior and the test; locate the function signature (e.g.,
build_truncated_response or the helper in _helpers.py where line_limit is
declared as line_limit: int | str) and change the annotation to line_limit: int
| str | None (or Optional[int | str]) to reflect that None is accepted and will
fallback to 30.
Comment on lines
+66
to
+70
| actual_line_count = ( | ||
| len([line for line in result.split("\n") if line]) if result else 0 | ||
| ) | ||
|
|
||
| # Property: Zoom level should remain positive | ||
| assert mock_viewer.camera.zoom > 0 | ||
| assert actual_line_count <= max(line_limit, 0) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how truncate_output counts lines
ast-grep --pattern $'def truncate_output($_) {
$$$
}'
# Also search for the implementation
rg -n "def truncate_output" --type py -A 20Repository: royerlab/napari-mcp
Length of output: 1185
🏁 Script executed:
cat -n src/napari_mcp/output.pyRepository: royerlab/napari-mcp
Length of output: 1165
Line counting logic in the test does not match truncate_output implementation.
The function uses splitlines(keepends=True) and counts ALL lines (line 30-31 in src/napari_mcp/output.py), but the test counts only non-empty lines with len([line for line in result.split("\n") if line]). This means the test can pass even if truncate_output produces more lines than the limit allows, as long as some lines are empty.
Fix the test to count lines the same way the function does:
actual_line_count = len(result.splitlines(keepends=True))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_property_based.py` around lines 66 - 70, The test's line-counting
logic for variable result doesn't match truncate_output's behavior; change the
computation of actual_line_count to mirror truncate_output by using
result.splitlines(keepends=True) (and handle result being falsy as before) so
actual_line_count = len(result.splitlines(keepends=True)) — update the assertion
accordingly; reference variables: actual_line_count, result and function
truncate_output to locate the code to change.
Comment on lines
+138
to
+139
| if mock_shutdown.called: | ||
| mock_shutdown.assert_called_once() |
There was a problem hiding this comment.
Conditional assertion may silently pass without verification.
If the Qt destroyed signal doesn't fire (which is noted as fragile in the comment), the test passes without asserting anything. This could hide regressions.
Consider making the test more deterministic:
🔧 Suggested approach
- # If the Qt signal fired, request_shutdown was called
- if mock_shutdown.called:
- mock_shutdown.assert_called_once()
+ # Note: Qt signal firing is fragile in tests.
+ # At minimum, verify the signal connection was established.
+ assert napari_mcp_server._state.window_close_connected is TrueOr skip the test if reliable verification isn't possible, rather than having a test that may not test anything.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_qt_helpers.py` around lines 138 - 139, The test currently only
asserts mock_shutdown.assert_called_once() inside an if mock_shutdown.called
guard so it can silently pass if the Qt destroyed signal never fires; change the
test to be deterministic by explicitly emitting or invoking the cleanup path
(e.g., trigger the widget's destroyed signal or call the method that should run
shutdown) and then assert mock_shutdown.assert_called_once() unconditionally, or
if you cannot reliably trigger the signal make the test skip (pytest.skip)
instead of silently passing; locate the assertion around mock_shutdown and the
code that connects/emits the Qt destroyed signal to implement this change.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
190-199: Consider improving the safety check invocation.Two observations:
Safety check without arguments: Running
safety checkwithout specifying a requirements file may not scan the project's actual dependencies. Consider generating requirements first or using uv's export:run: | uv export --no-hashes > requirements.txt safety check -r requirements.txtVersion consistency: The workflow installs bandit via
pip install bandit(gets latest), whilepyproject.tomldeclaresbandit>=1.8.6. This is minor since the job is advisory, but could cause inconsistent results between local and CI runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 190 - 199, Update the "Install security tools" and "Safety check" steps so the safety scan runs against an exported requirements file and pin bandit to the project-declared version: generate a requirements.txt from the project environment (e.g., using uv export --no-hashes) before the "Safety check" step and invoke safety with -r requirements.txt, and change the pip install for bandit in the "Install security tools" step to match the pyproject constraint (e.g., install "bandit>=1.8.6") to keep CI tooling consistent with local/dev requirements.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test.yml:
- Around line 43-47: Update the GitHub Actions usage of the astral-sh/setup-uv
action by changing the version tag from "astral-sh/setup-uv@v6" to
"astral-sh/setup-uv@v7" wherever it appears (e.g., the occurrences around the
name: Install uv step and the second occurrence near line 96); keep the existing
inputs (enable-cache, cache-suffix) unchanged.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 190-199: Update the "Install security tools" and "Safety check"
steps so the safety scan runs against an exported requirements file and pin
bandit to the project-declared version: generate a requirements.txt from the
project environment (e.g., using uv export --no-hashes) before the "Safety
check" step and invoke safety with -r requirements.txt, and change the pip
install for bandit in the "Install security tools" step to match the pyproject
constraint (e.g., install "bandit>=1.8.6") to keep CI tooling consistent with
local/dev requirements.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5efb371-c0ec-4ad0-94b7-a2a89163f4c3
📒 Files selected for processing (1)
.github/workflows/test.yml
Comment on lines
43
to
47
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v4 | ||
| uses: astral-sh/setup-uv@v6 | ||
| with: | ||
| enable-cache: true | ||
| cache-suffix: "smoke" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check available tags for setup-uv action
gh api repos/astral-sh/setup-uv/tags --jq '.[].name' | head -20Repository: royerlab/napari-mcp
Length of output: 223
🌐 Web query:
astral-sh/setup-uv action releases versions v6
💡 Result:
astral-sh/setup-uv does have a v6 major line (usable as uses: astral-sh/setup-uv@v6, which floats to the latest v6.x.y). The last v6 release is v6.8.0 (“Add **/*.py.lock to cache-dependency-glob”, dated Sep 30). [1]
As of Mar 16, 2026, the latest overall major is v7 (latest release shown: v7.6.0), and Astral’s docs now show uses: astral-sh/setup-uv@v7 in examples. [2], [3]
Sources: [1] [2] [3]
Upgrade setup-uv action from v6 to v7.
The workflow currently uses astral-sh/setup-uv@v6, but v7 is now the latest stable release and is recommended in Astral's official documentation. This appears in lines 44 and 96.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/test.yml around lines 43 - 47, Update the GitHub Actions
usage of the astral-sh/setup-uv action by changing the version tag from
"astral-sh/setup-uv@v6" to "astral-sh/setup-uv@v7" wherever it appears (e.g.,
the occurrences around the name: Install uv step and the second occurrence near
line 96); keep the existing inputs (enable-cache, cache-suffix) unchanged.
Contributor
|
✅ Documentation builds successfully! The site will be updated when this PR is merged. |
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
./.venv/bin/pytest tests/ --verbose --cov=napari_mcp --cov-report=xml --cov-report=term-missing --cov-fail-under=80 --durations=20 --timeout=60./.venv/bin/ruff check src/ tests/./.venv/bin/ruff format --check src/ tests/./.venv/bin/mypy src/napari_mcp/ --ignore-missing-importsMerging this into
loic-refactorshould update the existing PR tomainwith the CI fixes.Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Documentation