Skip to content

Enhancement/mcp apps#161

Open
MarcSkovMadsen wants to merge 5 commits into
mainfrom
enhancement/mcp-apps
Open

Enhancement/mcp apps#161
MarcSkovMadsen wants to merge 5 commits into
mainfrom
enhancement/mcp-apps

Conversation

@MarcSkovMadsen

Copy link
Copy Markdown
Owner

No description provided.

Copilot AI review requested due to automatic review settings March 6, 2026 18:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds MCP App-compatible rendering for visualization tools, including a new show_pyodide flow for browser/Pyodide execution, plus URL externalization support for proxied/Codespaces environments.

Changes:

  • Introduces MCP App resources/templates for show and show_pyodide, and returns structured JSON payloads for app rendering.
  • Extends the display system to accept a pyodide execution method and improves URL generation for Jupyter proxy/Codespaces.
  • Adds/updates tests and documentation to cover the new tool behavior and configuration.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/test_server.py Asserts show_pyodide is registered on the composed server.
tests/docs_mcp/test_docs_mcp.py Adds contract tests for show JSON payloads, show_pyodide payloads, and URL rewriting behavior.
tests/display_mcp/test_endpoints.py New endpoint tests for method handling and proxy/Codespaces URL formation.
tests/display_mcp/test_database.py Tests database acceptance/validation of the pyodide method.
tests/config/test_loader.py Verifies display config is preserved and can be overridden by user config.
src/holoviz_mcp/holoviz_mcp/templates/show_pyodide.html Adds an MCP App UI for rendering tool payloads via panel-live/Pyodide.
src/holoviz_mcp/holoviz_mcp/templates/show.html Adds an MCP App UI for rendering show results (JSON + legacy text fallback).
src/holoviz_mcp/holoviz_mcp/server.py Adds App resources, URL externalization helper, and JSON payload output for show / new show_pyodide tool.
src/holoviz_mcp/display_mcp/pages/view_page.py Prepends Panel reset code before executing snippets.
src/holoviz_mcp/display_mcp/endpoints.py Centralizes external base URL derivation (proxy/Codespaces/config).
src/holoviz_mcp/display_mcp/database.py Adds pyodide to supported methods and validates method values.
src/holoviz_mcp/config/loader.py Adds default display config and ensures it isn’t filtered out.
docs/explanation/tools.md Documents show / show_pyodide tools.
docs/explanation/display-system.md Updates architecture explanation to include show_pyodide and pyodide method.
.vscode/settings.json Removes an unused VS Code setting entry.
Comments suppressed due to low confidence (1)

src/holoviz_mcp/holoviz_mcp/server.py:521

  • show() now forwards method through to the display server and the rest of the PR adds support for pyodide, but this tool’s type hints/docs still restrict method to only "jupyter" | "panel". This makes the public MCP API inconsistent with both the docs and the display server behavior; update the Literal[...] (and the docstring section that enumerates methods) to include "pyodide" or remove the Literal if you want to allow future methods without churn.
    code: str,
    name: str = "",
    description: str = "",
    method: Literal["jupyter", "panel"] = "jupyter",
    ctx: Context | None = None,

Comment on lines 131 to 167
@@ -150,7 +161,7 @@ def _execute_code(snippet: Snippet) -> pn.viewable.Viewable | None:
else: # panel method
# Execute code that should call .servable()
execute_in_module(
snippet.app,
app,
module_name=module_name,
cleanup=True, # Can cleanup immediately
)

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Snippet.method now allows "pyodide", but _execute_code() treats any non-"jupyter" method as the Panel execution path, so pyodide snippets would be executed server-side and expected to call .servable(). This contradicts the intent of the pyodide method and can lead to confusing failures. Add an explicit elif snippet.method == "pyodide" branch (e.g., render a message + the source code, or redirect users to the show_pyodide MCP App flow) and keep the current behavior only for "panel".

