Enhancement/browser tools#147
Conversation
Co-authored-by: MarcSkovMadsen <42288570+MarcSkovMadsen@users.noreply.github.com>
…eenshot parameter Co-authored-by: MarcSkovMadsen <42288570+MarcSkovMadsen@users.noreply.github.com>
Co-authored-by: MarcSkovMadsen <42288570+MarcSkovMadsen@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR enhances the Panel MCP server's screenshot tool by renaming it to panel_inspect_app and extending it to capture both screenshots and browser console logs in a single operation, making it easier to debug JavaScript errors in Panel apps (especially custom components). Additionally, it adds two new Panel extension repositories (panel-live and panel-reactflow) to the built-in documentation repositories.
Changes:
- Renamed
panel_take_screenshottopanel_inspect_appwith enhanced functionality to capture both screenshots and browser console logs - Added support for filtering console logs by level (error, warning, log, etc.) and saving screenshots to disk with configurable paths
- Added
panel-liveandpanel-reactflowas built-in documentation repositories in the configuration
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/holoviz_mcp/panel_mcp/server.py | Renamed tool from take_screenshot to inspect_app, added console log capture via Playwright event handlers, and implemented screenshot saving with configurable paths |
| src/holoviz_mcp/panel_mcp/models.py | Added ConsoleLogEntry Pydantic model to represent browser console log entries with level, message, and timestamp fields |
| src/holoviz_mcp/config/models.py | Added screenshots_dir field to ServerConfig for configuring the default screenshot save location |
| src/holoviz_mcp/config/config.yaml | Added panel-live and panel-reactflow repository configurations following the same pattern as other HoloViz documentation repositories |
| tests/test_panel_mcp.py | Updated test suite with comprehensive coverage for the renamed tool including tests for screenshot-only, logs-only, log filtering, error handling, and screenshot saving functionality |
| docs/explanation/tools.md | Updated documentation to reflect the new tool name, parameters, and use cases with detailed examples |
| CLAUDE.md | Added file containing reference to AGENTS.md (not mentioned in PR description) |
| import holoviz_mcp.panel_mcp.server as _srv | ||
|
|
||
| if _srv._playwright_manager is not None: | ||
| await _srv._playwright_manager.close() | ||
| _srv._playwright_manager = None |
There was a problem hiding this comment.
The import statement import holoviz_mcp.panel_mcp.server as _srv should be placed at the beginning of the test function before resetting the PlaywrightManager, for consistency with other test functions that follow this pattern (see test_inspect_app_screenshot_only, test_inspect_app_console_logs_only, test_inspect_app_log_level_filter). This ensures the module is imported before attempting to access its attributes.
| import holoviz_mcp.panel_mcp.server as _srv | ||
|
|
||
| if _srv._playwright_manager is not None: | ||
| await _srv._playwright_manager.close() | ||
| _srv._playwright_manager = None |
There was a problem hiding this comment.
The import statement import holoviz_mcp.panel_mcp.server as _srv should be placed at the beginning of the test function before resetting the PlaywrightManager, for consistency with other test functions that follow this pattern (see test_inspect_app_screenshot_only, test_inspect_app_console_logs_only, test_inspect_app_log_level_filter). This ensures the module is imported before attempting to access its attributes.
| import holoviz_mcp.panel_mcp.server as _srv | ||
|
|
||
| image_content = result.content[0] | ||
| if _srv._playwright_manager is not None: | ||
| await _srv._playwright_manager.close() | ||
| _srv._playwright_manager = None |
There was a problem hiding this comment.
The import statement import holoviz_mcp.panel_mcp.server as _srv should be placed at the beginning of the test function before resetting the PlaywrightManager, for consistency with other test functions that follow this pattern (see test_inspect_app_screenshot_only, test_inspect_app_console_logs_only, test_inspect_app_log_level_filter). This ensures the module is imported before attempting to access its attributes.
| @@ -0,0 +1 @@ | |||
| @AGENTS.md | |||
There was a problem hiding this comment.
The CLAUDE.md file addition is not mentioned in the PR description or test plan, and appears unrelated to the "Enhancement/browser tools" changes. If this file is intentional, consider adding it in a separate PR with appropriate documentation explaining its purpose. If it's accidental, it should be removed from this PR.
| await client.call_tool("inspect_app", {"url": "data:text/html,<html></html>", "screenshot": False, "console_logs": False}) | ||
| assert "at least one" in str(exc_info.value).lower() | ||
|
|
There was a problem hiding this comment.
Missing PlaywrightManager cleanup after test execution. This test should follow the same cleanup pattern as other Playwright tests to prevent cross-test browser leaks. Add cleanup code in a finally block or after the assertion, similar to test_inspect_app_rejects_relative_path.
| await client.call_tool("inspect_app", {"url": "data:text/html,<html></html>", "screenshot": False, "console_logs": False}) | |
| assert "at least one" in str(exc_info.value).lower() | |
| await client.call_tool( | |
| "inspect_app", | |
| {"url": "data:text/html,<html></html>", "screenshot": False, "console_logs": False}, | |
| ) | |
| assert "at least one" in str(exc_info.value).lower() | |
| import holoviz_mcp.panel_mcp.server as _srv | |
| if _srv._playwright_manager is not None: | |
| await _srv._playwright_manager.close() | |
| _srv._playwright_manager = None |
| @@ -499,38 +505,129 @@ def _get_playwright_manager() -> PlaywrightManager: | |||
|
|
|||
|
|
|||
| @mcp.tool() | |||
There was a problem hiding this comment.
The tool decorator should use @mcp.tool without parentheses for consistency with other tools in the codebase (list_packages, search_components, list_components, get_component, get_component_parameters). Only use @mcp.tool() with parentheses when passing arguments like enabled=False.
| @mcp.tool() | |
| @mcp.tool |
Summary
panel_take_screenshottopanel_inspect_appand extend it to capture both screenshots and browser console logs in a single call, making it easy to debug JavaScript errors in Panel apps (especially custom components)panel-liveandpanel-reactflowas built-in documentation repositories inconfig.yamldocs/explanation/tools.mdto document the new tool name, parameters, and use casesChanges
panel_inspect_app(renamed frompanel_take_screenshot)The tool now returns a
list[TextContent | ImageContent]instead of a singleImageContent, with three new parameters:screenshotboolTrueconsole_logsboolTruelog_levelstr | NoneNone"error","warning","log", etc.)All existing parameters (
url,width,height,full_page,delay,save_screenshot) are preserved unchanged.Raises
ValueErrorif bothscreenshot=Falseandconsole_logs=False.New built-in doc repositories
panel-extensions/panel-live)panel-extensions/panel-reactflow)Files changed
src/holoviz_mcp/panel_mcp/server.py— Rename tool, add console log capture via Playwrightpage.on("console"), return mixed content listsrc/holoviz_mcp/panel_mcp/models.py— AddConsoleLogEntryPydantic modelsrc/holoviz_mcp/config/config.yaml— Addpanel-liveandpanel-reactflowrepositoriessrc/holoviz_mcp/config/models.py— Updatescreenshots_dirdescription to reference new tool nametests/test_panel_mcp.py— Update tests for renamed tool and add coverage for console log capture, log level filtering, and the error casedocs/explanation/tools.md— Replacepanel_take_screenshotsection withpanel_inspect_appdocumentationTest plan
pixi run test— all existing and new tests passpixi run pre-commit-run— linting, formatting, and type checks passpixi run mkdocs build --strict— docs build without warningspanel_inspect_appreturns both screenshot and console logs against a running Panel applog_levelfiltering returns only matching entriesscreenshot=Falsereturns only console logs (no image)console_logs=Falsereturns only the screenshot (no text)screenshot=False, console_logs=FalseraisesValueError