feat: LLM Auto-Installation CLI System#34
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
d020e39 to
53b1e4b
Compare
|
✅ Documentation builds successfully! The site will be updated when this PR is merged. |
Pull Request Review: LLM Auto-Installation CLI SystemI've conducted a comprehensive review of this PR that introduces an extensive CLI installer system for napari-mcp across multiple AI platforms. This is a significant and well-architected contribution, but there are several critical issues that should be addressed. 🎯 Overall AssessmentGrade: B+ (Good implementation, but needs dependency fixes and minor improvements) This PR adds a sophisticated CLI installer system that follows good architectural patterns with a base class and platform-specific implementations. The code is generally well-structured and follows Python best practices. 🚨 Critical Issues to Address1. Missing Dependencies in pyproject.tomlThe CLI code extensively uses typer and rich, but these dependencies are not declared in pyproject.toml. This will cause import failures for users. Action Required: Add to dependencies:
2. Missing CLI Entry PointThe new CLI command napari-mcp-install is not defined in [project.scripts]. Action Required: Add napari-mcp-install entry point to pyproject.toml 🔍 Code Quality Assessment✅ Strengths
🔧 Code Quality Issuessrc/napari_mcp/cli/main.py:240-242Issue: Extra blank lines between function definitions src/napari_mcp/cli/install/utils.py:125-152Issue: get_python_executable doesn't validate if uv is installed 🛡️ Security Assessment✅ Security Strengths
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/napari_mcp/cli/install/claude_desktop.py (1)
1-8: Prefer built-indictovertyping.Dict(Python 3.9+).Static analysis indicates that
typing.Dictis deprecated in favor of the built-indicttype annotation (PEP 585). If the project targets Python 3.9 or later, consider updating the import.Apply this diff:
from pathlib import Path -from typing import Any, Dict +from typing import AnyAnd update line 36:
- def get_extra_config(self) -> Dict[str, Any]: + def get_extra_config(self) -> dict[str, Any]:src/napari_mcp/cli/install/cursor.py (1)
4-4: Update deprecated type hint.Use
dictinstead oftyping.Dictper PEP 585 (Python 3.9+).Apply this diff:
-from typing import Any, Dict, Optional +from typing import Any, OptionalAnd update the return type annotations:
- def get_extra_config(self) -> Dict[str, Any]: + def get_extra_config(self) -> dict[str, Any]:src/napari_mcp/cli/install/codex_cli.py (2)
4-4: Update deprecated type hint.Use
dictinstead oftyping.Dictper PEP 585 (Python 3.9+).Apply this diff:
-from typing import Any, Dict +from typing import AnyAnd update the return type:
- def get_extra_config(self) -> Dict[str, Any]: + def get_extra_config(self) -> dict[str, Any]:
77-81: Simplify nested if statements.Combine the nested conditionals into a single statement for better readability.
Apply this diff:
- if server_name in config.get("mcp_servers", {}): - if not self.force: + if server_name in config.get("mcp_servers", {}) and not self.force: from .utils import prompt_update_existing if not prompt_update_existing("Codex CLI", config_path): return False, "User cancelled update"src/napari_mcp/cli/install/base.py (2)
5-5: Update deprecated type hints.Use
tupleanddictinstead oftyping.Tupleandtyping.Dictper PEP 585 (Python 3.9+).Apply this diff:
-from typing import Any, Dict, Optional, Tuple +from typing import Any, OptionalAnd update return type annotations:
- def get_extra_config(self) -> Dict[str, Any]: + def get_extra_config(self) -> dict[str, Any]: """Get any extra configuration fields specific to this application. - def install(self) -> Tuple[bool, str]: + def install(self) -> tuple[bool, str]: """Install the MCP server configuration. - def uninstall(self) -> Tuple[bool, str]: + def uninstall(self) -> tuple[bool, str]: """Uninstall the MCP server configuration.
133-135: Simplify nested if statements.Combine the nested conditionals into a single statement.
Apply this diff:
# Check for existing installation - if check_existing_server(config, self.server_name): - if not self.force and not prompt_update_existing(self.app_name, config_path): + if check_existing_server(config, self.server_name) and not self.force: + if not prompt_update_existing(self.app_name, config_path): return False, "User cancelled update"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
INSTALLATION.md(1 hunks)src/napari_mcp/cli/__init__.py(1 hunks)src/napari_mcp/cli/install/__init__.py(1 hunks)src/napari_mcp/cli/install/base.py(1 hunks)src/napari_mcp/cli/install/claude_code.py(1 hunks)src/napari_mcp/cli/install/claude_desktop.py(1 hunks)src/napari_mcp/cli/install/cline_cursor.py(1 hunks)src/napari_mcp/cli/install/cline_vscode.py(1 hunks)src/napari_mcp/cli/install/codex_cli.py(1 hunks)src/napari_mcp/cli/install/cursor.py(1 hunks)src/napari_mcp/cli/install/gemini_cli.py(1 hunks)src/napari_mcp/cli/install/utils.py(1 hunks)src/napari_mcp/cli/main.py(1 hunks)tests/test_base_installer.py(1 hunks)tests/test_cli_installer.py(1 hunks)tests/test_cli_installers/test_platform_installers.py(1 hunks)tests/test_cli_utils.py(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/napari_mcp/cli/init.py
- INSTALLATION.md
🧰 Additional context used
🧬 Code graph analysis (14)
src/napari_mcp/cli/install/cline_vscode.py (3)
src/napari_mcp/cli/install/base.py (4)
BaseInstaller(24-221)get_config_path(66-74)get_extra_config(77-85)show_post_install_message(214-221)src/napari_mcp/cli/install/utils.py (2)
expand_path(36-53)get_platform(19-33)src/napari_mcp/cli/install/cline_cursor.py (3)
get_config_path(21-42)get_extra_config(44-56)show_post_install_message(58-72)
src/napari_mcp/cli/install/cline_cursor.py (3)
src/napari_mcp/cli/install/base.py (4)
BaseInstaller(24-221)get_config_path(66-74)get_extra_config(77-85)show_post_install_message(214-221)src/napari_mcp/cli/install/utils.py (2)
expand_path(36-53)get_platform(19-33)src/napari_mcp/cli/install/cline_vscode.py (3)
get_config_path(21-42)get_extra_config(44-56)show_post_install_message(58-71)
src/napari_mcp/cli/install/claude_desktop.py (2)
src/napari_mcp/cli/install/base.py (3)
BaseInstaller(24-221)get_config_path(66-74)get_extra_config(77-85)src/napari_mcp/cli/install/utils.py (2)
expand_path(36-53)get_platform(19-33)
src/napari_mcp/cli/install/cursor.py (3)
src/napari_mcp/cli/install/base.py (4)
BaseInstaller(24-221)get_config_path(66-74)get_extra_config(77-85)show_post_install_message(214-221)src/napari_mcp/cli/install/utils.py (1)
expand_path(36-53)src/napari_mcp/cli/install/gemini_cli.py (3)
get_config_path(62-89)get_extra_config(91-104)show_post_install_message(106-121)
src/napari_mcp/cli/install/gemini_cli.py (3)
src/napari_mcp/cli/install/base.py (4)
BaseInstaller(24-221)get_config_path(66-74)get_extra_config(77-85)show_post_install_message(214-221)src/napari_mcp/cli/install/utils.py (1)
expand_path(36-53)src/napari_mcp/cli/install/cursor.py (3)
get_config_path(62-89)get_extra_config(91-99)show_post_install_message(101-107)
src/napari_mcp/cli/install/codex_cli.py (3)
src/napari_mcp/cli/install/base.py (6)
BaseInstaller(24-221)get_config_path(66-74)get_extra_config(77-85)install(107-168)validate_environment(87-105)uninstall(170-212)src/napari_mcp/cli/install/utils.py (3)
expand_path(36-53)prompt_update_existing(219-240)get_python_executable(125-152)src/napari_mcp/cli/main.py (1)
uninstall(401-486)
src/napari_mcp/cli/install/claude_code.py (2)
src/napari_mcp/cli/install/base.py (3)
BaseInstaller(24-221)get_config_path(66-74)get_extra_config(77-85)src/napari_mcp/cli/install/utils.py (1)
expand_path(36-53)
tests/test_base_installer.py (1)
src/napari_mcp/cli/install/base.py (6)
install(107-168)BaseInstaller(24-221)get_config_path(66-74)get_extra_config(77-85)validate_environment(87-105)uninstall(170-212)
src/napari_mcp/cli/install/__init__.py (7)
src/napari_mcp/cli/install/claude_desktop.py (1)
ClaudeDesktopInstaller(10-44)src/napari_mcp/cli/install/claude_code.py (1)
ClaudeCodeInstaller(10-36)src/napari_mcp/cli/install/cursor.py (1)
CursorInstaller(15-107)src/napari_mcp/cli/install/cline_vscode.py (1)
ClineVSCodeInstaller(14-71)src/napari_mcp/cli/install/cline_cursor.py (1)
ClineCursorInstaller(14-72)src/napari_mcp/cli/install/gemini_cli.py (1)
GeminiCLIInstaller(15-121)src/napari_mcp/cli/install/codex_cli.py (1)
CodexCLIInstaller(14-187)
tests/test_cli_utils.py (1)
src/napari_mcp/cli/install/utils.py (10)
build_server_config(155-195)check_existing_server(198-216)expand_path(36-53)get_app_display_name(322-345)get_platform(19-33)get_python_executable(125-152)prompt_update_existing(219-240)read_json_config(56-77)show_installation_summary(243-262)validate_python_environment(265-297)
tests/test_cli_installer.py (10)
src/napari_mcp/cli/main.py (2)
main(40-47)uninstall(401-486)src/napari_mcp/cli/install/base.py (3)
install(107-168)uninstall(170-212)get_config_path(66-74)src/napari_mcp/cli/install/codex_cli.py (3)
install(42-124)uninstall(126-174)get_config_path(21-30)src/napari_mcp/cli/install/claude_code.py (1)
get_config_path(17-26)src/napari_mcp/cli/install/claude_desktop.py (1)
get_config_path(17-34)src/napari_mcp/cli/install/cline_cursor.py (1)
get_config_path(21-42)src/napari_mcp/cli/install/cline_vscode.py (1)
get_config_path(21-42)src/napari_mcp/cli/install/cursor.py (1)
get_config_path(62-89)src/napari_mcp/cli/install/gemini_cli.py (1)
get_config_path(62-89)tests/test_base_installer.py (1)
get_config_path(16-18)
tests/test_cli_installers/test_platform_installers.py (8)
src/napari_mcp/cli/install/base.py (4)
install(107-168)get_config_path(66-74)get_extra_config(77-85)uninstall(170-212)src/napari_mcp/cli/install/codex_cli.py (4)
install(42-124)get_config_path(21-30)get_extra_config(32-40)uninstall(126-174)src/napari_mcp/cli/install/claude_code.py (3)
ClaudeCodeInstaller(10-36)get_config_path(17-26)get_extra_config(28-36)src/napari_mcp/cli/install/claude_desktop.py (3)
ClaudeDesktopInstaller(10-44)get_config_path(17-34)get_extra_config(36-44)src/napari_mcp/cli/install/cline_cursor.py (3)
ClineCursorInstaller(14-72)get_config_path(21-42)get_extra_config(44-56)src/napari_mcp/cli/install/cline_vscode.py (3)
ClineVSCodeInstaller(14-71)get_config_path(21-42)get_extra_config(44-56)src/napari_mcp/cli/install/cursor.py (3)
CursorInstaller(15-107)get_config_path(62-89)get_extra_config(91-99)src/napari_mcp/cli/install/gemini_cli.py (3)
GeminiCLIInstaller(15-121)get_config_path(62-89)get_extra_config(91-104)
src/napari_mcp/cli/install/base.py (7)
src/napari_mcp/cli/install/utils.py (8)
build_server_config(155-195)check_existing_server(198-216)expand_path(36-53)get_python_executable(125-152)prompt_update_existing(219-240)read_json_config(56-77)validate_python_environment(265-297)write_json_config(80-122)src/napari_mcp/cli/install/claude_code.py (2)
get_config_path(17-26)get_extra_config(28-36)src/napari_mcp/cli/install/claude_desktop.py (2)
get_config_path(17-34)get_extra_config(36-44)src/napari_mcp/cli/install/codex_cli.py (4)
get_config_path(21-30)get_extra_config(32-40)install(42-124)show_post_install_message(176-187)src/napari_mcp/cli/install/cursor.py (3)
get_config_path(62-89)get_extra_config(91-99)show_post_install_message(101-107)src/napari_mcp/cli/install/gemini_cli.py (3)
get_config_path(62-89)get_extra_config(91-104)show_post_install_message(106-121)tests/test_base_installer.py (2)
get_config_path(16-18)get_extra_config(20-22)
src/napari_mcp/cli/main.py (5)
src/napari_mcp/cli/install/base.py (3)
install(107-168)uninstall(170-212)get_config_path(66-74)src/napari_mcp/cli/install/codex_cli.py (3)
install(42-124)uninstall(126-174)get_config_path(21-30)src/napari_mcp/cli/install/cursor.py (1)
get_config_path(62-89)src/napari_mcp/cli/install/gemini_cli.py (1)
get_config_path(62-89)src/napari_mcp/cli/install/utils.py (4)
get_app_display_name(322-345)show_installation_summary(243-262)check_existing_server(198-216)read_json_config(56-77)
🪛 GitHub Check: quality
src/napari_mcp/cli/install/cline_vscode.py
[failure] 4-4: Ruff (UP035)
src/napari_mcp/cli/install/cline_vscode.py:4:1: UP035 typing.Dict is deprecated, use dict instead
src/napari_mcp/cli/install/cline_cursor.py
[failure] 4-4: Ruff (UP035)
src/napari_mcp/cli/install/cline_cursor.py:4:1: UP035 typing.Dict is deprecated, use dict instead
src/napari_mcp/cli/install/claude_desktop.py
[failure] 4-4: Ruff (UP035)
src/napari_mcp/cli/install/claude_desktop.py:4:1: UP035 typing.Dict is deprecated, use dict instead
src/napari_mcp/cli/install/cursor.py
[failure] 4-4: Ruff (UP035)
src/napari_mcp/cli/install/cursor.py:4:1: UP035 typing.Dict is deprecated, use dict instead
src/napari_mcp/cli/install/codex_cli.py
[failure] 77-78: Ruff (SIM102)
src/napari_mcp/cli/install/codex_cli.py:77:13: SIM102 Use a single if statement instead of nested if statements
[failure] 4-4: Ruff (UP035)
src/napari_mcp/cli/install/codex_cli.py:4:1: UP035 typing.Dict is deprecated, use dict instead
src/napari_mcp/cli/install/claude_code.py
[failure] 4-4: Ruff (UP035)
src/napari_mcp/cli/install/claude_code.py:4:1: UP035 typing.Dict is deprecated, use dict instead
src/napari_mcp/cli/install/base.py
[failure] 131-132: Ruff (SIM102)
src/napari_mcp/cli/install/base.py:131:13: SIM102 Use a single if statement instead of nested if statements
[failure] 5-5: Ruff (UP035)
src/napari_mcp/cli/install/base.py:5:1: UP035 typing.Tuple is deprecated, use tuple instead
[failure] 5-5: Ruff (UP035)
src/napari_mcp/cli/install/base.py:5:1: UP035 typing.Dict is deprecated, use dict instead
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: test-all-platforms (ubuntu-latest, 3.12)
- GitHub Check: test-all-platforms (ubuntu-latest, 3.10)
- GitHub Check: test-all-platforms (ubuntu-latest, 3.11)
- GitHub Check: test-all-platforms (ubuntu-latest, 3.13)
- GitHub Check: test-all-platforms (windows-latest, 3.13)
- GitHub Check: smoke-tests
- GitHub Check: claude-review
🔇 Additional comments (59)
src/napari_mcp/cli/install/utils.py (13)
1-17: LGTM!Module structure and imports are well-organized. The shared
Consoleinstance provides consistent formatting across all installer utilities.
19-34: LGTM!Platform detection logic correctly normalizes system names to a consistent set of values.
36-54: LGTM!Path expansion correctly handles user home (
~) and environment variables, with proper resolution for cross-platform compatibility.
56-78: LGTM!JSON configuration reading correctly preserves key order using
OrderedDictand gracefully handles missing files and parse errors.
80-123: LGTM!JSON configuration writing implements atomic writes correctly using temp files and rename. The backup mechanism with PID-based filenames prevents collisions, and error handling ensures temp files are cleaned up.
125-153: LGTM!Python executable resolution correctly handles custom paths, persistent environments, and uv-based ephemeral environments with appropriate user feedback.
155-196: LGTM!Server configuration construction correctly distinguishes between uv-based ephemeral environments (using package name
napari-mcp) and persistent Python environments (using module pathnapari_mcp.server), with support for app-specific extra configuration.
198-217: LGTM!Existence check for server configuration is straightforward and handles missing keys gracefully.
219-241: LGTM!User prompt for updating existing configuration has a safe default (False) and provides clear context about what will be modified.
243-263: LGTM!Installation summary table provides clear visual feedback with status indicators and color coding for easy scanning of results.
265-298: LGTM!Python environment validation uses a subprocess check with appropriate timeout and helpful error messages guiding users to install missing dependencies.
300-320: LGTM!Python environment detection correctly identifies conda environments, virtual environments, and system Python using standard detection patterns.
322-345: LGTM!Application display name mapping covers all supported installers with a sensible fallback for unknown keys. The inclusion of both
"codex"and"codex-cli"provides flexibility for different naming conventions.tests/test_base_installer.py (3)
1-23: LGTM!Test setup with a concrete implementation of
BaseInstallerprovides a clean fixture for testing the abstract base class behavior.
25-311: LGTM!Comprehensive test suite covering initialization, environment validation, install/uninstall workflows, dry-run mode, error handling, and edge cases with appropriate use of mocking.
313-376: LGTM!Integration tests verify critical behaviors like preserving existing server configurations during installation, force mode skipping prompts, and proper application of extra configuration fields.
src/napari_mcp/cli/install/claude_desktop.py (2)
13-15: LGTM!Initialization correctly delegates to
BaseInstallerwith the appropriate app key.
17-44: LGTM!Configuration path resolution correctly handles platform-specific locations for Claude Desktop on macOS, Windows, and Linux. Empty extra configuration is appropriate as Claude Desktop doesn't require additional fields.
tests/test_cli_installer.py (5)
1-27: LGTM!Test fixtures provide clean reusable components for CLI testing with appropriate mocking of installer behavior.
30-154: LGTM!CLI command tests comprehensively cover all installer entry points with proper verification of option propagation and mode-specific behavior (global vs project-specific installs).
156-224: LGTM!Install-all command tests cover both complete success and partial failure scenarios with appropriate mocking of multiple installer classes.
226-354: LGTM!Uninstall and list command tests provide comprehensive coverage including single-app operations, bulk uninstalls, invalid input handling, and special TOML configuration handling for Codex CLI.
356-381: LGTM!Error handling tests verify graceful exception handling and dry-run mode behavior with proper option propagation.
tests/test_cli_installers/test_platform_installers.py (7)
1-19: LGTM!Test imports are comprehensive and include all necessary installer classes and testing utilities.
21-72: LGTM!Platform-specific path tests for Claude Desktop and Claude Code correctly verify configuration locations across macOS, Windows, and Linux with appropriate path component checks.
74-153: LGTM!Tests comprehensively cover Cursor's global and project-specific installation modes, and Cline installers for both VS Code and Cursor IDE with platform-specific paths and extra configuration verification.
155-186: LGTM!Gemini CLI tests verify both global and project-specific configuration paths and validate the extra configuration fields (cwd, timeout, trust) with expected default values.
188-256: LGTM!Codex CLI tests correctly verify TOML-based configuration handling with appropriate mocking of file I/O and TOML operations, including validation of the underscore naming convention (
napari_mcp) for TOML compatibility.
258-317: LGTM!Edge case tests systematically verify path expansion across installers, cross-platform compatibility, and interface contract compliance (required methods and attributes) for all installer implementations.
319-367: LGTM!Integration tests verify end-to-end installation flows for different installer types and configurations, including persistent Python environments and project-specific installations with appropriate mocking and success verification.
src/napari_mcp/cli/install/gemini_cli.py (2)
62-89: LGTM: Global vs. project-specific configuration logic is well-implemented.The branching logic correctly handles both global and project-specific installation paths, with appropriate user confirmation for project installs. This mirrors the pattern used in the Cursor installer.
91-104: LGTM: Gemini-specific configuration fields are appropriate.The extra config includes sensible defaults for
cwd,timeout, andtrustfields specific to Gemini CLI.src/napari_mcp/cli/install/cline_vscode.py (2)
21-42: LGTM: Platform-specific paths are correctly implemented.The config path logic correctly handles macOS, Windows, and Linux variants for VS Code's global storage directory structure.
44-56: LGTM: Cline-specific configuration is appropriate.The
alwaysAllowanddisabledfields provide necessary tool permission controls for the Cline extension.src/napari_mcp/cli/install/cline_cursor.py (2)
21-42: LGTM: Platform-specific paths correctly target Cursor IDE.The paths appropriately reference Cursor's directory structure instead of VS Code's, maintaining the same extension storage pattern.
58-72: Excellent disambiguation in post-install message.The warning at lines 71-72 helpfully clarifies the difference between Cline extension configuration in Cursor vs. Cursor's native MCP support, preventing user confusion.
src/napari_mcp/cli/install/__init__.py (1)
1-19: LGTM: Clean package exports.The file correctly aggregates and exports all installer classes, providing a clean public API for the install package.
src/napari_mcp/cli/install/cursor.py (4)
18-60: LGTM!The initialization properly extends
BaseInstallerand stores the additionalglobal_installandproject_dirparameters needed for project-specific installations.
62-89: LGTM!The config path resolution correctly handles both global and project-specific installations with appropriate user confirmation. The pattern matches
GeminiCLIInstallerand the ValueError on cancellation will be caught by the parentinstall()method.
91-99: LGTM!Correctly returns an empty dict since Cursor doesn't require extra configuration fields like Gemini's timeout/trust settings.
101-107: LGTM!The post-install message appropriately delegates to the base implementation and adds helpful context for project-specific installations.
tests/test_cli_utils.py (1)
1-435: LGTM! Comprehensive test coverage.This test suite provides excellent coverage of the utility functions:
- Platform detection across OS types
- Path expansion with home directories and environment variables
- JSON config read/write with order preservation, backups, atomic writes, and error handling
- Python executable resolution for uv, persistent, and custom paths
- Environment validation with subprocess mocking
- Server config building for different modes
- Display name mapping
- User prompts and installation summaries
- Edge cases including Unicode and permissions
The use of mocks is appropriate for testing I/O-bound operations without side effects.
src/napari_mcp/cli/install/codex_cli.py (6)
17-19: LGTM!The initialization correctly passes the app_key to the base class using
**kwargsfor flexibility.
21-30: LGTM!Config path correctly points to
~/.codex/config.tomlwhich is the expected location for Codex CLI configuration.
32-40: LGTM!Correctly returns an empty dict since TOML format handles configuration structure differently than JSON-based installers.
42-124: LGTM!The custom
install()implementation correctly:
- Checks for the
tomldependency with a helpful error message- Handles TOML format with
mcp_servers(underscore) andnapari_mcpnaming conventions- Validates environment and checks for existing installations
- Supports dry-run mode
- Creates parent directories and writes TOML atomically
126-174: LGTM!The
uninstall()method correctly mirrors the install logic for TOML format, including dependency checks, config reading, server removal, and cleanup of empty sections.
176-187: LGTM!The post-install message provides clear next steps and helpful notes about persistent mode and TOML format.
src/napari_mcp/cli/install/base.py (6)
27-63: LGTM!The initialization properly stores all configuration parameters and derives the display name from the app key. The comprehensive docstring clearly documents all parameters.
65-85: LGTM!The abstract methods are properly defined with clear contracts. Subclasses must implement
get_config_path()and can optionally overrideget_extra_config()(which has a default implementation returning an empty dict).
87-105: LGTM!Environment validation correctly checks Python environments when using persistent or custom Python paths, validates napari-mcp availability, and respects the
--forceflag to continue despite validation failures.
107-168: LGTM!The install workflow is well-structured:
- Gets config path and validates environment
- Reads existing config and initializes
mcpServersif needed- Checks for existing installations with optional prompts
- Builds server config with extra fields
- Supports dry-run mode
- Writes config with optional backups
- Shows post-install guidance
Comprehensive error handling throughout.
170-212: LGTM!The uninstall workflow correctly:
- Checks config file exists
- Verifies server is configured
- Supports dry-run mode
- Removes server entry
- Cleans up empty
mcpServerssections- Writes config with backups
214-221: LGTM!Default post-install message provides clear next steps and notes about persistent Python environments. Subclasses can extend this with app-specific guidance.
src/napari_mcp/cli/main.py (5)
30-47: LGTM!The version callback and main app setup are correctly structured. Using
is_eager=Trueensures the version is checked before other processing.
50-238: LGTM!All per-app install commands follow a consistent pattern:
- Standard options (persistent, python_path, force, backup, dry_run)
- Additional options for cursor/gemini (global_install, project_dir)
- Instantiate installer with appropriate parameters
- Call install() and exit with code 1 on failure
The consistent structure makes the CLI predictable and maintainable.
324-397: LGTM!The
install allcommand correctly:
- Iterates through all installer classes
- Handles special case for cursor/gemini with
global_install=True- Collects results from each installer
- Shows a summary table
- Exits with error if any installation failed
This provides a convenient way to set up all applications at once.
400-486: LGTM!The uninstall command properly:
- Supports both single app and 'all' modes
- Uses the same app_map for consistency
- Handles cursor/gemini global_install flag
- Shows results summary for 'all' mode
- Provides helpful error messages for unknown apps
489-567: LGTM!The list command effectively:
- Creates a Rich table showing installation status
- Checks each app's config file
- Handles TOML format specially for Codex CLI
- Uses appropriate status symbols (✓/○/−/✗)
- Shows config paths and details
- Includes a legend for clarity
This provides users with a quick overview of their installation state.
Code ReviewThis is an excellent and comprehensive implementation of a CLI installer system for napari-mcp! I've conducted a thorough review across code quality, security, testing, and architecture. Here's my detailed feedback: StrengthsArchitecture & Design
Code Quality
Test Coverage
Security Considerations
Areas for ImprovementMinor Issues
Enhancement Opportunities
Performance Considerations
Potential Edge Cases
Overall AssessmentScore: 9.5/10 This PR demonstrates exceptional software engineering practices:
The implementation is robust, well-tested, and ready for production. The few minor suggestions are enhancements rather than fixes - the current code is of very high quality. Recommendation: APPROVE - This is exemplary work that significantly enhances the napari-mcp project! |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/scripts/gen_ref_pages.py (1)
10-42: Align function_categories with actual server implementations. The session and utilities lists reference functions that don’t exist in src/napari_mcp/server.py (detect_viewers, init_viewer, close_viewer, session_information, timelapse_screenshot, read_output), and the total count is 12, not 22. Update the categories or add the missing functions.
♻️ Duplicate comments (2)
src/napari_mcp/cli/install/claude_code.py (1)
1-37: LGTM! Implementation follows the installer pattern correctly.The ClaudeCodeInstaller properly extends BaseInstaller with:
- Correct app_key initialization
- Platform-agnostic config path (~/.claude.json)
- Empty extra_config as appropriate for Claude Code
- Modern type hints (dict[str, Any] instead of deprecated Dict)
Note: The deprecated
typing.Dictissue mentioned in past reviews has been resolved - the code now uses the moderndict[str, Any]syntax.src/napari_mcp/cli/install/cline_vscode.py (1)
1-20: LGTM! Proper initialization and imports.The file correctly uses modern type hints (dict[str, Any]) and follows the BaseInstaller pattern.
Note: The deprecated
typing.Dictissue from past reviews has been resolved.
🧹 Nitpick comments (17)
docs/integrations/other-llms.md (1)
1-248: LGTM! Comprehensive documentation for Gemini CLI and Codex CLI.This documentation provides thorough coverage of both platforms including:
- Quick setup with the new CLI installer
- Configuration file locations and formats
- Manual configuration examples (JSON for Gemini, TOML for Codex)
- Management commands and troubleshooting
- Security considerations and platform comparison
The content aligns well with the PR's CLI installer framework and provides users with clear, actionable guidance.
Minor suggestion: Line 208 uses lowercase "maturity" in the comparison table header, while other headers use title case. Consider capitalizing to "Maturity" for consistency.
docs/integrations/index.md (1)
137-161: Fix indented code blocks to use fenced format.The troubleshooting section uses indented code blocks, which triggers markdown linting warnings. For consistency with the rest of the documentation and better rendering, these should use fenced code blocks.
Apply this pattern to the code blocks at lines 139, 148, and 160:
- ```bash - # Reinstall package - pip install --force-reinstall napari-mcp +```bash +# Reinstall package +pip install --force-reinstall napari-mcp - # Verify - napari-mcp-install --version - ``` +# Verify +napari-mcp-install --version +```Based on static analysis hints.
docs/examples/direct_mcp_client.py (2)
12-20: Consider documenting the env parameter and server command choice.The example uses
env=Noneand hard-codes"python"as the command. For a documentation example:
- Consider adding a comment explaining that
env=Noneinherits the current environment- Consider mentioning that
"python"should match the environment where napari-mcp is installed, or suggest usingsys.executableExample improvement:
async def napari_workflow(): """Example automated napari workflow.""" + # Start napari MCP server via stdio + # command should point to the Python with napari-mcp installed server_params = StdioServerParameters( command="python", args=["-m", "napari_mcp.server"], - env=None + env=None # Inherit current environment )
26-44: Consider adding basic error handling for demonstration purposes.While this is an example script, adding basic error handling would make it more robust and educational for users. Tool calls can fail for various reasons (server issues, invalid arguments, etc.).
Consider wrapping tool calls with try-except:
try: await session.call_tool("execute_code", arguments={"code": code}) print("✓ Created test image") except Exception as e: print(f"✗ Failed to create image: {e}") returnThis would demonstrate best practices for production workflows while keeping the example simple.
docs/examples/anthropic_integration.py (2)
21-25: Consider documenting the Python command assumption.The server uses
command="python", which assumespythonresolves to the correct environment. Users with multiple Python installations may encounter issues. Consider adding a comment or referencing the environment setup in the docstring.
62-62: Inconsistent output format with OpenAI example.This example prints
type(result.content[0])while the OpenAI example (openai_integration.py:72) printsresult.contentdirectly. Consider aligning the output format for consistency across examples.Apply this diff for consistency:
- print(f"Tool result of type {type(result.content[0])}") + print(f"Tool result: {result.content}")docs/integrations/cline.md (1)
60-98: Add language identifiers to example code blocks.The example command blocks (lines 60-98) lack language identifiers, which affects syntax highlighting and accessibility. Consider adding language hints like
bashortextfor user prompts.For example:
-``` +```text Can you call session_information() to show the napari session? ```docs/examples/openai_integration.py (3)
16-16: Consider moving import to module level.The
import osstatement at line 16 is inside the function. While functional, it's more conventional to place imports at the module level for consistency with other imports (lines 6-10).Apply this diff:
import asyncio +import os from mcp import ClientSession, StdioServerParameters from mcp.client.stdio import stdio_client from openai import OpenAI async def main(): """Main function demonstrating OpenAI + napari MCP integration.""" # Initialize OpenAI client - import os api_key = os.getenv("OPENAI_API_KEY", "your-api-key-here")
69-69: Consider moving import to module level.The
import jsonat line 69 is inside a conditional block. For consistency and clarity, consider moving it to the module level with other imports.
21-25: Consider documenting the Python command assumption.The server uses
command="python", which assumespythonresolves to the correct environment. Users with multiple Python installations may encounter issues. Consider adding a comment or referencing the environment setup in the docstring.docs/getting-started/quickstart.md (1)
70-70: Add language identifier to fenced code block.The code block at line 70 is missing a language identifier for proper syntax highlighting.
Apply this diff:
- === "Basic Connection" + === "Basic Connection" ``` Can you call session_information() to tell me about my napari session? ```Based on the static analysis hint, though this appears to be example text rather than code, so the current format may be intentional.
src/napari_mcp/cli/install/cursor.py (1)
12-12: Consider reusing the console from base.py.A module-level
console = Console()is declared here, butBaseInstalleralready imports and uses a console from the same rich library. Creating multiple Console instances across installer modules may lead to inconsistent output formatting or duplicate handlers.Consider importing the console from base.py instead:
from .base import BaseInstaller +from .base import console from .utils import expand_path - -console = Console()This would centralize console configuration and ensure consistent styling across all installers.
README.md (2)
32-34: Add language identifiers to fenced code blocks.Several fenced code blocks lack language identifiers, which causes markdown lint warnings. While these blocks contain natural language prompts rather than code, adding
textorplaintextidentifiers would satisfy the linter and make the blocks more semantically explicit.Based on learnings (markdown best practices)
Apply language identifiers:
-``` +```text "Can you call session_information() to show my napari session details?"Repeat for blocks at lines 41-45, 48-55, and 58-62. Also applies to: 41-55, 58-62 --- `97-104`: **Convert indented code block to fenced format.** Line 100 uses an indented code block style, which is flagged by the markdown linter. For consistency with the rest of the document, use fenced code blocks. The markdown around line 97-104 uses an admonition/warning block. Ensure the code example inside uses fenced style: ```diff !!! warning "Code Execution Capabilities" This server includes powerful tools that allow arbitrary code execution: - - **`execute_code()`** - Runs Python code in the server environment + - `execute_code()` - Runs Python code in the server environmentdocs/getting-started/zero-install.md (1)
287-295: Use fenced code block for consistency.Line 293 uses an indented code block, flagged by the markdown linter. For consistency with the rest of the document, convert to a fenced code block.
- ```bash - uv cache clean +```bash +uv cache clean - # Then try again - uv run --with napari-mcp napari-mcp - ``` +# Then try again +uv run --with napari-mcp napari-mcp +```src/napari_mcp/cli/main.py (2)
52-321: Consider refactoring to reduce parameter duplication.The individual installer commands share significant parameter duplication (persistent, python_path, force, backup, dry_run). While the current explicit approach is clear and functional, you could reduce repetition by extracting common parameters into a shared set.
Example approach:
from typing import TypeVar from dataclasses import dataclass @dataclass class CommonInstallerOptions: persistent: bool = False python_path: str | None = None force: bool = False backup: bool = True dry_run: bool = False # Define shared annotations once PersistentOption = Annotated[bool, typer.Option("--persistent", help="Use Python path instead of uv run")] PythonPathOption = Annotated[str | None, typer.Option("--python-path", help="Custom Python executable path")] ForceOption = Annotated[bool, typer.Option("--force", "-f", help="Skip prompts and force update")] BackupOption = Annotated[bool, typer.Option("--backup/--no-backup", help="Create backup before updating")] DryRunOption = Annotated[bool, typer.Option("--dry-run", help="Preview changes without applying")]Then simplify each command:
@app.command("claude-desktop") def install_claude_desktop( persistent: PersistentOption = False, python_path: PythonPathOption = None, force: ForceOption = False, backup: BackupOption = True, dry_run: DryRunOption = False, ): """Install napari-mcp for Claude Desktop.""" installer = ClaudeDesktopInstaller( persistent=persistent, python_path=python_path, force=force, backup=backup, dry_run=dry_run, ) success, message = installer.install() if not success: raise typer.Exit(1)
436-492: Consider extracting installer creation logic.The installer creation logic (with special handling for cursor/gemini) is duplicated between the "all" branch and the single-app branch. Extracting this into a helper function would reduce repetition.
Example:
def _create_installer(app_key: str, installer_class, force: bool, backup: bool, dry_run: bool): """Create installer instance with appropriate options.""" if app_key in ["cursor", "gemini"]: return installer_class( force=force, backup=backup, dry_run=dry_run, global_install=True, ) else: return installer_class( force=force, backup=backup, dry_run=dry_run, )Then use in both branches:
installer = _create_installer(app_key, installer_class, force, backup, dry_run)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
LLM_INTEGRATIONS.md(0 hunks)README.md(1 hunks)docs/examples/README.md(1 hunks)docs/examples/anthropic_integration.py(1 hunks)docs/examples/direct_mcp_client.py(1 hunks)docs/examples/openai_integration.py(1 hunks)docs/getting-started/index.md(1 hunks)docs/getting-started/installation.md(1 hunks)docs/getting-started/quickstart.md(2 hunks)docs/getting-started/zero-install.md(1 hunks)docs/guides/index.md(3 hunks)docs/guides/troubleshooting.md(1 hunks)docs/index.md(4 hunks)docs/integrations/chatgpt.md(1 hunks)docs/integrations/claude-code.md(1 hunks)docs/integrations/claude-desktop.md(1 hunks)docs/integrations/cline.md(1 hunks)docs/integrations/cursor.md(1 hunks)docs/integrations/index.md(1 hunks)docs/integrations/other-llms.md(1 hunks)docs/integrations/python.md(1 hunks)docs/scripts/gen_ref_pages.py(7 hunks)mkdocs.yml(2 hunks)pyproject.toml(4 hunks)src/napari_mcp/cli/__init__.py(1 hunks)src/napari_mcp/cli/install/__init__.py(1 hunks)src/napari_mcp/cli/install/base.py(1 hunks)src/napari_mcp/cli/install/claude_code.py(1 hunks)src/napari_mcp/cli/install/claude_desktop.py(1 hunks)src/napari_mcp/cli/install/cline_cursor.py(1 hunks)src/napari_mcp/cli/install/cline_vscode.py(1 hunks)src/napari_mcp/cli/install/codex_cli.py(1 hunks)src/napari_mcp/cli/install/cursor.py(1 hunks)src/napari_mcp/cli/install/gemini_cli.py(1 hunks)src/napari_mcp/cli/install/utils.py(1 hunks)src/napari_mcp/cli/main.py(1 hunks)src/napari_mcp/server.py(2 hunks)tests/test_base_installer.py(1 hunks)tests/test_cli_installer.py(1 hunks)tests/test_cli_installers/test_platform_installers.py(1 hunks)tests/test_cli_utils.py(1 hunks)
💤 Files with no reviewable changes (1)
- LLM_INTEGRATIONS.md
✅ Files skipped from review due to trivial changes (3)
- docs/guides/troubleshooting.md
- docs/integrations/claude-code.md
- docs/examples/README.md
🚧 Files skipped from review as they are similar to previous changes (11)
- src/napari_mcp/cli/install/cline_cursor.py
- src/napari_mcp/cli/install/gemini_cli.py
- tests/test_cli_installer.py
- src/napari_mcp/cli/init.py
- src/napari_mcp/cli/install/codex_cli.py
- src/napari_mcp/cli/install/claude_desktop.py
- src/napari_mcp/cli/install/init.py
- tests/test_cli_installers/test_platform_installers.py
- tests/test_base_installer.py
- tests/test_cli_utils.py
- src/napari_mcp/cli/install/base.py
🧰 Additional context used
🧬 Code graph analysis (9)
src/napari_mcp/cli/install/cursor.py (3)
src/napari_mcp/cli/install/base.py (4)
BaseInstaller(23-236)get_config_path(65-72)get_extra_config(75-83)show_post_install_message(227-236)src/napari_mcp/cli/install/utils.py (1)
expand_path(36-53)src/napari_mcp/cli/install/gemini_cli.py (3)
get_config_path(62-91)get_extra_config(93-106)show_post_install_message(108-129)
docs/examples/anthropic_integration.py (2)
docs/examples/openai_integration.py (1)
main(13-72)src/napari_mcp/server.py (2)
main(2169-2175)run(2136-2138)
docs/examples/openai_integration.py (2)
docs/examples/anthropic_integration.py (1)
main(14-62)src/napari_mcp/server.py (2)
main(2169-2175)run(2136-2138)
src/napari_mcp/cli/install/cline_vscode.py (3)
src/napari_mcp/cli/install/base.py (4)
BaseInstaller(23-236)get_config_path(65-72)get_extra_config(75-83)show_post_install_message(227-236)src/napari_mcp/cli/install/utils.py (2)
expand_path(36-53)get_platform(19-33)src/napari_mcp/cli/install/cline_cursor.py (3)
get_config_path(21-42)get_extra_config(44-56)show_post_install_message(58-80)
src/napari_mcp/cli/install/claude_code.py (8)
src/napari_mcp/cli/install/base.py (3)
BaseInstaller(23-236)get_config_path(65-72)get_extra_config(75-83)src/napari_mcp/cli/install/utils.py (1)
expand_path(36-53)src/napari_mcp/cli/install/claude_desktop.py (2)
get_config_path(17-34)get_extra_config(36-44)src/napari_mcp/cli/install/cline_cursor.py (2)
get_config_path(21-42)get_extra_config(44-56)src/napari_mcp/cli/install/cline_vscode.py (2)
get_config_path(21-42)get_extra_config(44-56)src/napari_mcp/cli/install/codex_cli.py (2)
get_config_path(21-30)get_extra_config(32-40)src/napari_mcp/cli/install/cursor.py (2)
get_config_path(62-91)get_extra_config(93-101)src/napari_mcp/cli/install/gemini_cli.py (2)
get_config_path(62-91)get_extra_config(93-106)
src/napari_mcp/cli/install/utils.py (1)
src/napari_mcp/server.py (1)
run(2136-2138)
src/napari_mcp/server.py (3)
src/napari_mcp/cli/install/base.py (1)
install(107-173)src/napari_mcp/cli/install/codex_cli.py (1)
install(42-131)src/napari_mcp/cli/main.py (1)
main(38-49)
docs/examples/direct_mcp_client.py (1)
src/napari_mcp/server.py (1)
run(2136-2138)
src/napari_mcp/cli/main.py (9)
src/napari_mcp/cli/install/base.py (3)
install(107-173)uninstall(175-225)get_config_path(65-72)src/napari_mcp/cli/install/codex_cli.py (4)
install(42-131)CodexCLIInstaller(14-202)uninstall(133-187)get_config_path(21-30)src/napari_mcp/cli/install/claude_code.py (2)
ClaudeCodeInstaller(10-36)get_config_path(17-26)src/napari_mcp/cli/install/claude_desktop.py (2)
ClaudeDesktopInstaller(10-44)get_config_path(17-34)src/napari_mcp/cli/install/cline_cursor.py (2)
ClineCursorInstaller(14-80)get_config_path(21-42)src/napari_mcp/cli/install/cline_vscode.py (2)
ClineVSCodeInstaller(14-77)get_config_path(21-42)src/napari_mcp/cli/install/cursor.py (2)
CursorInstaller(15-111)get_config_path(62-91)src/napari_mcp/cli/install/gemini_cli.py (2)
GeminiCLIInstaller(15-129)get_config_path(62-91)src/napari_mcp/cli/install/utils.py (4)
get_app_display_name(328-351)show_installation_summary(245-264)check_existing_server(199-219)read_json_config(56-77)
🪛 GitHub Actions: Build and Deploy Documentation
mkdocs.yml
[error] 1-1: mkdocs build --clean --strict failed. Process completed with exit code 1 due to warnings being treated as errors in strict mode.
docs/integrations/python.md
[warning] 1-1: Doc file 'integrations/python.md' contains a link to 'troubleshooting.md', but the target 'integrations/troubleshooting.md' is not found among documentation files.
🪛 markdownlint-cli2 (0.18.1)
README.md
32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
58-58: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
100-100: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
docs/integrations/index.md
141-141: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
150-150: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
159-159: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
docs/integrations/cline.md
60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
64-64: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
72-72: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
76-76: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
82-82: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
86-86: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
96-96: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
192-192: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
221-221: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
236-236: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
docs/integrations/claude-desktop.md
58-58: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
72-72: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
82-82: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
86-86: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
90-90: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
96-96: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
100-100: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
104-104: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
110-110: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
118-118: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
192-192: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
220-220: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
229-229: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
247-247: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
docs/getting-started/quickstart.md
70-70: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
92-92: Spaces inside code span elements
(MD038, no-space-in-code)
137-137: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
183-183: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/getting-started/zero-install.md
293-293: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
docs/integrations/python.md
37-37: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
114-114: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
128-128: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
195-195: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
207-207: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
258-258: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
299-299: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
327-327: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
340-340: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
350-350: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
359-359: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
370-370: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
docs/integrations/cursor.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
49-49: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
61-61: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
67-67: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
71-71: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
81-81: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
159-159: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
182-182: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
201-201: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: smoke-tests
- GitHub Check: claude-review
- GitHub Check: smoke-tests
🔇 Additional comments (64)
docs/guides/index.md (3)
17-17: LGTM! Documentation enhancement aligns with PR objectives.The addition of the Python Integration quick link properly references the new Python integration documentation introduced in this PR.
34-34: LGTM! Consistent reference to new Python examples.The update to reference Python scripts and examples aligns with the new CLI installer system and Python integration documentation.
87-87: LGTM! Appropriate cross-reference for automated analysis.The link to Python examples in the Data Science & AI section provides users with relevant workflow examples for automated processing pipelines.
docs/integrations/chatgpt.md (1)
1-209: LGTM! Comprehensive documentation addressing ChatGPT limitations.This documentation effectively:
- Clarifies that ChatGPT Desktop/Web lack MCP support
- Distinguishes OpenAI's different products and their MCP capabilities
- Provides clear alternatives (Codex CLI, Cursor, Claude Desktop, OpenAI Python API)
- Includes practical setup commands using the new
napari-mcp-installCLI- Offers comparison tables and troubleshooting guidance
The content is well-structured, accurate, and aligns perfectly with the PR's CLI installer system.
docs/integrations/index.md (1)
3-178: LGTM! Excellent documentation restructure for CLI installer workflow.The updates successfully transform the documentation to emphasize the new automated CLI installer system. Key improvements:
- Clear Quick Installation section with consistent
napari-mcp-installcommands- Comprehensive platform support matrix including new Gemini/Codex targets
- Well-structured installation workflow with Step 1-3
- Practical installation options and management commands
- Platform-specific guide links properly organized
The content aligns perfectly with the PR's CLI installer system and provides excellent user guidance.
docs/examples/direct_mcp_client.py (1)
46-47: LGTM! Clean script entry point.The
if __name__ == "__main__"guard withasyncio.run()is the correct pattern for async entry points.docs/integrations/python.md (1)
1-421: Excellent comprehensive documentation!This Python integration guide provides clear, practical examples for multiple LLM providers with proper error handling, resource management, and security warnings. The structure is well-organized and the examples are runnable.
docs/integrations/cline.md (1)
1-260: Well-structured integration guide.The Cline documentation provides comprehensive setup instructions for both VS Code and Cursor, with clear troubleshooting steps and security considerations.
mkdocs.yml (2)
107-111: Navigation structure properly reflects new documentation.The addition of Cline, Other LLMs, and the Advanced subsection with Python Scripts properly integrates the new documentation files into the site navigation.
147-147: Copyright year updated appropriately.The copyright year has been updated to 2025, which is consistent with the PR creation date.
docs/integrations/cursor.md (1)
1-224: Documentation looks comprehensive and well-structured.The Cursor integration guide provides clear setup instructions, configuration options, troubleshooting steps, and practical examples. The static analysis warnings about missing language specifiers on code blocks (lines 45-83) are false positives—these are natural language prompts meant to be typed into Cursor's chat interface, not code to be executed. Similarly, the indented code blocks within admonitions (lines 159, 182, 201) follow standard markdown convention for content inside callout boxes.
docs/integrations/claude-desktop.md (1)
1-271: Excellent Claude Desktop integration guide.The documentation is thorough, covering automated installation, platform-specific paths, testing procedures, example workflows, and comprehensive troubleshooting. Like the Cursor guide, the static analysis warnings are false positives—the "code blocks" at lines 58-120 are natural language prompts for Claude's chat interface, and the indented blocks within admonitions follow standard markdown conventions.
pyproject.toml (3)
35-36: Appropriate CLI dependencies added.Typer and Rich are excellent choices for building a modern CLI installer. Typer provides a clean API for command definitions, and Rich enables the colored terminal output and tables shown throughout the documentation.
65-65: Type stub for TOML support added.The
types-tomlpackage provides type hints for TOML parsing, which aligns with the Codex CLI installer's TOML-based configuration format mentioned in the PR objectives.Also applies to: 218-218
75-75: New CLI entry point correctly configured.The
napari-mcp-installconsole script maps tonapari_mcp.cli.main:app, which will invoke the Typer app object. This follows Typer's standard entry point pattern.docs/getting-started/index.md (1)
1-95: Well-organized getting started overview.The updated documentation effectively introduces the CLI installer workflow and provides clear guidance for different user paths. The supported applications table (lines 68-77) aligns with the PR objectives and gives users a complete overview of installation options. The timing estimates and success indicators help set appropriate expectations.
src/napari_mcp/cli/install/utils.py (11)
19-34: Platform detection looks correct.The function properly maps Darwin to "macos" and handles the three major platforms. The logic is straightforward and appropriate.
36-54: Path expansion handles user and environment variables.The function correctly expands both
~(user home) and environment variables like%APPDATA%, then resolves to an absolute path. This is essential for cross-platform config path resolution.
56-78: JSON config reader with proper error handling.The function correctly uses
OrderedDictto preserve key order (important for config files) and returns an empty dict on errors rather than raising exceptions. Error messages are logged to the console for user visibility.
80-123: Atomic config writes with backup support.The implementation follows best practices:
- Creates parent directories as needed
- Backs up existing files before overwriting
- Uses atomic write-then-rename to prevent corruption
- Cleans up temp files on error
- Adds trailing newline for better git diffs
125-157: Python executable resolution supports multiple modes.The function correctly handles three modes:
- Custom Python path (with existence check and warning)
- Persistent mode using
sys.executable- Default uv-based ephemeral mode
Returns both the command and a user-friendly description.
159-197: Server config builder handles uv and persistent modes.The function constructs the appropriate MCP server configuration:
- uv mode:
["run", "--with", "napari-mcp", "napari-mcp"]- Persistent mode:
["-m", "napari_mcp.server"]- Supports extra_args for app-specific fields (e.g., Gemini CLI)
199-243: User prompts for existing installations.The functions check for existing server configurations and prompt users before overwriting. The prompt defaults to
Falseto prevent accidental overwrites, which is a safe choice.
245-265: Rich table for installation summaries.The summary table provides clear visual feedback with colored status indicators (✓/✗) and per-app messages. Good UX for multi-app installations.
267-304: Python environment validation with subprocess.The function validates that
napari-mcpcan be imported in the target Python environment. Includes timeout protection and clear error messages. However, consider that the subprocess check runs synchronously—for the CLI use case this is fine, but be aware it could block briefly.
306-326: Environment type detection covers common cases.The function detects conda, venv, and system Python environments. The detection logic is standard (checking
CONDA_DEFAULT_ENV,sys.real_prefix, andsys.base_prefix). This helps provide context-aware messaging to users.
328-352: App display name mapping is complete.The mapping covers all seven supported applications mentioned in the PR objectives. Note that both "codex" and "codex-cli" map to "Codex CLI", which provides flexibility in how users reference the app.
docs/getting-started/quickstart.md (3)
92-96: LGTM! Example commands are clear and useful.The test commands and expected responses provide excellent guidance for verifying the installation.
103-141: Comprehensive examples cover key use cases effectively.The Basic Operations and Advanced Features sections provide excellent practical examples for users to get started.
197-233: Troubleshooting section addresses common failure modes well.The common issues section covers installation verification, configuration checks, environment issues, and permissions problems with actionable solutions.
src/napari_mcp/cli/install/cline_vscode.py (3)
44-56: LGTM! Extra configuration fields match Cline's requirements.The
alwaysAllowanddisabledfields are consistent with the cline_cursor.py implementation and appropriate for Cline's tool permission model.
58-77: Excellent post-install guidance with VS Code Insiders note.The post-install message provides clear next steps and includes a helpful note about VS Code Insiders using a different path, which aids users running the Insiders build.
21-42: Approve platform-specific config paths: Verified correct path patterns and extension ID usage across code and tests.docs/scripts/gen_ref_pages.py (3)
45-88: Excellent documentation structure enhancement.The dual-layer documentation approach clearly separates:
- MCP Tool Interface - What users call from AI assistants
- Implementation - The underlying NapariMCPTools class methods
This improves clarity for both end-users and developers. The addition of
show_source: truehelps users understand the implementation.
109-112: Update utilities description to match new capabilities.The description correctly mentions "timelapse capture" and "output retrieval" which align with the new functions added.
169-176: Verify the total tool count is accurate.The overview table shows:
- Session: 4 functions
- Layers: 8 functions
- Navigation: 5 functions
- Utilities: 5 functions
- Total: 22 MCP tools
Simple math: 4 + 8 + 5 + 5 = 22 ✓
docs/index.md (6)
22-28: LGTM! AI Integration section accurately reflects supported applications.The expanded list correctly includes:
- Claude Desktop & Claude Code
- Cursor IDE
- Cline extensions (VS Code & Cursor)
- Gemini CLI & Codex CLI support
This aligns with the installer implementations added in the PR.
35-40: Good addition of Workflow Automation feature.The new bullet point appropriately highlights the Python automation capabilities with a reference to examples, which is an important use case.
44-74: Excellent Quick Start rewrite for CLI installer workflow.The 3-step flow is clear and concise:
- Install package with pip
- Auto-configure with napari-mcp-install
- Restart and test
This significantly improves the onboarding experience compared to manual configuration.
116-129: Supported Applications table is comprehensive and actionable.The table clearly lists all supported applications with their installation commands and status. This provides an excellent quick reference for users.
163-172: Well-structured navigation paths for different user needs.The revised "Choose Your Path" section effectively guides users to:
- Quick Start for immediate setup
- Installation Options for advanced config
- Integrations for app-specific guides
- Python Automation for programmatic use
- Troubleshooting for issues
104-111: Tools table validation passed
All functions listed in docs/index.md were confirmed present in docs/scripts/gen_ref_pages.py—no discrepancies found.src/napari_mcp/cli/install/cursor.py (4)
18-60: LGTM!The initialization correctly extends the base class with project-scoped configuration support. The pattern matches other project-aware installers (Gemini CLI, Codex CLI) and properly delegates to the base constructor.
62-91: LGTM!The configuration path resolution correctly handles both global and project-specific modes. The interactive confirmation for project installs provides good user experience, and the exception handling integrates properly with the base installer's error flow.
93-101: LGTM!Returning an empty dict is correct for Cursor, which uses the standard MCP server configuration without additional fields.
103-111: LGTM!The post-install messaging appropriately extends the base implementation with project-specific guidance. This helps users understand the scope of their installation.
src/napari_mcp/server.py (5)
34-34: LGTM!The typer import is appropriate for the new CLI functionality being added.
2128-2132: LGTM!The Typer app configuration is clean and appropriate. Disabling completion avoids unnecessary complexity for this use case.
2135-2138: LGTM!The run command provides a clear, explicit way to start the server via CLI.
2141-2166: Clear deprecation messaging with helpful guidance.The install command stub provides excellent user experience by explaining the deprecation and directing users to the new CLI installer with concrete examples. Using exit code 1 appropriately signals that the old command should not be used.
2169-2176: LGTM!The main entry point provides excellent backward compatibility by defaulting to server.run() when no arguments are provided, while still supporting the new CLI commands when arguments are present. This ensures existing usage patterns continue to work.
README.md (1)
69-114: LGTM!The supported applications table and documentation structure provide clear guidance for users across multiple LLM applications. The security notice is appropriately concise while highlighting the key risks.
docs/getting-started/installation.md (3)
25-59: LGTM!The configuration file locations are accurate and comprehensive. The platform-specific paths align with the actual installer implementations, and the notes about variants (VS Code Insiders) and scope (global/project) are helpful.
60-144: LGTM!The configuration examples are accurate and comprehensive. The zero-install (uv) and persistent modes are clearly differentiated, and application-specific configurations correctly show extended features (permissions, trust settings, TOML format).
145-386: LGTM!The remaining sections provide comprehensive coverage of advanced installation scenarios. The examples are correct, the troubleshooting guidance is practical, and the environment variable configurations demonstrate proper integration with the installer system.
docs/getting-started/zero-install.md (3)
1-33: LGTM!The zero-install concept is clearly explained with accurate benefits and execution flow. The ephemeral, cache-based execution model is well-described for users.
34-196: LGTM!The setup instructions are comprehensive and accurate. The progression from automated to manual to advanced usage provides clear guidance for users at different skill levels, and all commands and configurations are correct.
197-380: LGTM!The comparison table, troubleshooting guidance, and advanced configuration sections are excellent. The comparison is fair, the troubleshooting addresses common issues with practical solutions, and the environment variable examples show proper integration with the MCP configuration system.
src/napari_mcp/cli/main.py (6)
1-26: LGTM!The imports are well-organized, and the Typer app initialization is correctly configured with appropriate settings.
28-34: LGTM!The version callback follows Typer conventions and correctly displays version information.
37-49: LGTM!The main callback is properly configured with eager evaluation for the version option, following Typer best practices.
324-401: LGTM!The install-all command correctly handles per-app installations with appropriate special cases for project-specific installers (Cursor and Gemini), proper exception handling, and summary reporting.
501-585: LGTM!The list command correctly checks installation status across all apps, handles different config formats (TOML for Codex, JSON for others), and provides clear status indicators with appropriate exception handling.
588-589: LGTM!Standard CLI entry point guard correctly implemented.
| async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session: | ||
| # Initialize the connection |
There was a problem hiding this comment.
Fix indentation inconsistency.
The indentation at line 28 is excessive—it should align with the async context manager on line 27.
Apply this diff:
async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session:
- # Initialize the connection
- await session.initialize()
+ # Initialize the connection
+ await session.initialize()Note: This indentation issue affects all subsequent lines within the context manager (lines 28-62). They should all be dedented by one level.
📝 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.
| async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session: | |
| # Initialize the connection | |
| async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session: | |
| # Initialize the connection | |
| await session.initialize() | |
| # …and similarly dedent all subsequent lines in this block by one level… |
🤖 Prompt for AI Agents
In docs/examples/anthropic_integration.py around lines 27 to 28, the block
inside the async context manager is over-indented; line 28 and all subsequent
lines within that context (through line 62) should be dedented one level so they
align under the "async with stdio_client(...)" context manager. Update the
indentation for lines 28–62 accordingly so the entire inner block is shifted
left one indent level, preserving relative indentation inside that block.
| async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session: | ||
| # Initialize | ||
| await session.initialize() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix indentation inconsistency.
Line 22-24 use 12 spaces of indentation, while the rest of the file uses 4-space indentation. This should be corrected for consistency.
Apply this diff:
async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session:
- # Initialize
- await session.initialize()
-
+ # Initialize
+ await session.initialize()
+📝 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.
| async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session: | |
| # Initialize | |
| await session.initialize() | |
| async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session: | |
| # Initialize | |
| await session.initialize() | |
🤖 Prompt for AI Agents
In docs/examples/direct_mcp_client.py around lines 21 to 24, the block showing
"async with stdio_client(server_params) as (read, write), ClientSession(read,
write) as session:" is indented with 12 spaces while the file uses 4-space
indentation; fix by reducing the leading indentation of lines 22–24 to match the
file's 4-space style so the async with and its inner await/session lines align
with the rest of the file.
| async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session: | ||
| # Initialize the connection |
There was a problem hiding this comment.
Fix indentation inconsistency.
The indentation at line 28 is excessive—it should align with the async context manager on line 27.
Apply this diff:
async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session:
- # Initialize the connection
- await session.initialize()
+ # Initialize the connection
+ await session.initialize()Note: This indentation issue affects all subsequent lines within the context manager (lines 28-72). They should all be dedented by one level.
📝 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.
| async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session: | |
| # Initialize the connection | |
| async with stdio_client(server_params) as (read, write), ClientSession(read, write) as session: | |
| # Initialize the connection | |
| await session.initialize() |
🤖 Prompt for AI Agents
In docs/examples/openai_integration.py around lines 27 to 72, the body of the
async context manager is over-indented: line 28 and all subsequent lines inside
the context (through line 72) should be dedented one level so they align
directly under the async with clause on line 27; update indentation for lines
28–72 accordingly so the content is nested exactly once under "async with
stdio_client(... ) as (read, write), ClientSession(read, write) as session:" and
maintain consistent 4-space (or project) indentation for the block.
| "napari-mcp": { | ||
| "command": "python", | ||
| "args": ["-m", "napari_mcp.server"], | ||
| "always Allow": [], |
There was a problem hiding this comment.
Fix typo in JSON property name.
The property name "always Allow" (with a space) should be "alwaysAllow" (camelCase). This typo will cause the configuration to be invalid.
Apply this diff:
"args": ["-m", "napari_mcp.server"],
- "always Allow": [],
+ "alwaysAllow": [],
"disabled": false📝 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.
| "always Allow": [], | |
| "args": ["-m", "napari_mcp.server"], | |
| "alwaysAllow": [], | |
| "disabled": false |
🤖 Prompt for AI Agents
In docs/integrations/cline.md around line 144, the JSON property "always Allow"
contains a space and is invalid; change the key to "alwaysAllow" (camelCase) so
the JSON config is valid and consistent with expected property naming.
| console.print(f"[red]Unknown application: {app_name}[/red]") | ||
| console.print( | ||
| "Available: claude-desktop, claude-code, cursor, cline-vscode, cline-cursor, gemini, codex, all" | ||
| ) |
There was a problem hiding this comment.
Fix inconsistent app name in error message.
The error message mentions "cline" but the actual app names in app_map are "cline-vscode" and "cline-cursor". Users seeing this message might try "cline" and get the error again.
Apply this diff:
else:
console.print(f"[red]Unknown application: {app_name}[/red]")
console.print(
- "Available: claude-desktop, claude-code, cursor, cline-vscode, cline-cursor, gemini, codex, all"
+ "Available: " + ", ".join(list(app_map.keys()) + ["all"])
)
raise typer.Exit(1)📝 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.
| console.print(f"[red]Unknown application: {app_name}[/red]") | |
| console.print( | |
| "Available: claude-desktop, claude-code, cursor, cline-vscode, cline-cursor, gemini, codex, all" | |
| ) | |
| else: | |
| console.print(f"[red]Unknown application: {app_name}[/red]") | |
| console.print( | |
| "Available: " + ", ".join(list(app_map.keys()) + ["all"]) | |
| ) | |
| raise typer.Exit(1) |
🤖 Prompt for AI Agents
In src/napari_mcp/cli/main.py around lines 494 to 497, the "Available" error
message lists "cline" which is inconsistent with the actual keys in app_map
("cline-vscode" and "cline-cursor"); update the printed available apps to
exactly match app_map (e.g., "claude-desktop, claude-code, cursor, cline-vscode,
cline-cursor, gemini, codex, all") so users are shown valid app names.
… broken doc link - Add toml>=0.10.2 to dependencies for Codex CLI TOML config support - Fix test_build_server_config_python to handle version-specific python binaries (e.g., python3.12) - Fix broken documentation link in python.md (troubleshooting.md -> ../guides/troubleshooting.md) Fixes 4 test failures across all platforms (Ubuntu/macOS/Windows × Python 3.10-3.13) and documentation build failure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
✅ Documentation builds successfully! The site will be updated when this PR is merged. |
🔍 Pull Request Review: LLM Auto-Installation CLI System📊 OverviewThis is a substantial and well-structured PR that adds a comprehensive CLI installer system for napari-mcp across multiple LLM applications. The implementation demonstrates solid engineering practices with good architecture, comprehensive testing, and attention to security. Key Statistics:
✅ Strengths🏗️ Excellent Architecture Design
🧪 Comprehensive Test Coverage
🔒 Security Considerations
📚 Documentation & UX
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/integrations/python.md (3)
55-59: Consider more robust server parameter defaults.The hardcoded
command="python"may not resolve correctly in all environments (virtual envs, conda, etc.). Additionally,env=Nonemeans the subprocess inherits no environment variables, which could cause issues with Python path resolution or Qt platform plugins.Consider this more portable approach:
+import sys +import os + # Start napari MCP server server_params = StdioServerParameters( - command="python", + command=sys.executable, args=["-m", "napari_mcp.server"], - env=None + env=os.environ.copy() )This ensures the same Python interpreter is used and environment variables are properly inherited.
146-150: Consider more robust server parameter defaults.Same issue as the OpenAI example: hardcoded
command="python"andenv=Nonemay cause portability issues.Apply the same fix:
+import sys +import os + # Start napari MCP server server_params = StdioServerParameters( - command="python", + command=sys.executable, args=["-m", "napari_mcp.server"], - env=None + env=os.environ.copy() )
221-225: Consider more robust server parameter defaults.Same portability issue with
command="python"andenv=None.Apply the same fix as previous examples:
+import sys +import os + # Start napari MCP server server_params = StdioServerParameters( - command="python", + command=sys.executable, args=["-m", "napari_mcp.server"], - env=None + env=os.environ.copy() )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
docs/integrations/python.md(1 hunks)pyproject.toml(4 hunks)tests/test_cli_utils.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_cli_utils.py
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/integrations/python.md
37-37: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
114-114: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
128-128: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
195-195: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
207-207: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
258-258: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
299-299: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
327-327: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
340-340: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
350-350: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
359-359: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
370-370: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: smoke-tests
- GitHub Check: claude-review
- GitHub Check: smoke-tests
🔇 Additional comments (5)
docs/integrations/python.md (2)
370-382: Good practice demonstrated here!This section correctly shows how to properly handle environment variables by spreading
os.environand adding custom overrides. This is the pattern that should be used in the earlier examples (lines 55-59, 146-150, 221-225) instead ofenv=None.
416-416: Past review comment has been addressed.The broken link to troubleshooting.md has been fixed. The path now correctly points to
../guides/troubleshooting.md, which matches the mkdocs.yml navigation structure.pyproject.toml (3)
66-66: LGTM!Adding
types-tomltype stubs is appropriate for mypy type checking support when introducing thetomlruntime dependency.
216-220: LGTM!The multiline formatting improves readability, and including
types-tomlin the dev dependency group is consistent with its addition to[project.optional-dependencies].
76-76: Console script entry pointnapari-mcp-installis correctly defined. The modulenapari_mcp.cli.mainexists and exportsapp = typer.Typer(...)as expected.
| "typer>=0.9.0", | ||
| "rich>=13.0.0", | ||
| "toml>=0.10.2", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify latest versions and security status of new dependencies.
The specified minimum versions for the new dependencies appear dated (typer 0.9.0, rich 13.0.0, toml 0.10.2). Since these are new runtime dependencies being added, ensure they are up-to-date and free from known vulnerabilities.
Run the following script to check the latest versions and security advisories:
🏁 Script executed:
#!/bin/bash
# Check latest versions and security advisories for new dependencies
echo "=== Checking PyPI for latest versions ==="
for pkg in typer rich toml; do
echo -n "$pkg latest: "
curl -s "https://pypi.org/pypi/$pkg/json" | jq -r '.info.version'
done
echo -e "\n=== Checking for security advisories ==="
for pkg in typer rich toml; do
echo "--- $pkg ---"
gh api graphql -f query="
{
securityVulnerabilities(first: 5, ecosystem: PIP, package: \"$pkg\") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}"
doneLength of output: 332
Bump version constraints for new dependencies
- Update typer to “typer>=0.19.2”
- Update rich to “rich>=14.1.0”
- Leave toml at “toml>=0.10.2” (current latest)
- No known security advisories for these packages
🤖 Prompt for AI Agents
In pyproject.toml around lines 35 to 37, the dependency version constraints need
updating: change "typer>=0.9.0" to "typer>=0.19.2" and "rich>=13.0.0" to
"rich>=14.1.0", leaving "toml>=0.10.2" unchanged; update the lines accordingly
and run your dependency tool (poetry/pip) to refresh lockfile/installed packages
as needed.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #34 +/- ##
==========================================
+ Coverage 77.35% 78.40% +1.05%
==========================================
Files 4 16 +12
Lines 1170 1792 +622
Branches 145 230 +85
==========================================
+ Hits 905 1405 +500
- Misses 195 280 +85
- Partials 70 107 +37
🚀 New features to boost your workflow:
|
Use path.as_posix() instead of str(path) for cross-platform path assertions. On Windows, str(path) returns paths with backslashes (\), causing test failures when checking for Unix-style paths with forward slashes (/). Fixes all Windows test failures: - test_config_path_macos, test_config_path_linux (Claude Desktop) - test_global_config_path, test_project_config_path (Cursor, Gemini) - test_cline_vscode_path_macos, test_cline_cursor_path_macos (Cline) - test_config_path (Codex) - test_cursor_installer_path_expansion, test_gemini_installer_path_expansion - test_expand_path_home, test_get_python_executable_nonexistent_custom - test_expand_path_with_multiple_vars 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
✅ Documentation builds successfully! The site will be updated when this PR is merged. |
Summary
Adds a comprehensive CLI installer system for napari-mcp that automates configuration across multiple LLM applications and IDEs.
Features
🎯 Multi-Platform Support
🛠 Installation Commands
✨ Key Features
📁 Architecture
Documentation
Test Coverage
Comprehensive test suite added:
tests/test_base_installer.py- BaseInstaller functionalitytests/test_cli_installer.py- CLI command testingtests/test_cli_installers/test_platform_installers.py- Platform-specific installerstests/test_cli_utils.py- Utility functionsAll tests include:
Example Usage
Install for Claude Desktop:
Install for all applications:
Project-specific Cursor installation:
Dry-run to preview changes:
List current installations:
Breaking Changes
None - This is a new feature addition that doesn't modify existing functionality.
Dependencies
Added CLI dependencies:
typer- CLI frameworkrich- Terminal formattingtoml- TOML config support (for Codex)🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
Documentation
Tests