Copilot uses AI. Check for mistakes.
Comment on lines +108 to +121
if (payload.name) {
title.textContent = payload.name;
}
if (payload.description) {
description.textContent = payload.description;
}

const code = payload.code ?? "";
source.textContent = code || "No code provided.";

if (!code.trim()) {
runner.innerHTML = "";
return;
}

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

show_pyodide can return an error payload (e.g. when code is blank), but the renderer only looks at payload.code and otherwise shows “No code provided.”. This means users won’t see the actual error message in MCP App hosts. Consider rendering payload.error / payload.message when present so error states are visible.

Copilot uses AI. Check for mistakes.
Comment thread docs/explanation/tools.md

**Use Case**: Standard display-server backed visualization flow with URL fallback behavior.

**Returns**: Success/error message containing a view URL.

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The show tool now returns a JSON text payload (as implemented in holoviz_mcp.server.display()), not a plain success/error message string. This “Returns” description is now misleading; update it to describe the JSON payload shape (and/or the fallback behavior in non-MCP-App hosts).

Suggested change
**Returns**: Success/error message containing a view URL.
**Returns**: JSON text payload (as returned by `holoviz_mcp.server.display()`) describing the view, including a view URL and rendering metadata. In non–MCP-App hosts, this may be surfaced to the user as a success/error message string containing the view URL.

Copilot uses AI. Check for mistakes.
Comment on lines 18 to +25
When you use the `show` tool:

1. AI sends code to the MCP server via the tool
2. MCP server makes HTTP request to Display Server
3. Display Server stores the snippet in SQLite database
4. Display Server executes the code and captures output
5. MCP server returns URL to view visualization
6. User accesses visualization via URL in browser
6. User accesses visualization via URL in browser (or via in-chat MCP App UI when supported)

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flow description still says the MCP server “returns URL to view visualization”, but show now returns a JSON payload (with url embedded) for MCP App rendering. Adjust the wording here to reflect the new contract, otherwise this doc contradicts the implementation and the show MCP App template logic.

Copilot uses AI. Check for mistakes.
Comment on lines 43 to +48
The `show` MCP tool is the primary interface for creating visualizations. It accepts:

- **app** (required): Python code to execute
- **name** (optional): Human-readable title for the visualization
- **description** (optional): Explanation of what the code does
- **method** (optional): Execution method - "jupyter" (default) or "panel"
- **method** (optional): Execution method - "jupyter" (default), "panel", or "pyodide"

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the show tool section, the parameter name is documented as app, but the MCP tool parameter is code. Also, the surrounding text claims the tool returns id/created_at, but the current implementation returns a JSON payload with fields like status, message, url, and echoes code. Please update this section to match the actual show tool interface and payload.

Copilot uses AI. Check for mistakes.
Comment on lines +491 to +506
@mcp.resource(
SHOW_RESOURCE_URI,
app=AppConfig(
csp=ResourceCSP(
resource_domains=[
"'unsafe-inline'",
"https://unpkg.com",
],
frame_domains=[
"http://localhost",
"http://127.0.0.1",
"https://localhost",
"https://127.0.0.1",
"https://*.app.github.dev",
"https://*.github.dev",
],

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The show MCP App template embeds the visualization URL in an <iframe>, but the CSP frame_domains here only whitelists localhost and *.github.dev. When JUPYTER_SERVER_PROXY_URL / server.jupyter_server_proxy_url is set to a custom domain, _externalize_display_url() will return that domain and the iframe will likely be blocked by this CSP. Consider either (a) adding an allow-list entry for the configured proxy host, (b) relaxing frame_domains to a broader pattern (e.g. https:) if acceptable, or (c) not setting a restrictive frame_domains directive for this resource.

Copilot uses AI. Check for mistakes.
_internalize_url() converts externalized URLs (GitHub Codespaces port
forwarding, Jupyter server proxy) back to localhost before Playwright
opens them, so inspect_app() works correctly when panel-live-server
externalizes the URL for the end user.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants